Skip to content

Reset update percentage after install#818

Draft
TheJulianJES wants to merge 3 commits into
zigpy:devfrom
TheJulianJES:tjj/reset-update-percentage-after-install
Draft

Reset update percentage after install#818
TheJulianJES wants to merge 3 commits into
zigpy:devfrom
TheJulianJES:tjj/reset-update-percentage-after-install

Conversation

@TheJulianJES

Copy link
Copy Markdown
Contributor

DRAFT.

Reset update_percentage when a firmware update finishes

The firmware update entity set in_progress back to False when an install completed, failed, or raised, but never cleared update_percentage. This left the entity reporting a stale percentage (typically 100) alongside in_progress == False — an internally inconsistent state at the library boundary.

Home Assistant's update platform happens to mask the percentage (update_percentage if in_progress else None), so an HA frontend user usually doesn't notice. But the ZHA library's own state dict and websocket contract emit the stale value directly, and relying on a downstream consumer to sanitize our output is fragile — the library should report correct state itself.

Fix

The three exit paths (exception, non-SUCCESS status, success) each duplicated self._attr_in_progress = False + maybe_emit_state_changed_event(). These are collapsed into a single finally block that also clears self._attr_update_percentage, so the progress state is always cleared regardless of the update's outcome.

This also fixes a latent bug on cancellation. asyncio.CancelledError is a BaseException, so it was never caught by except Exception, and without a finally the old code skipped cleanup entirely — leaving the entity stuck at in_progress: true / 100% indefinitely. The install call chain is unshielded end to end (HA async_install_with_progress → HA ZHA async_install → this entity → zigpy update_firmware), so a cancelled install task propagates CancelledError straight into this code. This is reachable in practice whenever the install task is cancelled while ZHA keeps running — e.g. an automation with mode: restart re-triggering, automation.reload/script.reload, or a timeout: wrapping update.install. The finally now clears the state and lets the CancelledError propagate untouched.

Tests

  • test_firmware_update_success — assert update_percentage is reset to None after a successful install (the primary regression; fails against the old code with assert 100.0 is None).
  • test_firmware_update_raises — assert the progress state is cleared after a failed install.
  • test_firmware_update_cancelled — new test: a CancelledError from update_firmware propagates untouched (not wrapped in ZHAException) and both in_progress and update_percentage are still cleared.

The update entity set `in_progress` back to `False` when an install
completed, failed, or raised, but never cleared `update_percentage`. This
left the entity reporting a stale percentage (typically 100%) with
`in_progress == False`, an inconsistent state at the library boundary.

Collapse the three duplicated reset blocks into a single `finally` that
also clears `update_percentage`, so the progress state is always cleared
regardless of the update's outcome.
Cover the regression where the update entity kept reporting a stale
progress percentage after an install finished: verify it is reset to
`None` on both successful and failed installs.
A CancelledError is a BaseException, so it bypasses the except handler
and propagates untouched, while the finally block still clears
in_progress and update_percentage. Guard both against regression.
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.29%. Comparing base (a0e3195) to head (f91bf23).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #818      +/-   ##
==========================================
- Coverage   97.29%   97.29%   -0.01%     
==========================================
  Files          55       55              
  Lines       10933    10930       -3     
==========================================
- Hits        10637    10634       -3     
  Misses        296      296              

☔ 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant