Skip to content

Do not abort OTA when the image notify cannot be delivered - #1909

Open
zigpy-review-bot wants to merge 2 commits into
devfrom
zigpy-bot/ota-notify-non-fatal
Open

zigpy-review-bot wants to merge 2 commits into
devfrom
zigpy-bot/ota-notify-non-fatal

Conversation

@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Fixes #1891.

A sleepy end device often can't receive the OTA Image Notify. It only polls its parent occasionally, and the coordinator may report a missing APS ack while the parent still holds the frame (analysis in #1891 (comment)). Image Notify is optional in the ZCL OTA spec; devices are expected to query for new images on their own.

Until now, a failed Image Notify finished the OTA with FAILURE. That ended the session, so the device's own Query Next Image got NO_IMAGE_AVAILABLE and its Image Block requests got ABORT, even when the device had already started downloading during the notify retries.

Changes

  • A failed Image Notify is now logged, and the OTA waits for the device to request the image (MAX_TIME_WITHOUT_PROGRESS, 30 s). If it never does, the OTA finishes with TIMEOUT.
  • The device's first OTA request cancels a still pending Image Notify. A pending notify takes the request slot that the replies to the device need (normal priority shares a single per-device slot with low priority), so it would otherwise delay those replies.
  • The device's first OTA request also restarts the stall timer, so the OTA can't wait forever once the device has started talking. Only the first request does this, so it never shortens the 60 s final-block timeout.

Behavior change

A device that really is unreachable now ends with TIMEOUT about 30 s after the notify gives up, instead of FAILURE right away. ZHA's update entity reports "Update was not successful" either way. During those 30 s, ota_in_progress stays set, so a second update attempt or a re-interview of that device is refused until then.

Tests

New tests in tests/ota/test_ota_manager.py simulate a sleepy device whose Image Notify can't be delivered. They cover:

  • the device querying after the notify gave up (the update succeeds);
  • the device never querying (TIMEOUT);
  • the device sending its first request while the notify is still pending, either a query or (when resuming) an image block request (the notify is cancelled, its request slot is released and the update succeeds);
  • the first block response never reaching the device, which then goes quiet (TIMEOUT, no hang);
  • a slow upgrade-end response (still within the final-block timeout);
  • the update being cancelled while the notify is pending.

All new tests except the last one fail on dev; the last one guards the new notify task handling. A test fails if any of these is removed: the device-request hook in the query or block handler, the notify cancellation, the first-request stall timer restart, or the cancellation of the notify task when the update itself is cancelled.

The two existing notify-failure cases in tests/test_device.py now assert TIMEOUT instead of just "not SUCCESS".

The full suite passes, also when merged with current dev.

Not in this PR

Image notify is optional and sleepy end devices often cannot receive it,
they are expected to query for new images on their own. Previously, a
failed image notify finished the OTA with `FAILURE`, which tore down the
session and made the device's own requests get `NO_IMAGE_AVAILABLE` or
`ABORT`, even when the device had already started downloading the image
during the notify retries.

- A failed image notify is now logged and the OTA waits for the device to
  request the image, finishing with `TIMEOUT` if it never does.
- The device's first OTA request cancels a still pending image notify, so
  the notify does not delay our responses to the device.
- The device's first OTA request also restarts the stall timer, so the OTA
  can never wait forever once the device has started talking to us.
@codecov

codecov Bot commented Oct 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.49%. Comparing base (fb63170) to head (634a7d5).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1909      +/-   ##
==========================================
- Coverage   99.51%   99.49%   -0.02%     
==========================================
  Files          61       61              
  Lines       12733    13158     +425     
==========================================
+ Hits        12671    13092     +421     
- Misses         62       66       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This PR fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OTA: notify() treats Image Notify delivery failure as fatal, blocking OTA for pure sleepy end devices

1 participant