samples: cellular: http_update: modem DFU samples refactoring #11036

eivindj-nordic · 2023-05-04T11:45:44Z

With changes to the modem firmware and integration it is not necessary to reboot the application to perform a modem firmware
update. This PR refactors the delta and full modem firmware update samples to only reinitialize the modem and modem library when performing the update.

NordicBuilder · 2023-05-04T12:24:33Z

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.

eivindj-nordic · 2023-06-02T10:46:11Z

@richardmccrae-nordicsemi This PR changes the name of the http_update/full_modem_update folder to http_update/modem_full_update, to align with modem_delta_update.

NordicBuilder · 2023-06-02T10:49:08Z

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-ci-nrfconnect-boot-fw-update	X
test-fw-nrfconnect-chip	X
test-fw-nrfconnect-nrf-iot_samples	X
test-fw-nrfconnect-nrf-iot_serial_lte_modem	X
test-fw-nrfconnect-nrf-iot_zephyr_lwm2m	X
test-fw-nrfconnect-zigbee	X
test-sdk-find-my	X
test-sdk-sidewalk	X

Detailed information of selected test modules

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

lemrey

Have left some comments, please check if they apply to more than one main.c file.

lemrey · 2023-06-05T13:00:43Z

samples/nrf9160/http_update/modem_delta_update/src/main.c

+ while (1) {
+ k_sleep(K_FOREVER);
+ }


Can't we just return?

lemrey · 2023-06-05T13:01:10Z

samples/nrf9160/http_update/modem_delta_update/src/main.c

 if (err != 0) {
- printk("fota_download_init() failed, err %d\n", err);
 return 0;


return err ?

Actually, I am wondering if this is the correct one and main should return 0 in all cases.

In theory main() is just like any other function, so we can return zero on success and non-zero on error.

samples/nrf9160/http_update/modem_delta_update/src/main.c

lemrey · 2023-06-06T07:31:22Z

samples/nrf9160/http_update/modem_delta_update/src/main.c

+
+ printk("LTE Link Connecting ...\n");
+ err = lte_lc_init_and_connect_async(lte_lc_handler);
+ __ASSERT(err == 0, "LTE link could not be established.");


We could clean this up, and do proper error checking instead (above too)

samples/nrf9160/http_update/modem_delta_update/src/main.c

samples/nrf9160/http_update/modem_full_update/src/main.c

samples/nrf9160/http_update/application_update/src/main.c

samples/nrf9160/http_update/modem_full_update/src/main.c

richardmccrae-nordicsemi · 2023-06-09T10:14:09Z

HTTP Update automated samples passing with this PR

richardmccrae-nordicsemi · 2023-06-09T10:57:56Z

samples/nrf9160/http_update/application_update/src/main.c

+ printk("LTE link could not be established.");
+ return err;
+ }
+ printk("LTE Link Connected!\n");


Compiler warning:

../src/main.c: In function 'modem_configure_and_connect':
../src/main.c:280:1: warning: control reaches end of non-void function [-Wreturn-type]
280 | }

Same goes for function in modem_full_update/src/main.c:392

Yes, I will push an update shortly.

samples/nrf9160/http_update/modem_delta_update/src/main.c

richardmccrae-nordicsemi · 2023-06-09T11:03:10Z

samples/nrf9160/http_update/application_update/src/main.c

+ printk("LTE link could not be established.");
+ return err;
+ }
+ printk("LTE Link Connected!\n");


Same goes for function in modem_full_update/src/main.c:392

NordicBuilder · 2023-09-29T08:46:49Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
nrfxlib	nrfconnect/sdk-nrfxlib@`741729d` (main)	nrfconnect/sdk-nrfxlib#1079	nrfconnect/sdk-nrfxlib#1079/files
zephyr	nrfconnect/sdk-zephyr@`b9253f6`	nrfconnect/sdk-zephyr#1316	nrfconnect/sdk-zephyr#1316/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

eivindj-nordic · 2023-10-10T10:11:39Z

Don't know why the DNM label is not removed, as the manifest change is removed.

divipillai

some minor comments.

samples/cellular/http_update/modem_full_update/README.rst

samples/cellular/http_update/application_update/README.rst

samples/cellular/http_update/modem_full_update/README.rst

anhmolt

LGTM

samples/cellular/http_update/modem_full_update/README.rst

* With changes to the modem firmware and integration it is not necessary to reboot the application to perform a modem firmware update. This commit refactors the full and delta modem firmware update samples to only reinitialize the modem and modem library when performing the update. * With changes to full and delta modem firmware update samples, this was the only sample using the common folder so this is merged into the application. Restructure the sample to align with the modem firmware update samples. Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

eivindj-nordic self-assigned this May 4, 2023

eivindj-nordic requested review from hakonfam, sigvartmh, rlubos and lemrey as code owners May 4, 2023 11:45

eivindj-nordic marked this pull request as draft May 4, 2023 11:45

github-actions bot added changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval. labels May 4, 2023

eivindj-nordic force-pushed the 21120_modem_fota_update branch from 21b81e3 to 104f88b Compare May 4, 2023 12:34

eivindj-nordic force-pushed the 21120_modem_fota_update branch from 104f88b to aadc31c Compare May 12, 2023 12:46

eivindj-nordic force-pushed the 21120_modem_fota_update branch from aadc31c to d79b969 Compare May 23, 2023 11:51

