-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add FOTA Service mock example #370
Conversation
For now, let's use my mbed-os fork as ARMmbed/mbed-os#14574 is stuck in review. |
Add comments for the update BlockDevice hook and demo FOTA event handler
BLE_GattServer_FOTAService/source/BlockDeviceFOTAEventHandler.cpp
Outdated
Show resolved
Hide resolved
BLE_GattServer_FOTAService/source/BlockDeviceFOTAEventHandler.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent submission @noonfom. Once it passes CI we can merge and iterate.
FOTA_OP_CODE_SET_FRAGMENT_ID = bytearray(b'\x43') | ||
|
||
# Number of bytes in a single BSC packet, excluding fragment ID | ||
FRAGMENT_SIZE = 128 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be to 19
so it will work on all targets. Ideally we would get the MTU of the link and derive fragment size from it. There's a PR open in Bleak for that: hbldh/bleak#525 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't be 19 as that is not a multiple of the program block size (= 4 for nRF & DISCO). In general, this is a QSPIF configuration option that we can override in mbed_app.json
:
{
"target_overrides": {
"*": {
"QSPI_MIN_READ_SIZE": "19",
"QSPI_MIN_PROG_SIZE": "19"
}
}
}
Since 19 is prime. That will only work for nRF and DISCO which use QSPIF. Targets with a different default block device instance will require separate overrides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's interesting, we probably need some sort of buffering in the client side to accommodate between the block size and packet length.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave this open as it's a good observation and the bleak PR is worth keeping an eye. However, a fragment size of 128 data bytes + 1 fragment ID byte (=129 bytes) should work on nRF and DISCO due to the cordio.desired-att-mtu
override in our mbed-app.json
. Making this 19 would also affect the transfer speed. We can iterate on this PR once it lands in the FOTA service example branch. For now, perhaps we can simply add a note about hardware support in the README?
packet_data = bytearray(get_chunk_n(data, FRAGMENT_SIZE, n)) | ||
packet_size = len(packet_data) | ||
if packet_size < FRAGMENT_SIZE: | ||
binary_sent = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you handle the case where the last fragment is not received ? It seems to go out of this loop but a new status can be posted by the server.
BLE_GattServer_FOTAService/source/BlockDeviceFOTAEventHandler.cpp
Outdated
Show resolved
Hide resolved
Repetitive traces left by the target and client affect the time it takes to transfer the binary. Set these as DEBUG and update the trace level to INFO.
Co-authored-by: Vincent Coubard <[email protected]>
Co-authored-by: Vincent Coubard <[email protected]>
This PR adds a mock (i.e. no bootloader) example for the FOTA service.
Co-authored by @AGlass0fMilk