applications: nrf_desktop: Add SMP SUIT DFU support for nrf54h20 #15553

zycz · 2024-05-28T16:56:31Z

Add SMP SUIT DFU support for nrf_desktop application.

JIRA: NCSDK-26998

NordicBuilder · 2024-05-28T16:58:07Z

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
desktop52_verification	X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

NordicBuilder · 2024-05-28T17:50:30Z

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

kapi-no

Looking good, suggested a few improvements

applications/nrf_desktop/src/modules/dfu_mcumgr.c

applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/prj_release.conf

applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/prj.conf

applications/nrf_desktop/src/modules/Kconfig.dfu_mcumgr

applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/app.overlay

applications/nrf_desktop/src/modules/dfu_mcumgr.c

MarekPieta · 2024-05-29T12:21:04Z

applications/nrf_desktop/src/modules/dfu_mcumgr.c

+				suit_manifest_class_id_t *)&manifest_class_id,
+				&seq_num, NULL, NULL, NULL, NULL);
+	if (!err) {
+		LOG_INF("SUIT image version: %d", seq_num);


Can we call it SUIT sequence number? To be consistent with how SUIT names it.

applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/prj.conf

MarekPieta · 2024-05-29T12:38:50Z

applications/nrf_desktop/src/modules/Kconfig.dfu_mcumgr

@@ -9,20 +9,16 @@ menuconfig DESKTOP_DFU_MCUMGR_ENABLE
 	select EXPERIMENTAL
 	select CAF_BLE_SMP_TRANSFER_EVENTS if MCUMGR_TRANSPORT_BT
 	select MCUMGR
-	select MCUMGR_GRP_IMG
 	select MCUMGR_GRP_OS


Do we need CONFIG_MCUMGR_GRP_OS for SUIT?

Yes because:

Allow a DFU host to retrieve information about the number and size of MCUmgr buffers.
The nRF Connect Device Manager uses this information to speed up the DFU.

CONFIG_MCUMGR_GRP_OS_MCUMGR_PARAMS=y

If it's only speed improvement (not an actual requirement) it should be enabled in *.conf file I think (or eventually it can be implied). Keep in mind that OS mgmt is required for SMP DFU on nRF52, because it enables system reboot command that is used right after finalized DFU image transfer (phone triggers reboot command automatically on finalized DFU).

MarekPieta · 2024-05-29T12:39:29Z

applications/nrf_desktop/src/modules/Kconfig.dfu_mcumgr

 	select MCUMGR_GRP_OS
 	select MCUMGR_MGMT_NOTIFICATION_HOOKS
 	select MCUMGR_GRP_IMG_MUTEX if DESKTOP_DFU_LOCK
 	select MCUMGR_SMP_COMMAND_STATUS_HOOKS
 	select NET_BUF
 	select ZCBOR
 	select CRC
-	select IMG_MANAGER
-	select MCUBOOT_BOOTUTIL_LIB
 	select STREAM_FLASH


Is stream flash used by SUIT? (please also double check the remaining Kconfig options selected as common)

applications/nrf_desktop/src/modules/dfu_mcumgr.c

kapi-no · 2024-05-29T14:25:43Z

@zycz, could you resolve Compliance issue?

applications/nrf_desktop/src/util/desktop_suit.c

applications/nrf_desktop/src/modules/dfu_mcumgr.c

applications/nrf_desktop/src/util/CMakeLists.txt

applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/prj.conf

MarekPieta · 2024-06-04T07:01:43Z

...tions/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpurad/images/ipc_radio/prj_release.conf

+
+# Allow for large Bluetooth data packets.
+CONFIG_BT_BUF_ACL_RX_SIZE=502
+CONFIG_BT_BUF_ACL_TX_SIZE=251


Shouldn't we also set CONFIG_BT_CTLR_DATA_LENGTH_MAX=251 ?

MarekPieta · 2024-06-04T07:07:58Z

applications/nrf_desktop/src/util/desktop_suit_util.h

+#ifndef _DESKTOP_SUIT_UTIL_H_
+#define _DESKTOP_SUIT_UTIL_H_
+
+int suit_get_installed_manifest_seq_num(unsigned int *seq_num);


Not sure if it's worth to use separate file for this function (it's like 3 lines of code, it may be simply copy-pasted to the DFU module if needed)

