FOTA Service #60

AGlass0fMilk · 2021-06-03T17:14:19Z

This PR contains the FOTA service and associated tests as outlined in #46.

Closes #46

Co-Authored-By: George Beckstein <becksteing@embeddedplanet.com>

This allows an application to add its own logic for accepting writes to the FOTA update characteristic.

AGlass0fMilk · 2021-06-07T17:03:59Z

After working with this service on some projects, I think we should introduce another standard FOTA op-code, "FOTA confirm".

Bootloaders like Mcuboot support rolling back firmware updates if they do not mark themselves as "good". ie: the firmware has a severe bug that prevents normal operation. When reset, mcuboot will then revert the update to prevent "bricking".

One possible failure of this system is, since the update mechanism is in the application, there is a possibility a fatal bug could be introduced into the update mechanism code. This would prevent any future firmware upgrades to the device. The FOTA Confirm op-code would allow the FOTA client to mark the updated firmware as "OK".

This is essentially a self-test of the firmware update mechanism in the BLE FOTA service case -- if BLE can be brought up, a connection can be made, and a characteristic can be written, it is pretty safe to assume that the FOTA code has not been broken by the update.

There are some other recommended self-tests that should be performed, such as testing update flash memory initialization.

I think this would be a common enough use case to deserve a dedicated, standard op-code.

@pan- Thoughts on this? This is a problem for all field-upgradeable devices where the update mechanism is in the application itself. If Mbed-OS creates a framework for DFU, it should include features like a flexible, easy-to-integrate "factory reset" that can reinstall an image stored on a dedicated "recovery partition" (eg: a well-tested application that does nothing but let the firmware be reprogrammed in the case of a broken update).

EDIT: Mcuboot has features that allow multiple images to be selected from for boot. This would be a use case for that.

noonfom · 2021-06-11T12:24:47Z

.github/workflows/run_tests.yml

@@ -56,3 +56,4 @@ jobs:
          ./tests/TESTS/build.sh -s DeviceInformation -t GCC_ARM -m NRF52840_DK
          ./tests/TESTS/build.sh -s LinkLoss -t GCC_ARM -m NRF52840_DK
          ./tests/TESTS/build.sh -s LinkLoss -t GCC_ARM -m DISCO_L496AG
+          ./tests/TESTS/build.sh -s FOTA -t GCC_ARM -m NRF52840_DK


It should also work on disco. There are MCUboot problems but the mock example works fine locally, i.e. I can transfer a binary into the external flash of my B-L4S5I-IOT01A using the FOTA service.

Note: The disco build is also missing for the Device Information service: #62

noonfom · 2021-06-11T12:24:51Z

common/inc/BaseService.h

+//};
+
+
+#endif /* MBED_OS_EXPERIMENTAL_BLE_SERVICES_INC_BASESERVICE_H_ */


It's a good idea but perhaps we should add it in a separate PR with the other utility files:

ConnectionThingy.h

GattPresentationFormatDescriptor.h

CharacteristicPresentationFormatDescriptor.h

CharacteristicUserDescriptionDescriptor.h

Ha, sorry, these files weren't supposed to be included in this PR 😅

noonfom · 2021-06-11T12:24:55Z

common/inc/ConnectionThingy.h

+ *
+ * This creates a lot of boilerplate code where a service must filter out by comparing
+ * which handle was written to the callback's parameters, figure out which connection,
+ * and then perform the appropriate action.


Do setWriteAuthorizationCallback and setReadAuthorizationCallback not solve this problem?

Unrelated to the PR at hand, but:

Those APIs "solve" the problem, but I think it creates a confusing situation in the BLE API:
You can still handle characteristic writes that are authorized but you then have to filter them based on the value handle. It creates two ways to do the same thing which can be confusing to users.

The naming of "setWriteAuthorizationCallback" and "setReadAuthorizationCallback" imply they are only for read authorization/write authorization. Likewise, the naming of GattServer::EventHandler::onCharacteristicWritten (or whatever it is) implies it should be where characteristic writes are handled.

noonfom · 2021-06-11T12:24:58Z

common/inc/ConnectionThingy.h

+
+
+
+#endif /* MBED_OS_EXPERIMENTAL_BLE_SERVICES_COMMON_INC_CONNECTIONTHINGY_H_ */


See comment under BaseService.h

noonfom · 2021-06-11T12:25:02Z

common/inc/GattPresentationFormatDescriptor.h

+		uint8_t format[7];
+};
+
+#endif /* EP_CORE_FEATURES_FEATURE_BLE_GATTPRESENTATIONFORMATDESCRIPTOR_H_ */


See comment under BaseService.h

noonfom · 2021-06-11T12:25:06Z

descriptors/CharacteristicPresentationFormatDescriptor.h

+        uint8_t format[7];
+};
+
+#endif /* MBED_OS_EXPERIMENTAL_BLE_SERVICES_DESCRIPTORS_CHARACTERISTICPRESENTATIONFORMATDESCRIPTOR_H_ */


See comment under BaseService.h

