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

Use new zigpy OTA providers for ZHA #111159

Merged
merged 31 commits into from
Feb 28, 2024

Conversation

puddly
Copy link
Contributor

@puddly puddly commented Feb 22, 2024

Breaking change

The OTA file provider was recently disabled without notice because we found dangerous incompatibility issues with Tuya devices that asked for incorrect firmware. We have implemented the Z2M file provider for people who want to use the Z2M OTA repository for community-contributed OTA updates.

zha:
  zigpy:
    ota:
      # Download the index over HTTPS
      z2m_remote_index: https://raw.githubusercontent.com/Koenkk/zigbee-OTA/master/index.json

      # Or, load the index offline
      z2m_local_index: /path/to/index.json

Updates from plain .ota files are not recommended and risky: OTA files do not contain enough metadata to make sure that they are always applied to the correct device. If you understand the risks and want to use OTA files that aren't present in the Z2M OTA index, you will need to change your configuration:

      allow_advanced_ota_dir: I understand I can *destroy* my devices by enabling OTA
        updates from files. Some OTA updates can be mistakenly applied to the
        wrong device, breaking it. I am consciously using this at my own risk.

      advanced_ota_dir: /path/to/ota/files

Proposed change

Update ZHA's update platform to utilize the new zigpy OTA system implemented in zigpy/zigpy#1340.

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

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

Hey there @dmulcahey, @Adminiuga, @TheJulianJES, 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.

@TheJulianJES
Copy link
Member

TheJulianJES commented Feb 25, 2024

EDIT: These following issues should be fixed with this PR too now:

(now fixed) issues (click to expand)

0 — Note:

The currently installed file version is (redundantly) stored in three places at the moment:

  • current_file_version zigpy attribute cache (updated on query_next_image OR when the attribute is read)
  • HA device registry (updated only on query_next_image)
  • update entity: _attr_installed_version (only updated from zigpy attr cache on "HA restart" atm AND when an OTA image is installed, it's updated to the header version — the OTA header might differ from the installed version though)

To get to the actual "issues". Again, they're all minor things and no blockers IMO, but some things we should keep in mind to possibly improve in the future:

1:

_attr_installed_version for the update entity isn't always updated when the zigpy attribute cache for current_file_version is updated.
This can be noticed if a device is temporarily paired to a manufacturer hub for updating it (without removing the device from ZHA) and then paired back to ZHA. It's clear that the attr cache will stay outdated until zigpy eventually checks for OTA updates (causing the device to send a query_next_image command with the updated currently installed version).
The attribute cache (and HA device registry) always update on that command, but the update entity only updates on a device_ota_update_available event from zigpy when there's actually an OTA image available.

A solution for this would be to have the ZHA OTA Cluster handler send a signal/event in attribute_updated and then have the update entities listen for attribute updates on their OTA cluster -> update the _attr_installed_version attribute with the new current_file_version and write state.

We could also override the installed_version method/property to always read from zigpy cache. But we still need to listen to attribute updates to (only) write state then.
We would also need to re-read current_file_version after an update then (to have that attribute update set the _attr_installed_version to the now newly installed version).

2:

There are some images (one from Hue comes to mind) where the "header img file version" is off by one compared to the current_file_version when the image is actually installed. (file version in OTA header is one smaller/older, so at least doesn't make ZHA show the update again in this case)

After a successful update, we set the _attr_installed_version to the _latest_firmware attribute. But it's not guaranteed that the header file version is the same as the one that was actually installed.

Ideally, we would also force read current_file_version (or send an "image notify" command to get a "query img" command), as soon as the device is back available again (and then should have the "update entity" update correctly based on that "current file version" attribute update / so "issue 1" mentioned above).

... but if "issue 1" above is fixed, then ZHA/zigpy would eventually set the correct "installed version" (even for the update entity), since zigpy now queries all devices regularly.
IMO, we should still try to get the actually installed version instead of assuming the version from the OTA header matches what the installed firmware reports for current_file_version.

3:

With this PR now (different from the first "update entity implementation", as that also restored update entity state):
If an update is installed, _attr_installed_version is set to the latest one, but the zigpy attr cache (and HA device registry) are not updated until there's a "query img command" (or current_file_version is re-read).

So, if ZHA is then restarted, the update entity will prompt the update again, since a restart now sets _attr_installed_version back to the value from zigpy attr cache. (which might be outdated until the next query cmd)

... or is there now some kind of logic in zigpy (with the OTA v2 PR) that already re-reads current_file_version (or issues a "image notify" cmd causing a "query img" command) after an update?

@puddly puddly marked this pull request as ready for review February 28, 2024 19:17
@balloob balloob merged commit 4ec75d6 into home-assistant:dev Feb 28, 2024
49 checks passed
@edenhaus edenhaus added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Feb 28, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bugfix by-code-owner cla-signed dependency integration: zha noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) Quality Scale: No score
Projects
None yet
6 participants