search

lancaster-university / codal-microbit-v2

CODAL target for the micro:bit v2.x series of devices
MIT License
43 stars 52 forks source link

Instantiating BLE classes with DEVICE_BLE vs MICROBIT_BLE_ENABLED #318

Open microbit-carlos opened 1 year ago

microbit-carlos commented 1 year ago

MicroBit.h/.cpp declares and initialises MicroBitBLEManager and BLEDevice instances only checking for the DEVICE_BLE config flag only: https://github.com/lancaster-university/codal-microbit-v2/blob/dfc0adc6fb820c965ee5913e0650ff5baaa438e9/model/MicroBit.h#L148-L153 https://github.com/lancaster-university/codal-microbit-v2/blob/dfc0adc6fb820c965ee5913e0650ff5baaa438e9/model/MicroBit.cpp#L58-L61

But then it only runs the bleManager.init() if MICROBIT_BLE_ENABLED is enabled as well: https://github.com/lancaster-university/codal-microbit-v2/blob/dfc0adc6fb820c965ee5913e0650ff5baaa438e9/model/MicroBit.cpp#L281-L284

I assume this is by design, as maybe the classes need to be instantiated for other BLE configuration (maybe partial flashing as well?), but bleManager.init() is only needed if we wanted to start other services ? Just wanted to double check this is correct.

microbit-carlos commented 1 year ago

Apart from that there is also this validateBootloaderSettings() run with DEVICE_BLE only:

https://github.com/lancaster-university/codal-microbit-v2/blob/dfc0adc6fb820c965ee5913e0650ff5baaa438e9/model/MicroBit.cpp#L181-L185

martinwork commented 1 year ago

The design comes from the DAL. https://github.com/lancaster-university/microbit/blob/master/inc/MicroBit.h#L123 https://github.com/lancaster-university/microbit/blob/master/source/MicroBit.cpp#L82

DEVICE_BLE is new in CODAL. It came about because early CODAL versions had NO_BLE, used to temporarily exclude all BLE code inherited from the DAL.

Note that bleManager is at the top of the uBit member variable list because of this. https://github.com/lancaster-university/codal-microbit-v2/blob/master/source/bluetooth/MicroBitBLEManager.cpp#L178

microbit-carlos commented 1 year ago

Oh right, thanks Martin! Also important to note that the "no soft device" mode is now configured by setting DEVICE_BLE to 0. In that case it also excludes SD all BLE related code, probably similarly to how NO_BLE used to work.

martinwork commented 1 year ago

Yes DEVICE_BLE is NO_BLE renamed and with reversed logic. We weren't really sure that we needed to keep DEVICE_BLE, but it looks like we chose right! https://github.com/microbit-foundation/codal-microbit/pull/50/files/675d948a25da50c688cc68ce5f788975fb44f006#diff-4ec23bebe492b0ab02ca146f33e70404abb3cb5735d87ef322f35523bac18fbd (private)