micro-nova / AmpliPi

Whole House Audio System 🔊
https://amplipi.com
GNU General Public License v3.0
278 stars 23 forks source link

Bug: Amplifiers out of Sync with interface #285

Open bs42 opened 2 years ago

bs42 commented 2 years ago

The issue I've encountered very frequently is that the mute state gets out of sync. Zones will show muted when they are not or vice versa.

Lots of testing on this one and I've figured out how to replicate it and it's an artifact with the REST API and doing things simultaneously.

On the discourse discussion I posted my temporary REST media_player interface that I'm using to control the AmpliPi in Home Assistant while the integration is being worked on and have been using it successfully for a while now, with this one issue.

I have an automation in Home Assistant that turns off (mutes) all my zones. If I run that automation with all the media_player entries and the media_player.volume_mute like such:

service: media_player.volume_mute
data:
  is_volume_muted: true
target:
  entity_id:
    - media_player.amplipi_zone_office
    - media_player.amplipi_zone_garage
    - media_player.amplipi_zone_master_bathroom
    - media_player.amplipi_zone_master_bedroom

Not all of the zones will get muted but they will all say they are muted. In the backend of Home Assistant it's running this code:

amplipi_zone_mute:
    url: http://amplipi.home.schuetz.us/api/zones
    method: PATCH
    payload: '{ "zones": [ {{ zoneId | int | default(2) - 1 }}  ],  "update": { "mute": {{ mute | string | lower }} } }'
    headers:
      content-type: 'application/json

When passed to media_player.volume_mute as a group my assumption is that it's sending 4 individual mute REST calls all at the same exact moment.

If I change the automation to run media_player.volume_mute individually for each zone, thus causing them to not all run at the exact same moment, then it works fine.

It seems to indicate there's an issue with the REST API allowing a call while another one is running and failing to change the hardware state but changing the software state.

linknum23 commented 2 years ago

Thanks for documenting this bug. This is a symptom of known design flaw we've been meaning to fix for awhile. The planned fix was to separate the web backend from the low level controller with a command and response fifo. We may be able to patch this in the meantime with a mutex inside the controller.

As for a patch on your side: is there a way to call api/zones once with a list of all of the zones?

amplipi_zone_mute:
    url: http://amplipi.home.schuetz.us/api/zones
    method: PATCH
    payload: '{ "zones": [ {{ list_of_all_zones }}  ],  "update": { "mute": {{ mute | string | lower }} } }'
    headers:
      content-type: 'application/json
bs42 commented 2 years ago

I think it's something that would have to be implemented in the actual integration (if possible) and not something I can cause to happen using the "universal" media_player. But for the moment I've worked around it by ensuring I never call the media_player methods against multiple AmpliPi zones at the same time and instead do them sequentially.

So it's been worked around for now, I just wanted to bring the issue to your attention for a future bug fix.

Still loving the device, and even with the very minor bugs it's still 100x better than the garbage Sonos it replaced.