MakeCode randint() not random when Bluetooth enabled

[microbit-carlos]

From:

• randint() not random when bluetooth enabled microsoft/pxt-microbit#5409

Replicable example from:

• randint() not random when bluetooth enabled microsoft/pxt-microbit#5409 (comment)

#include "MicroBit.h"

MicroBit uBit;

int main()
{
    uBit.init();
     microbit_seed_random(); 
     int seed = microbit_random(0x7fffffff); 
     DMESG("random seed: %d", seed); 
     uBit.seedRandom(seed); 
     int randomInt = uBit.random(9);
     uBit.display.print(randomInt);
}

{
    "target": {
        "name": "codal-microbit-v2",
        "url": "https://github.com/lancaster-university/codal-microbit-v2",
        "branch": "master",
        "type": "git"
    },
    "config":{
        "MICROBIT_BLE_ENABLED" : 1
    }
}

With this example the random numbers are always seeded with the same value. Changing MICROBIT_BLE_ENABLED to 1 works.

Possible solution:

• randint() not random when bluetooth enabled microsoft/pxt-microbit#5409 (comment)

[martinwork]

@microbit-carlos

seedRandom() does not generate a random seed when BLE is running

codal-microbit-v2/source/MicroBitDevice.cpp

Line 67 in 96faa85

void MicroBitDevice::seedRandom()

seedRandom is called before BLE is enabled

codal-microbit-v2/model/MicroBit.cpp

Line 197 in 96faa85

seedRandom();

The seed for subsequent calls to seedRandom while BLE is running could be made pseudo random by using:
r = random(INT_MAX);

The softdevice calls work - they don't seem to need a call to start the RNG peripheral
But I don't see an equivalent of NRF_RNG->TASKS_STOP = 1 to stop the RNG afterwards.

        for (int i = 0; i < 4; i++)
        {
            // Wait for a number to be generated.
            uint8_t byte;
            do
            {
              sd_rand_application_bytes_available_get( &byte);
            }
            while( byte == 0);

            sd_rand_application_vector_get( &byte, 1);

            r = (r << 8) | ((int) byte);
        }

[microbit-carlos]

Thanks Martin, that's really useful!
I was playing with this and noticed that waiting until sd_rand_application_bytes_available_get() returns 4 (so that it has 4 random bytes) can take over 12 milliseconds.

Looking at the datasheet it should be 128 us to start, and 120 per byte, so 128 + 120 * 4 = 608 microseconds. Not sure why it's taking 20 times longer. Anything similar you've seen or experienced @martinwork @finneyj ?

[martinwork]

@microbit-carlos Good catch! I followed the non-BLE code without thinking, and didn't look at the DAL either!
https://github.com/lancaster-university/microbit-dal/blob/master/source/core/MicroBitDevice.cpp#L343

If we can guarantee the call will eventually work, maybe this is OK.
`while ( sd_rand_application_vector_get((uint8_t*)&r, sizeof(r))) /empty/;

It's quicker than the byte by byte version.
`
It seems it may take a while immediately after initialising softdevice
https://devzone.nordicsemi.com/f/nordic-q-a/47707/how-to-use-sd_rand_application_vector_get-correctly

I see up to 810 microseconds, but not 12ms. It seems a bit random.

#include "MicroBit.h"

MicroBit uBit;