MarekPieta · 2024-06-04T07:32:26Z

applications/nrf_desktop/src/modules/dfu_mcumgr.c

 	img_mgmt_reset_upload();
+#elif defined CONFIG_DESKTOP_DFU_BACKEND_SUIT
+	suit_dfu_cleanup();


Check returned value, please.

@tomchy, do you plan to expose a suitfu API to allow application trigger the upload cleanup or we can rely on suit_dfu_cleanup here? If you are going to cache some information about the image upload, our current approach may introduce side-effect for you - that's why I decided to notify you

Check returned value, please.

module state error

The idea was to provide suit_dfu module if you want to manage the update in a more advanced way, so this usage is correct.
The SUITFU resets the internal state whenever an upload with off == 0 is received.

MarekPieta · 2024-06-04T09:53:44Z

applications/nrf_desktop/src/modules/Kconfig.dfu_mcumgr

-	depends on BOOTLOADER_MCUBOOT
+	select FLASH_MAP
+	select ZCBOR
+


Remove extra empty line, please

MarekPieta · 2024-06-04T09:56:56Z

applications/nrf_desktop/src/modules/dfu_mcumgr.c

+	if (!err) {
+		LOG_INF("SUIT sequence number: %d", seq_num);
+	} else {
+		LOG_ERR("suit_get_installed_manifest_seq_num failed (err: %d)", err);


The error log seems outdated

applications/nrf_desktop/Kconfig.sysbuild

tomchy · 2024-06-04T10:44:46Z

applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/app.overlay

@@ -52,3 +52,18 @@
 		};
 	};
 };
