Skip to content
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

Revert OTA hack #18

Merged
merged 2 commits into from
Jan 19, 2024
Merged

Revert OTA hack #18

merged 2 commits into from
Jan 19, 2024

Conversation

gminn
Copy link
Member

@gminn gminn commented Jan 9, 2024

Summary

We had a hack to get OTA working that involved trying indefinitely
to get a successful firmware upgrade. Failures would happen in the
middle of pulling the payload, seemingly connected to a context switch
where another thread started interacting with the modem.

This problem was actually caused by a bug in our FOTA code, where
the dynamic memory holding the download URL would sometimes get
freed early. The latest SDK release fixed this bug, so we are pulling in
that fix and reverting the indefinite retrying logic.

Also, add a github action that builds the app. Long overdue!
With the memfault version bumped up, the build fails due to insufficient
space. We added the self test, which caused this image size bump of
about ~1kB. Disabling the memfault shell for now until we get the option
to disable the self test (currently a feature in review).

Test Plan

OTA fails when the hack is removed but before pulling in the SDK fix:

[00:01:57.313,293] <inf> download_client: Downloaded 260096/428252 bytes (60%)
[00:01:57.416,229] <inf> app_event_manager: DATA_EVT_DATA_READY
[00:01:57.433,746] <inf> app_event_manager: DATA_EVT_DATA_SEND_BATCH
[00:01:57.434,875] <inf> app_event_manager: CLOUD_EVT_DATA_SEND_QOS
[00:01:57.718,383] <inf> download_client: Downloaded 261120/428252 bytes (60%)
[00:01:57.952,270] <err> download_client: Unexpected HTTP response: 400 bad request
[00:01:57.952,301] <err> fota_download: Download client error
[00:01:57.952,301] <inf> dfu_target_mcuboot: MCUBoot image upgrade aborted.
[00:01:57.952,392] <inf> dfu_target_mcuboot: MCUBoot image upgrade aborted.

OTA succeeds after pulling in SDK fix. Notice no error when there is a context switch.

[00:01:57.195,983] <inf> download_client: Downloaded 267264/428252 bytes (62%)
[00:01:57.283,020] <inf> app_event_manager: DATA_EVT_DATA_READY
[00:01:57.314,147] <inf> app_event_manager: DATA_EVT_DATA_SEND_BATCH
[00:01:57.315,368] <inf> app_event_manager: CLOUD_EVT_DATA_SEND_QOS
[00:01:57.616,180] <inf> download_client: Downloaded 268288/428252 bytes (62%)
[00:01:57.938,964] <inf> download_client: Downloaded 269312/428252 bytes (62%)
[00:01:58.661,651] <inf> download_client: Downloaded 270336/428252 bytes (63%)

Github action testing:

  • Failed before adding CONFIG_MEMFAULT_SHELL=n (see here)
  • Succeeded (is one of the checks in this PR) after adding

Resolves: MFLT-12841

@gminn gminn force-pushed the gminn/revert-ota-hack branch 2 times, most recently from e1aed22 to 64c033b Compare January 10, 2024 17:17
@memfault memfault deleted a comment from github-actions bot Jan 10, 2024
@gminn gminn requested a review from a team January 10, 2024 17:58
@gminn gminn marked this pull request as ready for review January 10, 2024 17:58
.github/workflows/build.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@noahp noahp left a comment

Choose a reason for hiding this comment

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

You served us well, workaround 🫡

@gminn gminn merged commit 20e58e2 into main Jan 19, 2024
3 checks passed
@gminn gminn deleted the gminn/revert-ota-hack branch January 19, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants