randint() not random when bluetooth enabled

Amerlander commented 9 months ago

Describe the bug When pxt is set to have bluetooth anabled the randint() block always produces the same number. Regardless of v1 or v2.

To Reproduce Steps to reproduce the behavior:

Go to https://makecode.microbit.org
Add bluetooth extension
Set code to basic.showNumber(randint(0, 9))
See every time the microbit starts, the same number (one) is displayed

Alternative check this shared project where ble was enabled without loading the bluetooth extension: https://makecode.microbit.org/S91179-28567-56186-10264

Expected behavior I expect the microbit to show a random number that is not predictable.

micro:bit version (please complete the following information): Tested on 1.3, but happens on v1 and v2 afaik.

Additional context On v1 the seedrandom function checks if ble is enabled. If so it tries to use nrf functions for generating a seed and if that fails it is a fix seed. On v2 it is always a fix seed if ble is enabled: https://github.com/lancaster-university/microbit-dal/blob/602153e9199c28c08fd2561ce7b43bf64e9e7394/source/core/MicroBitDevice.cpp#L340

The init function cals seedrandom(): https://github.com/lancaster-university/microbit/blob/1d1baa561f82ab0d16d1916b0026955c67d95e74/source/MicroBit.cpp#L118C18-L118C18 But since it gets called before ble is started, it should not matter what value in BLE_ENABLED is set.

I tried to reproduce this error by enabling bluetooth in the microbit samples, but I was not able to reproduce it. I always get random numbers on the samples.
I tried to replace the static seed fall back with random numbers generated by the sensors, but that still fails.
randint() is defined in pxt, but I didn't figure out how this actually calls the microbit random number functions. Since I was not able to reproduce this issue in the samples, I wonder if there is something on the pxt side I'm missing.

abchatra commented 9 months ago

@carlosperate @JohnVidler

abchatra commented 6 months ago

I can reproduce the bug on v2 hardware. The sequence of numbers are always 1,7,8 etc when the hardware is reset and random number is generated on button A pressed.

https://makecode.microbit.org/_EaXWh9CzjFWp

microbit-carlos commented 6 months ago

@Amerlander would you be able to provide a C++ example that replicates the issue in CODAL? that will help us replicating it and debugging it.

Amerlander commented 6 months ago

@microbit-carlos unfortunately no, that's why I think this is a microbit-pxt and not a DAL/CODAL issue, as mentioned in my initial post:

I tried to reproduce this error by enabling bluetooth in the microbit samples, but I was not able to reproduce it. I always get random numbers on the samples.

It produces non-random ints, when "MICROBIT_BLE_ENABLED" is set to 1 in microbit-pxt, the same program in C++ is working fine.

The code, that should - but actually doesn't - replicate this issue in CODAL would be:

#include "MicroBit.h"

MicroBit uBit;

int main()
{
    uBit.init();
    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
    }
}

microbit-carlos commented 6 months ago

Thanks @Amerlander!

Would adding this code to the CODAL example make a difference? https://github.com/microsoft/pxt-microbit/blob/53e49a9ec58a5c5b3b9214d4898298ef1dac7662/libs/core/codal.cpp#L52-L57 seedRandom() is probably from here, but assuming the seed is different every time it should technically work: https://github.com/microsoft/pxt-common-packages/blob/5148fecf59dbbef876ac53c3a877d814da6de0dd/libs/base/core.cpp#L333

microbit-carlos commented 6 months ago

randint() is defined in pxt, but I didn't figure out how this actually calls the microbit random number functions. Since I was not able to reproduce this issue in the samples, I wonder if there is something on the pxt side I'm missing.

@abchatra I couldn't easily find the randint() implementation either, could somebody point to how this function is implemented? As @Amerlander has commented, it might be an issue in the pxt side.

Amerlander commented 6 months ago

Would adding this code to the CODAL example make a difference?

Yes. Using this code reproduces the issue, when BLE is enabled and is working when BLE is disabled

main.cpp in microbit-v2-samples:

#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);
}

codal.json that produces random ints:

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

Changing the Config to enable BLE will always give the same number as first random int:

"MICROBIT_BLE_ENABLED" : 1

Amerlander commented 6 months ago

Uncommenting the line microbit_seed_random(); in libs/core/codal.cpp solves the issue for me: https://github.com/microsoft/pxt-microbit/blob/53e49a9ec58a5c5b3b9214d4898298ef1dac7662/libs/core/codal.cpp#L53

It seems like if microbit_seed_random(); is called after BLE is initialized in the main init() function, the seed stops being random. Since the init() function already calls microbit_seed_random(); right before starting the BLE service it should not be required to call it in platform_init() again.

But I'm not that much into the CODAL functions, so I might be wrong.

abchatra commented 1 week ago

@microbit-carlos I am going to assign this bug to you and move to hotfix milestone as the fix seems to be on the codal side.

microsoft / pxt-microbit

randint() not random when bluetooth enabled #5409