net: lib: nrf_cloud_coap: Various cleanups / fixes #17300

simensrostad · 2024-09-12T11:00:15Z

Various fixes / cleanups needed for the Thingy91x Out of Box firmware.
Mostly small stuff found during testing of Full Modem FOTA.

NordicBuilder · 2024-09-12T11:23:20Z

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.

NordicBuilder · 2024-09-12T11:26:07Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 15

Inputs:

Sources:

sdk-nrf: PR head: 2c58526c6fbf311065aa818db9f7869f8f0d1ae9

more details

sdk-nrf:

PR head: 2c58526c6fbf311065aa818db9f7869f8f0d1ae9
merge base: f1e956e1d2ea0d2acac7bbfb4a7cd03a9e629e28
target head (main): 66f9b23d37ebff1e69db8a85e2b222463ca760e1
Diff

Github labels

Enabled	Name	Description
	ci-disabled	Disable the ci execution
	ci-all-test	Run all of ci, no test spec filtering will be done
	ci-force-downstream	Force execution of downstream even if twister fails
	ci-run-twister	Force run twister
	ci-run-zephyr-twister	Force run zephyr twister

List of changed files detected by CI (7)

doc
│  ├── nrf
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
include
│  ├── net
│  │  ├── nrf_cloud.h
│  │  │ nrf_cloud_fota_poll.h
modules
│  ├── memfault-firmware-sdk
│  │  │ memfault_lte_metrics.c
subsys
│  ├── net
│  │  ├── lib
│  │  │  ├── nrf_cloud
│  │  │  │  ├── Kconfig.nrf_cloud_coap
│  │  │  │  ├── coap
│  │  │  │  │  ├── src
│  │  │  │  │  │  │ nrf_cloud_coap_transport.c
│  │  │  │  ├── src
│  │  │  │  │  │ nrf_cloud_fota_poll.c

Outputs:

Toolchain

Version:
Build docker image:

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

◻️ Toolchain
◻️ Build twister
◻️ Integration tests
- ◻️ test-fw-nrfconnect-boot
- ◻️ test-fw-nrfconnect-nrf-iot_cloud
- ◻️ test-fw-nrfconnect-nrf-iot_serial_lte_modem
- ◻️ test-fw-nrfconnect-nrf-iot_thingy91
- ◻️ test-fw-nrfconnect-nrf-iot_mosh
- ◻️ test-fw-nrfconnect-nrf-iot_positioning
- ◻️ test-sdk-mcuboot
- ⚠️ test-fw-nrfconnect-fw-update

Disabled integration tests

- desktop52_verification
- doc-internal
- test_ble_nrf_config
- test-fw-nrfconnect-apps
- test-fw-nrfconnect-ble_mesh
- test-fw-nrfconnect-ble_samples
- test-fw-nrfconnect-chip
- test-fw-nrfconnect-fem
- test-fw-nrfconnect-nfc
- test-fw-nrfconnect-nrf-iot_libmodem-nrf
- test-fw-nrfconnect-nrf-iot_lwm2m
- test-fw-nrfconnect-nrf-iot_nrf_provisioning
- test-fw-nrfconnect-nrf-iot_samples
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-nrf_crypto
- test-fw-nrfconnect-proprietary_esb
- test-fw-nrfconnect-ps
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-tfm
- test-fw-nrfconnect-thread
- test-fw-nrfconnect-zigbee
- test-low-level
- test-sdk-audio
- test-sdk-dfu
- test-sdk-find-my
- test-sdk-pmic-samples
- test-sdk-sidewalk
- test-sdk-wifi

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

modules/memfault-firmware-sdk/memfault_lte_metrics.c

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c

include/net/nrf_cloud_fota_poll.h

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_common.c

include/net/nrf_cloud_fota_poll.h

include/net/nrf_cloud.h

include/net/nrf_cloud_fota_poll.h

jayteemo · 2024-09-13T16:08:30Z

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c

