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

Fix ZHA update entity not updating installed version #109260

Conversation

TheJulianJES
Copy link
Member

@TheJulianJES TheJulianJES commented Feb 1, 2024

Proposed change

This fixes an issue where the ZHA update entity can show "unknown" for the currently installed version when there's a firmware update available.

I've also modified a test case to not plug the zigpy cache with current_file_version, but rather let ZHA get it from the "query image" command (checking the HA device registry that's updated on "query img").
That catches the issue fixed by this PR. The original test with plugging the attribute is also kept.

Background

Currently, the state for the update entity only gets set/calculated when the entity is initialized.
It looks like that if there's no installed firmware version in the HA device registry and also not in the zigpy attribute cache, this gets set to "unknown".
(Even though ZCL_INIT_ATTRS should have initialized the current_file_version attribute, but maybe network issued led to a case where it didn't at first or only did when the entity was already initialized?)

If the HA "check for updates" button is then used, an image notify command is sent to all devices and the devices respond with a command querying for an update.
That causes the OtaClientClusterHandler to send a SIGNAL_UPDATE_DEVICE.
ZHADevice/async_update_sw_build_id() picks that up and updates the firmware version in the HA device registry.

Now, if zigpy sees that there's an update available, it calls device_ota_update_available() in ZHAFirmwareUpdateEntity.
Here, this PR now also re-determines the currently installed version.

Screenshot of the issue (installed version: unknown, new version: correct):
image

Future

IMO, we could also consider zigpy (or ZHA?) directly updating the attribute cache for the current_file_version attribute when it gets a query_next_image command. (This would also be useful for quirks, as the current sw version is only inside the HA device registry.)
The ZHA update entity could then listen to a signal from attribute_updated in the OTA cluster handler and refresh the installed version of the entity when new information about that is available.

For now, this PR fixes the issue where "unknown" (or an outdated version) could be shown as the installed version IF there's an update available.

(device_ota_update_available() isn't called when there's no update available, thus the installed version not refreshed, even if it somehow changed (pairing to manufacturer hub to update, then re-pairing to HA without removing).
This would be addressed by the bigger changes I mentioned above where the entity always gets refreshed on a query image command -> updating zigpy attribute -> sending signal from OTA cluster -> updating update entity)

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@home-assistant
Copy link

home-assistant bot commented Feb 1, 2024

Hey there @dmulcahey, @Adminiuga, @puddly, mind taking a look at this pull request as it has been labeled with an integration (zha) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of zha can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign zha Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@dmulcahey dmulcahey added this to the 2024.2.0 milestone Feb 1, 2024
@frenck frenck merged commit c355dd7 into home-assistant:dev Feb 1, 2024
23 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants