lancaster-university / codal-core

MIT License
11 stars 28 forks source link

Add target_hash_serial_number() #119

Open mmoskal opened 4 years ago

mmoskal commented 4 years ago

It is to be used by target_get_serial().

It uses FNV-1a for lower half of the serial and murmur2 for the upper half. That should give reasonably good distribution.

Example on SAMD21 (yes, words of serial are not all adjacent).

uint64_t target_get_serial()
{
    static uint64_t cache;
    if (!cache)
    {
        uint32_t serial[4];
        serial[0] = *(uint32_t *)0x0080A00C;
        memcpy(serial + 1, (void *)0x0080A040, 3 * 4);
        cache = target_hash_serial_number(serial, 4);
    }
    return cache;
}

(the current version on D21, D51 is broken, hashing on STM32 is questionable; NRF52 has 64 bit serial, but I would be inclined to hash it anyway)

jamesadevine commented 4 years ago

The api for getting the hashed serial seems a bit uncouth... Could we instead have two variants? One that returns the "true" serial number, and one that returns the hashed version?

That's just a small inversion to the code you proposed. Instead it would be:

uint64_t target_hashed_serial()
{
     uint64_t serial = target_get_serial();
     uint32_t w1 = hash_fnv1a(&serial, 2 << 2);
     uint32_t w0 = murmur_hash2_core((uint32_t *)&serial, 2);
     w0 &= ~0x02000000; // clear "universal" bit
     return ((uint64_t)w0 << 32 | w1);
}
mmoskal commented 4 years ago

Well, the api for real serial would need to return a buffer then.

Given that all codal_target_hal.cpp files will need to be updated, I'll do some cleanup there.

jamesadevine commented 4 years ago

Return a buffer because there is no standard length for mcu identifiers?

Yes, in that case it may make sense to return a ManagedBuffer for the full serial number, and we use the hashing mechanism to coerce any serial number into 64-bits.

jamesadevine commented 4 years ago

I don't think it makes sense to change the api for target_get_serial. As you proposed, we should change the underlying implementation of target_get_serial to become a hash, and supply an alternative api for the full N-bit identifier.

target_get_serial should perhaps get the true serial number from this new api?

mmoskal commented 4 years ago

OK, added new API. Also added default implementation of target_* functions for arm, so we can stop repeating that across all targets. The codal_target_hal.cpp in SAMD21 is now:

#include "codal_target_hal.h"

void target_reset()
{
    NVIC_SystemReset();
}

unsigned target_get_serial_buffer(void *dst, unsigned maxbytes)
{
    if (maxbytes >= 20)
    {
        uint32_t *serial = (uint32_t *)dst;
        // random number to avoid collisions between serial numbers of different manufacturers
        serial[0] = 0xf8495164;
        serial[1] = *(uint32_t *)0x0080A00C;
        memcpy(serial + 2, (void *)0x0080A040, 3 * 4);
    }
    return 20;
}