+		 * download when doing a full modem FOTA, which does not currently work.
+		 */
+		if (ctx_ptr->img_type == DFU_TARGET_IMAGE_TYPE_FULL_MODEM) {
+			int err = fota_download_util_image_reset(DFU_TARGET_IMAGE_TYPE_FULL_MODEM);


good idea... seems like this should be done in fota_download since it knows more about the specific failure mode and if it is possible to resume.

but if not, i believe it makes sense to do this only when evt->cause is one of these values:

/** The update is invalid and was rejected. Retry will not help. */ FOTA_DOWNLOAD_ERROR_CAUSE_INVALID_UPDATE, /** Actual firmware type does not match expected. Retry will not help. */ FOTA_DOWNLOAD_ERROR_CAUSE_TYPE_MISMATCH, /** Generic error on device side. */ FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL,

FOTA_DOWNLOAD_ERROR_CAUSE_INTERNAL seems to be a catch-all cause... it is possible there are cases when resuming might still work, but would take some investigation in fota_download.

Hmm, the error code I get when it fails is FOTA_DOWNLOAD_ERROR_CAUSE_DOWNLOAD_FAILED.
I don't think resumption works at all for full modem fota atm. Sure, resumption does happen. But the hash is wrong after the image is downloaded.

Created a ticket for this:
https://nordicsemi.atlassian.net/browse/NCSDK-29230

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_common.c

plskeggs

Looks OK.

You need a change log though.

simensrostad · 2024-09-17T21:18:26Z

Looks OK.

You need a change log though.

Will add!

Convert error callback to include general status notifications from the FOTA library as well as error events. This gives the application the opportunity to block network related operations during FOTA updates. Add image type to reboot function to allow the application to reinitialize the modem after a firmware update. Its not nessecary to reboot the device after modem FOTA. Signed-off-by: Simen S. Røstad <[email protected]>

- Use nrf modem library CFUN callback to prevent modem info calls in offline and powered off states. This is needed instead of fetching the func mode due to the modem being shutdown when a lte handler notification is received. - Ease up on the logging when modem info API are called. Its expected that they fail in certain scenarios, and logging an error in those instances is not necessary. Signed-off-by: Simen S. Røstad <[email protected]>

Add option, CONFIG_NRF_CLOUD_COAP_DISCONNECT_ON_FAILED_REQUEST, that disconnects the CoAP client on a failed request. Disabled by default. Signed-off-by: Simen S. Røstad <[email protected]>

simensrostad requested review from plskeggs, jayteemo, glarsennordic, tejlmand and rlubos as code owners September 12, 2024 11:00

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

simensrostad changed the title ~~Variousfixesfota~~ net: lib: nrf_cloud_coap: Various cleanups / fixes Sep 12, 2024

simensrostad force-pushed the variousfixesfota branch 3 times, most recently from e8f3492 to 67febe5 Compare September 12, 2024 12:10

jtguggedal reviewed Sep 12, 2024

View reviewed changes

modules/memfault-firmware-sdk/memfault_lte_metrics.c Outdated Show resolved Hide resolved

modules/memfault-firmware-sdk/memfault_lte_metrics.c Outdated Show resolved Hide resolved

jayteemo reviewed Sep 12, 2024

View reviewed changes

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_poll.c Outdated Show resolved Hide resolved

include/net/nrf_cloud_fota_poll.h Outdated Show resolved Hide resolved

jayteemo reviewed Sep 12, 2024

View reviewed changes

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_common.c Outdated Show resolved Hide resolved

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_common.c Outdated Show resolved Hide resolved

simensrostad force-pushed the variousfixesfota branch from 67febe5 to 9cbd24b Compare September 13, 2024 08:38

jonhelge approved these changes Sep 13, 2024

View reviewed changes

simensrostad force-pushed the variousfixesfota branch from 9cbd24b to 826e381 Compare September 13, 2024 13:52

jhn-nordic approved these changes Sep 13, 2024

View reviewed changes

simensrostad force-pushed the variousfixesfota branch from 826e381 to 0da62af Compare September 13, 2024 15:20

jayteemo reviewed Sep 13, 2024

View reviewed changes

simensrostad force-pushed the variousfixesfota branch 4 times, most recently from e86842a to 92c9aa3 Compare September 16, 2024 10:49

jayteemo reviewed Sep 17, 2024

View reviewed changes

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_common.c Outdated Show resolved Hide resolved

subsys/net/lib/nrf_cloud/src/nrf_cloud_fota_common.c Outdated Show resolved Hide resolved

simensrostad force-pushed the variousfixesfota branch from 92c9aa3 to d0b445e Compare September 17, 2024 08:49

jayteemo approved these changes Sep 17, 2024

View reviewed changes

plskeggs approved these changes Sep 17, 2024

View reviewed changes

glarsennordic approved these changes Sep 17, 2024

View reviewed changes

divipillai requested a review from carlescufi as a code owner September 18, 2024 12:08

github-actions bot added doc-required PR must not be merged without tech writer approval. and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Sep 18, 2024

simensrostad added 3 commits September 18, 2024 15:11

net: lib: nrf_cloud_coap: Add option to disconnect on a failed request

2c58526

Add option, CONFIG_NRF_CLOUD_COAP_DISCONNECT_ON_FAILED_REQUEST, that disconnects the CoAP client on a failed request. Disabled by default. Signed-off-by: Simen S. Røstad <[email protected]>

simensrostad force-pushed the variousfixesfota branch from c3450f1 to 2c58526 Compare September 18, 2024 13:11

divipillai approved these changes Sep 18, 2024

View reviewed changes

rlubos merged commit c1f2ff6 into nrfconnect:main Sep 19, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net: lib: nrf_cloud_coap: Various cleanups / fixes #17300

net: lib: nrf_cloud_coap: Various cleanups / fixes #17300

simensrostad commented Sep 12, 2024 •

edited

Loading

NordicBuilder commented Sep 12, 2024

NordicBuilder commented Sep 12, 2024 •

edited

Loading

jayteemo Sep 13, 2024

simensrostad Sep 16, 2024

simensrostad Sep 16, 2024

plskeggs left a comment

simensrostad commented Sep 17, 2024

net: lib: nrf_cloud_coap: Various cleanups / fixes #17300

net: lib: nrf_cloud_coap: Various cleanups / fixes #17300

Conversation

simensrostad commented Sep 12, 2024 • edited Loading

NordicBuilder commented Sep 12, 2024

NordicBuilder commented Sep 12, 2024 • edited Loading

CI Information

Inputs:

Sources:

Github labels

Outputs:

Toolchain

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

jayteemo Sep 13, 2024

Choose a reason for hiding this comment

simensrostad Sep 16, 2024

Choose a reason for hiding this comment

simensrostad Sep 16, 2024

Choose a reason for hiding this comment

plskeggs left a comment

Choose a reason for hiding this comment

simensrostad commented Sep 17, 2024

simensrostad commented Sep 12, 2024 •

edited

Loading

NordicBuilder commented Sep 12, 2024 •

edited

Loading