Skip to content

Non blocking blockwise get #782

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

sam-golioth
Copy link
Contributor

This changes the golioth_coap_blockwise_get to be non-blocking, with block write callbacks executed on the coap client thread. Users are responsible for ensuring that they do not block the client thread unduly.

A second commit in this PR also reworks and simplifies the way that block retries are executed in the fw_update module. This simplifies program flow, removes a while(1) loop, and reduces the amount of state that has to be moved between the callback and the thread. One of the consequences is that we no longer report to the OTA state service when we retry a block download. I think that's acceptable and even desirable because:

  • Fundamentally, the state of the device has not changed (it is still attempting to download the package)
  • The reason for logging this is to provide some debugging visibility into connection issues, but the value of this data is pretty low because we only log it one time, and it is an overloading over the purpose of the OTA service to use it for connectivity debugging in this way
  • If we are failing to download blocks, then we are likely having connectivity issues. In this case, we are likely unable to report state to the cloud anyway (if the connection is down), and even worse, we could be taking relatively scarce connection time to report state that could be better spent on attempting to download the firmware. These issues are compounded by the work to retry state reporting, which will force us to attempt the report multiple times and also blocks the firmware.

Call the user-provided write block callback from the `on_block_rcvd()`
callback running on the coap client thread, rather than from the
loop in `golioth_blockwise_get()` running on the user thread. This
removes a memcpy, and makes block downloads non-blocking. But it
does mean that time user supplied write block callbacks now execute
on the coap client thread and care must be taken to ensure the
client thread is not unduly blocked.

Signed-off-by: Sam Friedman <sam@golioth.io>
Signed-off-by: Sam Friedman <sam@golioth.io>
@sam-golioth sam-golioth force-pushed the non_blocking_blockwise_get branch from 71ba68b to 3634d7e Compare May 1, 2025 19:04
Copy link

github-actions bot commented May 1, 2025

Visit the preview URL for this PR (updated for commit 3634d7e):

https://golioth-firmware-sdk-doxygen-dev--pr782-non-blocking-b-y80rqopl.web.app

(expires Thu, 08 May 2025 19:05:33 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: a9993e61697a3983f3479e468bcb0b616f9a0578

Copy link

github-actions bot commented May 1, 2025

Code Coverage

Code Coverage

Package Line Rate Branch Rate Health
include.golioth 75% 50%
port.linux 61% 34%
port.utils 58% 46%
port.zephyr 52% 22%
src 70% 31%
Summary 68% (2825 / 4146) 31% (1178 / 3762)

Copy link

codecov bot commented May 1, 2025

Codecov Report

Attention: Patch coverage is 46.87500% with 34 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/fw_update.c 0.00% 31 Missing ⚠️
src/coap_blockwise.c 86.36% 1 Missing and 2 partials ⚠️
Files with missing lines Coverage Δ
src/ota.c 67.60% <100.00%> (+2.45%) ⬆️
src/coap_blockwise.c 58.85% <86.36%> (-4.83%) ⬇️
src/fw_update.c 0.00% <0.00%> (ø)

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.

1 participant