pavoni
Volume control logic could be improved!
The roon api docs say:
* { "type": "db", "min": -80, "max": 0, "value": -50.5, "step": 0.5 }
* { "type": "db", "min": -80, "max": 10, "value": 4, "step": 1.0 }
* { "type": "number" "min": 0, "max": 100, "value": 80, "step": 1.0 }
* { "type": "number" "min": 1, "max": 99, "value": 65, "step": 1.0 }
* { "type": "incremental" }
The code in HA currently assumes that a number volume control is 0 to 100 and a db one is -80 to 0.
It should ask roon for the data before doing the calculation.
Probably best to add some utility conversion routines in the roon api.
Probably causing this issue: https://github.com/home-assistant/core/issues/81138#issuecomment-1295111202
Edit: sorry didn't notice this is the repo for the roon api interface!
Don't know all the code that should be changed for this but roon/media_player.py line 192 could be something like this maybe?
if volume_data["type"] == "db":
# device volume range
volume_range = volume_data["max"] - volume_data["min"]
# dB in one percent
volume_percentage = 100 / volume_range
# current dB value
volume_db_level = volume_data["value"]
# current volume in percentage of range (convert dB to positive value)
volume_percentage_level = volume_range - -volume_db_level
# volume percentage
current_volume_level_percentage = volume_percentage * volume_percentage_level
Example my case: -127dB - 0dB (range 127, current volume -56dB) (100 / 127) * (127 - 56) = 55,91%
Example: -80dB - +10dB (range 90, current volume -56dB) (100 / 80) * (80 - 56) = 37,78%
That code looks sensible but there is also a similar problem here:
https://github.com/pavoni/pyroon/blob/master/roonapi/roonapi.py#L268
What probably makes sense is to move the volume calculation from media_player into roon api and then fix both issues in the library.
If you're a developer feel free to submit a roon_api PR - it's a lot easier working on the roon_api library than home assistant., and there are some python demo examples that you could adapt to check your code.
Very happy to help if you fancy having a go.
So, tried to change te code but not really into Python. So not sure how to test...
What I would do is to modify one of the demo apps.
Perhaps something that displays the volume of a zone, then reduces the volume, and then puts it back after a delay.
But happy to help - you can just open a pull request with what you have and I can take a look.
Found out by running your demo.py that the Matrix Mini i3 reports the volume as 'type': 'number' instead of 'type': 'db'. So going to raise that issue with Roon first.
Does roon remote show db on the scale?
Regardless it would be nice to make the roonapi handle this case if we can.
Sure there will be other examples!
The missing dB scale is a bug. It doesn't show it in the remote. They passed it on to the dev-team.
What's your current thinking on this?
I'm sure we could come up with a utility function that will work with the ranges (as long as they are correct) without needing to depend on the db setting being correct.
It’s crazy that you mention this, because this morning i was thinking about it.
The example i gave above should work with every range because it converts the range to percentage.
edit:
Using the MIN and MAX volume from Roon API.
Allright, so the following code works when changing the volume from Roon it updates the HA volume accordingly.
media_player.py from line 179
try:
volume_data = player_data["volume"]
volume_muted = volume_data["is_muted"]
volume_step = convert(volume_data["step"], int, 0)
volume_max = volume_data["max"]
volume_min = volume_data["min"]
volume_current_level = volume_data["value"]
volume_range = volume_max - volume_min
volume_percentage_factor = 100 / volume_range
if volume_current_level > 0:
volume_percentage_level = volume_current_level * volume_percentage_factor
else:
volume_percentage_level = (volume_range + volume_current_level) * volume_percentage_factor
volume_level = convert(volume_percentage_level, int, 0) / 100
I tested it with a Hifi Berry card (0-100 range) and the Matrix Mini i3 (-127,5 - 0 range):
Matrix Mini i3 Pro:
Hifiberry DAC+:
The same logic should probably apply to the set_volume_level(), volume_up() and volume_down() functions for the other way round? (change from HA and update Roon)
This works for me when changing the volume from HA on both devices:
set_volume_level()
def set_volume_level(self, volume: float) -> None:
"""Send new volume_level to device."""
volume = int(volume * 100)
volume_data = self.player_data["volume"]
volume_max = volume_data["max"]
volume_min = volume_data["min"]
volume_range = volume_max - volume_min
volume_percentage_factor = volume_range / 100
percentage_volume = volume_min + volume * volume_percentage_factor
self._server.roonapi.change_volume(self.output_id, percentage_volume)
volume_up()
def volume_up(self) -> None:
"""Send new volume_level to device."""
volume_data = self.player_data["volume"]
volume_max = volume_data["max"]
volume_min = volume_data["min"]
volume_range = volume_max - volume_min
volume_percentage_factor = volume_range / 100
volume_percentage_change = convert(3 * volume_percentage_factor, int, 0)
self._server.roonapi.change_volume(self.output_id, volume_percentage_change, "relative")
volume_down()
def volume_down(self) -> None:
"""Send new volume_level to device."""
volume_data = self.player_data["volume"]
volume_max = volume_data["max"]
volume_min = volume_data["min"]
volume_range = volume_max - volume_min
volume_percentage_factor = volume_range / 100
volume_percentage_change = convert(3 * volume_percentage_factor, int, 0)
self._server.roonapi.change_volume(self.output_id, -volume_percentage_change, "relative")
Sorry, this works in the Roon component in HA... so my solution is on the wrong repository!
@doctorfree @Ramblurr
Do you have a view?
The question here is whether the 0-100 volume conversion should be in the library or outside.
My sense is to leave the current raw volume functions and add a second set - or some conversion functions.
The Roon wrapper in HA currently does this - but not always correctly for some endpoints!
My sense is to leave the current raw volume functions and add a second set - or some conversion function
This is good I think. Expose the raw control and some convenience functions
wry easy for me to steal your hard work!
Haha, please do. I'm to lazy to create a pull request!
And wouldn't label this "hard work" ;-)
- { "type": "incremental" }
Only don't know what this is, how this works. No min/max here... so an exception could be needed here to avoid errors in HA.
@juliusvaart Having looked at this more carefully, it will need to be a breaking change, because the current pyroon change_volume method does a conversion if the endpoint uses db, which we don't want with your revised logic.
So my current plan is to do a release of pyroon which will contain the new socket library, and the revised play path parsing.
I can then do an HA library update - which is straightforward as it requires no code changes to HA.
I will then look at the volume logic - and revise the library and HA wrapper.
The HA guys prefer PRs that don't contain multiple changes - since they are easier to review.