void seedRandom()
{
    uint32_t r = 0xBBC5EED;

    if(!ble_running())
    {
        // Start the Random number generator. No need to leave it running... I hope. :-)
        NRF_RNG->TASKS_START = 1;

        for(int i = 0; i < 4; i++)
        {
            // Clear the VALRDY EVENT
            NRF_RNG->EVENTS_VALRDY = 0;

            // Wait for a number ot be generated.
            while(NRF_RNG->EVENTS_VALRDY == 0);

            r = (r << 8) | ((int) NRF_RNG->VALUE);
        }

        // Disable the generator to save power.
        NRF_RNG->TASKS_STOP = 1;
    }
    else
    {
      CODAL_TIMESTAMP t0, t1;
      
      t0 = system_timer_current_time_us();
      while ( sd_rand_application_vector_get((uint8_t*)&r, sizeof(r))) /*empty*/;
      t1 = system_timer_current_time_us();
      DMESGN("%d, ", (int)(t1-t0));

      //t0 = system_timer_current_time_us();
      //for (int i = 0; i < 4; i++)
      //{
      //    // Wait for a number to be generated.
      //    uint8_t byte;
      //    do
      //    {
      //      sd_rand_application_bytes_available_get( &byte);
      //    }
      //    while( byte == 0);

      //    sd_rand_application_vector_get( &byte, 1);

      //    r = (r << 8) | ((int) byte);
      //}
      //t1 = system_timer_current_time_us();
      //DMESGN("%d, ", (int)(t1-t0));

      DMESG("");
    }

    uBit.seedRandom(r);
}


int main()
{
    int randomInt = 0;

    uBit.init();

    seedRandom();

    while (true)
    {
      uBit.display.print('*');
      uBit.sleep(1000);

      uBit.seedRandom(); 
      randomInt = uBit.random(9);
      uBit.display.print( randomInt);
      uBit.sleep(1000);

      seedRandom();
      randomInt = uBit.random(9);
      uBit.display.print( randomInt);
      uBit.sleep(1000);
    }
}

[martinwork]

The non-BLE code takes around 240us every time.

Waiting for 4 bytes with sd_rand_application_bytes_available_get takes a bit longer because of the extra code.

Quite a long wait (between 500 and 1000us) is needed to remove the randomness of the first call, and a wait inside the loop doesn’t seem to help.

Code variations

#include "MicroBit.h"

MicroBit uBit;

void seedRandom()
{
    uint32_t r = 0xBBC5EED;

    if(!ble_running())
    {
        // Start the Random number generator. No need to leave it running... I hope. :-)
        NRF_RNG->TASKS_START = 1;

        for(int i = 0; i < 4; i++)
        {
            // Clear the VALRDY EVENT
            NRF_RNG->EVENTS_VALRDY = 0;

            // Wait for a number ot be generated.
            while(NRF_RNG->EVENTS_VALRDY == 0);

            r = (r << 8) | ((int) NRF_RNG->VALUE);
        }

        // Disable the generator to save power.
        NRF_RNG->TASKS_STOP = 1;
    }
    else
    {
      while ( sd_rand_application_vector_get((uint8_t*)&r, sizeof(r))) 
      {
        //target_wait_us(100);
      }


      //uint8_t byte;
      //do
      //{
      //  sd_rand_application_bytes_available_get( &byte);
      //}
      //while( byte < sizeof(r));
      //sd_rand_application_vector_get( (uint8_t*)&r, sizeof(r));


      //for (int i = 0; i < 4; i++)
      //{
      //    // Wait for a number to be generated.
      //    uint8_t byte;
      //    do
      //    {
      //      sd_rand_application_bytes_available_get( &byte);
      //    }
      //    while( byte == 0);
      //    sd_rand_application_vector_get( &byte, 1);
      //    r = (r << 8) | ((int) byte);
      //}
    }

    uBit.seedRandom(r);
}


int main()
{
    CODAL_TIMESTAMP t0, t1;
    int randomInt = 0;

    uBit.init();

    target_wait_us(1000);

    //t0 = system_timer_current_time_us();
    //uBit.seedRandom();
    //t1 = system_timer_current_time_us();
    //uBit.sleep(1000);
    //DMESG("uBit.seedRandom %d", (int)(t1-t0));

    t0 = system_timer_current_time_us();
    seedRandom();
    t1 = system_timer_current_time_us();
    uBit.sleep(1000);
    DMESG("     seedRandom %d", (int)(t1-t0));

    while (true)
    {
      uBit.display.print('*');
      uBit.sleep(1000);

      t0 = system_timer_current_time_us();
      uBit.seedRandom();
      t1 = system_timer_current_time_us();
      DMESG("uBit.seedRandom %d", (int)(t1-t0));

      randomInt = uBit.random(9);
      uBit.display.print( randomInt);
      uBit.sleep(1000);

      t0 = system_timer_current_time_us();
      seedRandom();
      t1 = system_timer_current_time_us();
      DMESG("     seedRandom %d", (int)(t1-t0));

      randomInt = uBit.random(9);
      uBit.display.print( randomInt);
      uBit.sleep(1000);
    }
}

