Add Roon volume hooks #102470

pavoni · 2023-10-21T15:50:54Z

Proposed change

Replacement for #99684 using Entity events rather than device triggers.

Roon via their API provide a means of hooking the volume control in their app suite and providing your own implementation of the volume control.

This can be useful in a situation where Home Assistant has control over the volume of a media player - but Roon natively does not.

Roon media players are often streamers connected to other devices.

An example would be an amplifier or similar device connected to a Roon streamer - where the amplifier has its own HA integration with control over the volume, or can respond to HA via an IR blaster.

This PR allows you to set up a volume control in the Roon apps. When this is changed - a event entity created in HA - allowing you to control the volume using HA's facilities.

Type of change

[x ] 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: Add documentation for new feature roon volume hooks home-assistant.io#29697

This screenshot shows how the volume controls are exposed in the roon app:

And this shows the volume control buttons in the roon app:

And this is an example automation:

alias: Roon Study Volume
description: ""
trigger:
  - platform: state
    entity_id:
      - event.study_roon_volume
action:
  - choose:
      - conditions:
          - condition: state
            entity_id: event.study_roon_volume
            state: volume_up
            attribute: event_type
        sequence:
          - service: remote.send_command
            data:
              num_repeats: 1
              delay_secs: 0.4
              hold_secs: 0
              device: systemline
              command: volume_up
            target:
              entity_id: remote.study
      - conditions:
          - condition: state
            entity_id: event.study_roon_volume
            attribute: event_type
            state: volume_down
        sequence:
          - service: remote.send_command
            data:
              num_repeats: 1
              delay_secs: 0.4
              hold_secs: 0
              device: systemline
              command: volume_down
            target:
              entity_id: remote.study
mode: queued

This PR requires verion 0.1.5 of the pyroon library - which contains adds the ability to de-register volume hooks.

The library diff is here:

pavoni/pyroon@0.1.4...0.1.5

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 Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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

Documentation added/updated for www.home-assistant.io

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:

I have reviewed two other open pull requests in this repository.

pavoni · 2023-10-21T16:08:05Z

@MartinHjelmare Thanks for your feedback - have refactored using event entities. I think it's a decent improment - both from a development and user experience perspective.

pavoni · 2023-10-21T16:42:08Z

@MartinHjelmare I tried removing the "OPTIONS_SCHEMA" in config_flow.py - but as far as I can see - that's the only place the boolean config variable is specified - so without it the gui doesn't display the option..

MartinHjelmare · 2023-11-04T10:31:23Z

I meant that we shouldn't have an option at all. Just always create the event entity.

homeassistant/components/roon/media_player.py

home-assistant · 2023-11-04T10:33:08Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

pavoni · 2023-11-04T16:33:30Z

Sorry to ask for help @MartinHjelmare

My WIP refactor is in the repo - I've really just reproduced the media entity creation logic - but I'm getting a strange error when I try to add the event entities.

2023-11-04 16:28:46.319 ERROR (MainThread) [homeassistant.components.event] Error adding entities for domain event with platform roon
Traceback (most recent call last):
  File "/Users/greg/development/ha/core/homeassistant/helpers/entity_platform.py", line 507, in async_add_entities
    await asyncio.gather(*tasks)
  File "/Users/greg/development/ha/core/homeassistant/helpers/entity_platform.py", line 563, in _async_add_entity
    entity.add_to_platform_start(
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'tuple' object has no attribute 'add_to_platform_start'
2023-11-04 16:28:46.334 ERROR (MainThread) [homeassistant.components.event] Error adding entities for domain event with platform roon

As far as I can see this method is related to the locking of access to entities, although I imaging this is provided by a base class.

Any suggestion on where to look. Google seems not to be my friend in this case.