noonfom · 2021-06-11T12:25:10Z

descriptors/CharacteristicUserDescriptionDescriptor.h

+
+};
+
+#endif /* MBED_OS_EXPERIMENTAL_BLE_SERVICES_DESCRIPTORS_CHARACTERISTICUSERDESCRIPTIONDESCRIPTOR_H_ */


See comment under BaseService.h

noonfom · 2021-06-11T12:25:14Z

scripts/bootstrap.sh


-symlink dependencies/mbed-os       tests/TESTS/DeviceInformation/device/mbed-os
-symlink services/DeviceInformation tests/TESTS/DeviceInformation/device/DeviceInformation


Both files are required to build the integration tests for the Device Information service. Can you add them back in?

noonfom · 2021-06-11T12:25:17Z

scripts/bootstrap.sh


-# Create mbed-os.lib for CMake builds
-echo "https://github.com/ARMmbed/mbed-os" > tests/TESTS/LinkLoss/device/mbed-os.lib
-echo "https://github.com/ARMmbed/mbed-os" > tests/TESTS/DeviceInformation/device/mbed-os.lib


Mbed CLI 2 requires that a mbed-os.lib be present in the project root.

The following is also required for the FOTA service:

echo "https://github.com/ARMmbed/mbed-os" > tests/TESTS/FOTA/device/mbed-os.lib

noonfom · 2021-06-11T12:25:34Z

tests/TESTS/FOTA/device/.mbed

@@ -0,0 +1 @@
+ROOT=.


hmm... I don't think we should have .mbed files in device folders.

noonfom · 2021-06-11T12:25:40Z

tests/TESTS/FOTA/device/mbed-os.lib

@@ -0,0 +1 @@
+https://github.com/ARMmbed/mbed-os


That should be added during bootstrap.

paul-szczepanek-arm and others added 8 commits June 3, 2021 13:12

Add FOTA service documentation

fda3c95

Co-Authored-By: George Beckstein <becksteing@embeddedplanet.com>

Add FOTA service and dependencies

9f3d3a3

Co-Authored-By: George Beckstein <becksteing@embeddedplanet.com>

Add FOTA unittests

6072a67

Co-Authored-By: George Beckstein <becksteing@embeddedplanet.com>

Add FOTA integration tests

be56f99

Co-Authored-By: George Beckstein <becksteing@embeddedplanet.com>

add FOTA integration test build to workflow

698eb06

Co-Authored-By: George Beckstein <becksteing@embeddedplanet.com>

Add extra utility functions

6779524

Add sync lost warning trace message

7ec2fe6

Make the control write request authorization callback virtual

3060a98

This allows an application to add its own logic for accepting writes to the FOTA update characteristic.

AGlass0fMilk requested review from noonfom, paul-szczepanek-arm and pan- June 7, 2021 17:05

noonfom reviewed Jun 11, 2021

View reviewed changes

common/inc/ConnectionThingy.h

#endif /* MBED_OS_EXPERIMENTAL_BLE_SERVICES_COMMON_INC_CONNECTIONTHINGY_H_ */

Copy link

noonfom Jun 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment under BaseService.h

noonfom reviewed Jun 11, 2021

View reviewed changes

tests/TESTS/FOTA/device/.mbed

@@ -0,0 +1 @@

ROOT=.

Copy link

noonfom Jun 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... I don't think we should have .mbed files in device folders.

noonfom reviewed Jun 11, 2021

View reviewed changes

tests/TESTS/FOTA/device/mbed-os.lib

@@ -0,0 +1 @@

https://github.com/ARMmbed/mbed-os

Copy link

noonfom Jun 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be added during bootstrap.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FOTA Service #60

FOTA Service #60

AGlass0fMilk commented Jun 3, 2021

AGlass0fMilk commented Jun 7, 2021 •

edited

Loading

noonfom Jun 11, 2021

noonfom Jun 11, 2021

AGlass0fMilk Jun 30, 2021

noonfom Jun 11, 2021

AGlass0fMilk Jun 30, 2021 •

edited

Loading

noonfom Jun 11, 2021

noonfom Jun 11, 2021

noonfom Jun 11, 2021

noonfom Jun 11, 2021

noonfom Jun 11, 2021

noonfom Jun 11, 2021

noonfom Jun 11, 2021

noonfom Jun 11, 2021

		//};


		#endif /* MBED_OS_EXPERIMENTAL_BLE_SERVICES_INC_BASESERVICE_H_ */


		symlink dependencies/mbed-os tests/TESTS/DeviceInformation/device/mbed-os
		symlink services/DeviceInformation tests/TESTS/DeviceInformation/device/DeviceInformation

FOTA Service #60

Are you sure you want to change the base?

FOTA Service #60

Conversation

AGlass0fMilk commented Jun 3, 2021

AGlass0fMilk commented Jun 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AGlass0fMilk Jun 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AGlass0fMilk commented Jun 7, 2021 •

edited

Loading

AGlass0fMilk Jun 30, 2021 •

edited

Loading