[microbit-carlos]

I think maybe we are seeing differences in time because you are seeding and measuring after uBit.init(), which has a sleep(10) at the end, so there is already a 10ms wait since the softdevice is initialised.

I'm getting my measurements by moving seedRandom() from within uBit.init() from the top to the end, right before the sleep(10) and I'm measuring how long seedRandom() takes to return there:

codal-microbit-v2/model/MicroBit.cpp

Lines 297 to 306 in bd446c4

	#endif
	// Deschedule for a little while, just to allow for any components that finialise initialisation
	// as a background task, and to allow the power mamanger to repsonse to background events from the KL27
	// before any user code begins running.
	sleep(10);
	return DEVICE_OK;
	}

    #endif 

    // Seed our random number generator
    uint32_t t = system_timer_current_time_us();
    seedRandom();
    uint32_t t2 = system_timer_current_time_us();
    serial.printf("seed t: %d\n", t2 - t);

    // Deschedule for a little while, just to allow for any components that finialise initialisation
    // as a background task, and to allow the power mamanger to repsonse to background events from the KL27
    // before any user code begins running.
    
    sleep(10);

    return DEVICE_OK;
}

In this case it does make sense to move it after the sleep, but I still wonder why it would take so long...

[martinwork]

Oh, yes! I see the same - sometimes over 13ms. That explains why a ms pause is mentioned in
https://devzone.nordicsemi.com/f/nordic-q-a/47707/how-to-use-sd_rand_application_vector_get-correctly

For the initial seedRandom call in MicroBit:init(), I think it should stay before BLE is initialised. I have wondered why it wasn't further up, say before the component init() loop at least.

The BLE mode problem was revealed by MakeCode's extra call after uBit.init (which I don't understand).
https://github.com/microsoft/pxt-microbit/blob/7cc19bb881b1404071a521a3d290ef980eca063a/libs/core/codal.cpp#L52

It looks like MakeCode has a fair amount of code between uBit.init() and platform_init(), which hopefully would act like a long wait.

[microbit-carlos]

Yes, you are right, I was mostly playing around with a few different options initially when I encountered the odd timings, and 12ms felt excessive.
So we don't need to move seedRandom() within uBit.init(), as in its current position softdevice has not been initialised and ble_running() returns false, so the initial seed is successfully random.

The issue at hand in CODAL is (as you have mentioned) that because MakeCode re-seeds on its platform_init() function, at that point CODAL softdevice is running, and so MicroBitDevice::seedRandom() currently always uses 0xBBC5EED as the seed.

So, we should add the softdevice call in MicroBitDevice::seedRandom().

However, the weird thing is that in MakeCode, this is also a problem for V1, and in DAL we already have this sd code:
https://github.com/lancaster-university/microbit-dal/blob/318c6abbb3d952d3f49a595a726486230e82fb5b/source/core/MicroBitDevice.cpp#L340-L347

So not sure yet what might be the problem in V1, maybe sd_rand_application_vector_get() is not succeeding?

[martinwork]

Oh, I hadn't realised the report included V1. The problem there is that the V1 code gives up after one try rather than looping until it succeeds. I noticed this but thought maybe it happened to be working in V1. It does seem to work first time after the start-up delay.

[microbit-carlos]

Thanks Martin for all the help here!

I've created this PR code codal-microbit-v2:

• microbit_seed_random() doesn't always create a random seed when SD is running microbit-dal#495

This issue for DAL:

• microbit_seed_random() doesn't always create a random seed when SD is running microbit-dal#495

And this workaround for MakeCode (as it'll take a while for the DAL fix to be released):

• Remove RNG seed call in platform_init(). microsoft/pxt-microbit#5945