eivindj-nordic force-pushed the 21120_modem_fota_update branch from d79b969 to 2b486a5 Compare June 2, 2023 10:44

eivindj-nordic marked this pull request as ready for review June 2, 2023 10:44

eivindj-nordic force-pushed the 21120_modem_fota_update branch from 2b486a5 to 54dc31a Compare June 2, 2023 10:44

eivindj-nordic requested a review from richardmccrae-nordicsemi June 2, 2023 10:46

eivindj-nordic requested a review from MirkoCovizzi June 2, 2023 10:47

eivindj-nordic force-pushed the 21120_modem_fota_update branch 2 times, most recently from 76bb882 to fe13a4f Compare June 6, 2023 06:02

lemrey reviewed Jun 6, 2023

View reviewed changes

eivindj-nordic force-pushed the 21120_modem_fota_update branch 3 times, most recently from 0bcc062 to be28814 Compare June 9, 2023 08:43

lemrey reviewed Jun 9, 2023

View reviewed changes

richardmccrae-nordicsemi reviewed Jun 9, 2023

View reviewed changes

eivindj-nordic force-pushed the 21120_modem_fota_update branch from be28814 to e8e4f3b Compare June 9, 2023 11:16

richardmccrae-nordicsemi suggested changes Jun 9, 2023

View reviewed changes

NordicBuilder added manifest-zephyr manifest-nrfxlib DNM labels Sep 29, 2023

eivindj-nordic added this to the 2.5.0 milestone Oct 6, 2023

eivindj-nordic force-pushed the 21120_modem_fota_update branch from 2b93f27 to d706ed0 Compare October 9, 2023 07:24

github-actions bot removed the manifest label Oct 9, 2023

eivindj-nordic marked this pull request as ready for review October 9, 2023 08:36

eivindj-nordic requested a review from carlescufi as a code owner October 9, 2023 08:36

eivindj-nordic requested review from lemrey and anhmolt October 9, 2023 14:52

divipillai reviewed Oct 10, 2023

View reviewed changes

samples/cellular/http_update/modem_full_update/README.rst Outdated Show resolved Hide resolved

samples/cellular/http_update/application_update/README.rst Show resolved Hide resolved

divipillai reviewed Oct 10, 2023

View reviewed changes

samples/cellular/http_update/modem_full_update/README.rst Outdated Show resolved Hide resolved

eivindj-nordic force-pushed the 21120_modem_fota_update branch 4 times, most recently from c93431f to 0145136 Compare October 10, 2023 14:11

anhmolt approved these changes Oct 10, 2023

View reviewed changes

divipillai approved these changes Oct 11, 2023

View reviewed changes

samples/cellular/http_update/modem_full_update/README.rst Outdated Show resolved Hide resolved

eivindj-nordic force-pushed the 21120_modem_fota_update branch 3 times, most recently from 030824c to 06ee2c0 Compare October 12, 2023 06:39

eivindj-nordic force-pushed the 21120_modem_fota_update branch from 06ee2c0 to 90eada8 Compare October 12, 2023 07:00

rlubos removed DNM manifest-zephyr manifest-nrfxlib labels Oct 12, 2023

rlubos merged commit 5cd963a into nrfconnect:main Oct 12, 2023
21 of 24 checks passed

eivindj-nordic mentioned this pull request Jan 4, 2024

subsys: dfu: dfu_target: Do not clear configured flag #13537

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

samples: cellular: http_update: modem DFU samples refactoring #11036

samples: cellular: http_update: modem DFU samples refactoring #11036

eivindj-nordic commented May 4, 2023 •

edited

Loading

NordicBuilder commented May 4, 2023

eivindj-nordic commented Jun 2, 2023 •

edited

Loading

NordicBuilder commented Jun 2, 2023 •

edited

Loading

lemrey left a comment

lemrey Jun 5, 2023

lemrey Jun 5, 2023

eivindj-nordic Jun 8, 2023

lemrey Jun 9, 2023

lemrey Jun 6, 2023

richardmccrae-nordicsemi commented Jun 9, 2023

richardmccrae-nordicsemi Jun 9, 2023

richardmccrae-nordicsemi Jun 9, 2023

eivindj-nordic Jun 9, 2023

richardmccrae-nordicsemi Jun 9, 2023

NordicBuilder commented Sep 29, 2023

eivindj-nordic commented Oct 10, 2023

divipillai left a comment

anhmolt left a comment

samples: cellular: http_update: modem DFU samples refactoring #11036

samples: cellular: http_update: modem DFU samples refactoring #11036

Conversation

eivindj-nordic commented May 4, 2023 • edited Loading

NordicBuilder commented May 4, 2023

eivindj-nordic commented Jun 2, 2023 • edited Loading

NordicBuilder commented Jun 2, 2023 • edited Loading

Test specification

lemrey left a 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

richardmccrae-nordicsemi commented Jun 9, 2023

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 Sep 29, 2023

eivindj-nordic commented Oct 10, 2023

divipillai left a comment

Choose a reason for hiding this comment

anhmolt left a comment

Choose a reason for hiding this comment

eivindj-nordic commented May 4, 2023 •

edited

Loading

eivindj-nordic commented Jun 2, 2023 •

edited

Loading

NordicBuilder commented Jun 2, 2023 •

edited

Loading