+
+&cpuapp_rw_partitions {
+		dfu_partition: partition@13c000 {


I'll try to remember about it in #15490 🙂

tomchy · 2024-06-04T10:51:18Z

applications/nrf_desktop/src/modules/dfu_mcumgr.c

 	img_mgmt_reset_upload();
+#elif defined CONFIG_DESKTOP_DFU_BACKEND_SUIT
+	suit_dfu_cleanup();


The idea was to provide suit_dfu module if you want to manage the update in a more advanced way, so this usage is correct.
The SUITFU resets the internal state whenever an upload with off == 0 is received.

Add SMP SUIT DFU support for nrf_desktop application. JIRA: NCSDK-26998 NCSDK-25024 Signed-off-by: Jan Zyczkowski <[email protected]>

zycz requested review from tejlmand and MarekPieta as code owners May 28, 2024 16:56

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 28, 2024

zycz requested review from kapi-no, alstrzebonski and mkapala-nordic May 28, 2024 16:57

zycz force-pushed the issue26998_dfu_new branch from 9afedc6 to c0d26ca Compare May 28, 2024 17:10

zycz removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 28, 2024

kapi-no reviewed May 29, 2024

View reviewed changes

kapi-no added this to the 2.7.0 milestone May 29, 2024

zycz force-pushed the issue26998_dfu_new branch from c0d26ca to 81cb1df Compare May 29, 2024 07:44

zycz requested a review from kapi-no May 29, 2024 07:44

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 29, 2024

zycz force-pushed the issue26998_dfu_new branch 2 times, most recently from c8ddd78 to 1ced105 Compare May 29, 2024 08:32

kapi-no reviewed May 29, 2024

View reviewed changes

applications/nrf_desktop/src/modules/Kconfig.dfu_mcumgr Outdated Show resolved Hide resolved

kapi-no reviewed May 29, 2024

View reviewed changes

applications/nrf_desktop/configuration/nrf54h20dk_nrf54h20_cpuapp/app.overlay Show resolved Hide resolved

mkapala-nordic reviewed May 29, 2024

View reviewed changes

applications/nrf_desktop/src/modules/dfu_mcumgr.c Show resolved Hide resolved

applications/nrf_desktop/src/modules/dfu_mcumgr.c Outdated Show resolved Hide resolved

MarekPieta requested changes May 29, 2024

View reviewed changes

zycz force-pushed the issue26998_dfu_new branch 4 times, most recently from 82b4b6f to 4c1ec45 Compare June 3, 2024 13:57

zycz requested review from MarekPieta, mkapala-nordic and kapi-no June 3, 2024 14:14

zycz force-pushed the issue26998_dfu_new branch from 4c1ec45 to c9afa30 Compare June 3, 2024 14:17

zycz force-pushed the issue26998_dfu_new branch from c9afa30 to 4ba4a49 Compare June 3, 2024 14:27

mkapala-nordic requested changes Jun 3, 2024

View reviewed changes

zycz force-pushed the issue26998_dfu_new branch from 4ba4a49 to 4e59645 Compare June 3, 2024 16:37

zycz requested a review from mkapala-nordic June 3, 2024 16:38

zycz force-pushed the issue26998_dfu_new branch 2 times, most recently from 5d77a95 to 877c939 Compare June 3, 2024 16:42

MarekPieta requested changes Jun 4, 2024

View reviewed changes

zycz force-pushed the issue26998_dfu_new branch 2 times, most recently from f47c2ce to 305040e Compare June 4, 2024 08:37

mkapala-nordic approved these changes Jun 4, 2024

View reviewed changes

MarekPieta approved these changes Jun 4, 2024

View reviewed changes

zycz force-pushed the issue26998_dfu_new branch from 305040e to 4bdbd18 Compare June 4, 2024 10:10

MarekPieta approved these changes Jun 4, 2024

View reviewed changes

zycz force-pushed the issue26998_dfu_new branch from 4bdbd18 to 5638a4d Compare June 4, 2024 10:14

tomchy reviewed Jun 4, 2024

View reviewed changes

applications: nrf_desktop: Add SMP SUIT DFU support for nrf54h20

3c7b671

Add SMP SUIT DFU support for nrf_desktop application. JIRA: NCSDK-26998 NCSDK-25024 Signed-off-by: Jan Zyczkowski <[email protected]>

zycz force-pushed the issue26998_dfu_new branch from 5638a4d to 3c7b671 Compare June 4, 2024 12:13

mkapala-nordic approved these changes Jun 4, 2024

View reviewed changes

tomchy approved these changes Jun 4, 2024

View reviewed changes

kapi-no removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jun 4, 2024

rlubos merged commit 96bd576 into nrfconnect:main Jun 4, 2024
15 checks passed

zycz deleted the issue26998_dfu_new branch June 4, 2024 13:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

applications: nrf_desktop: Add SMP SUIT DFU support for nrf54h20 #15553

applications: nrf_desktop: Add SMP SUIT DFU support for nrf54h20 #15553

zycz commented May 28, 2024

NordicBuilder commented May 28, 2024 •

edited

Loading

NordicBuilder commented May 28, 2024

kapi-no left a comment

MarekPieta May 29, 2024

MarekPieta May 29, 2024

zycz Jun 3, 2024 •

edited

Loading

MarekPieta Jun 3, 2024 •

edited

Loading

MarekPieta May 29, 2024

kapi-no commented May 29, 2024

MarekPieta Jun 4, 2024

MarekPieta Jun 4, 2024

MarekPieta Jun 4, 2024

MarekPieta Jun 4, 2024

kapi-no Jun 4, 2024

tomchy Jun 4, 2024

MarekPieta Jun 4, 2024

MarekPieta Jun 4, 2024

tomchy Jun 4, 2024

tomchy Jun 4, 2024

applications: nrf_desktop: Add SMP SUIT DFU support for nrf54h20 #15553

applications: nrf_desktop: Add SMP SUIT DFU support for nrf54h20 #15553

Conversation

zycz commented May 28, 2024

NordicBuilder commented May 28, 2024 • edited Loading

Test specification

NordicBuilder commented May 28, 2024

kapi-no left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zycz Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

MarekPieta Jun 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kapi-no commented May 29, 2024

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

Choose a reason for hiding this comment

NordicBuilder commented May 28, 2024 •

edited

Loading

zycz Jun 3, 2024 •

edited

Loading

MarekPieta Jun 3, 2024 •

edited

Loading