Closed microbit-sam closed 6 years ago
Should there be a new config option to control creation of the PF service in MicroBitBLEManager::init? If so perhaps this PR shouldn't increase the default MICROBIT_SD_GATT_TABLE_SIZE.
The PF service uses the MemoryMap object but doesn't seem to use its ability to write the map to flash. The MemoryMap class doesn't have any code to read the map from flash.
Should there be a config option to control the inclusion of the MemoryMap flashing code and whether it shifts DEFAULT_SCRATCH_PAGE to reserve a flash page?
The PF service constructor creates a MemoryMap instance, but this doesn't do anything.
Should this PR change the version in module.json?
What is the reason for the change in MicroBitFlash::erase_page, where it doesn't now retry sd_flash_page_erase? The top while loop is pointless in the new version.
@microbit-sam This is really great! :)
Just being thorough with code review here as patches like this are really very critical to get right. Very excited to see this going forward. Are you happy to work on an update PR?
Note for myself (if anyone else spots anything feel free to edit):
Hi @microbit-sam
I'd like to merge this in soon - how are you getting on with those "notes to self" changes listed above?
Closing because this has been merged into microbit-dal via the dal-integration branch
is there a spec for the service? (i guess the code is the spec)
There's some (slightly out of date) docs here: https://github.com/microbit-sam/microbit-docs/blob/master/docs/ble/partial-flashing-service.md
We're currently working through the backlog of python/dal/support docs every morning so this will be up to date and on the lancaster uni docs soon
Is the Android sample still up to date?
@microbit-sam to write PR info
@finneyj