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

Simplify update_zone logic #39

Merged
merged 2 commits into from
Dec 2, 2024
Merged

Simplify update_zone logic #39

merged 2 commits into from
Dec 2, 2024

Conversation

webmeister
Copy link
Contributor

Avoid duplicate checks and loops and follow a simple structure:

  1. Update power state and skip all further updates if power is off, since those might not work anyway.
  2. Update generic properties that should always be supported.
  3. Update everything else that is supported by the respective zone, unless disabled via PARAM_DISABLE_AUTO_QUERY or PARAM_ENABLED_FUNCTIONS.

Avoid duplicate checks and loops and follow a simple structure:

1. Update power state and skip all further updates if power is off, since
those might not work anyway.
2. Update generic properties that should always be supported.
3. Update everything else that is supported by the respective zone, unless
disabled via PARAM_DISABLE_AUTO_QUERY or PARAM_ENABLED_FUNCTIONS.
elif (
comm == "set_channel_levels"
and "channels" in self._params.get(PARAM_ENABLED_FUNCTIONS)
and bool(self.power.get(Zones.Z1))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: this is probably a bug in the original code - it should probably check self.power.get(zone), rather than always check Zone 1 power irrespective of the zone being updated. Logged as #40

@crowbarz
Copy link
Owner

Thanks for the PR. It is definitely a lot cleaner. It seems ok, I have cherry-picked the PR and will soak it for a few days on my setup.

if (
await self.send_command("query_power", zone, ignore_error=True) is None
or await self.send_command("query_volume", zone, ignore_error=True) is None
await self.send_command("query_volume", zone, ignore_error=True) is None
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Argh, I lost my previous comment.)

Because query_volume and query_mute are not executed if the AVR zone is off, volume[zone] and mute[zone] are unset. This impacts the HA media_player entity supported_features which does not advertise volume and/or mute.

For some reason, query_volume and query_mute are not executed on my AVR when the zone is turned on, this should update the attributes checked when calculating supported_features. I need to debug this further.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So on my AVR (VSX-930, Zone 1 + HDZone) the scenario above occurs after the integration is started with all zones off, then HDZone is turned on before Zone 1. The initial full update is triggered only when Zone 1 is turned on for the first time because some commands don't work when only HDZone is turned on.

If volume and mute for each zone other than Zone 1 are not queried when the zone is off, the query_volume and query_mute will need to be queued the first time that zone is turned on, possibly after a delay.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracking of initial update of individual zones has been implemented in 7c12d70 and I am testing this PR rebased onto latest dev.

@crowbarz crowbarz merged commit b5a5043 into crowbarz:dev Dec 2, 2024
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.

2 participants