Add support for favorites #37
Conversation
…Media Management menu responses.
|
Thanks for your contribution 🎉 Just a few notes on initial review:
Let's go from there and then I can re-review. |
|
Also, out of curiosity, which controller are you testing with? |
Running a MCA-88X and 2 MCA-88s along with 2 MBX-PRE and a ST-1 AM/FM tuner as Russound sources (amongst some other analog sources). I have also been testing media_player.py code updates on the HA end that implements the "Browse Media" and the Russound Media Management menu system. I've been planning on cleaning that code up and doing some pull requests once this library had all the needed feature additions. I'd like to help out in the media management support if you need coding or testing help. |
Committed in error
Rollback json field added to RussoundMessage class
Rollback json handling addition
Rollback message json addition
Clean up unneeded whitespace changes
More whitespace corrections
Whitespace
aiorussound/const.py
Outdated
| ZONE_PROPERTIES: list[str] = ["currentSource"] | ||
| ZONE_PROPERTIES: list[str] = [ | ||
| "currentSource", | ||
| "favorite[1].valid", |
There was a problem hiding this comment.
Why are we loading these values into the cache on connection? The enumerate command will typically fetch these values directly instead of grabbing them from the cache.
How we handle this is completely up for debate, just trying to understand it better.
There was a problem hiding this comment.
No compelling reason other than it would happen during startup regardless. Since this is dependent on controller type, it doesn't make sense to pre-fetch these.
|
It's looking pretty good, just a few additional changes / clarifications. I'll review again tomorrow and we should have no issue getting this merged ✅ |
noahhusby
changed the title
aiorussound/rio.py
Outdated
| @@ -232,6 +237,95 @@ async def init_sources(self) -> None: | |||
| except CommandError: | |||
| break | |||
|
|
|||
| async def enumerate_zone_favorites(self, zone: Zone) -> list[Favorite]: | |||
There was a problem hiding this comment.
To maintain the object paradigm, let's move this method to the zone class and rename it enumerate_favorites, and remove the zone parameter. It will just return the favorites for that zone.
| @@ -69,3 +69,12 @@ def get_max_zones(model: str) -> int: | |||
| if model in ("MCA-66", "MCA-C3"): | |||
| return 6 | |||
| return 1 | |||
|
|
|||
|
|
|||
| def get_max_zones_favorites(model: str) -> int: | |||
There was a problem hiding this comment.
This looks good, please add appropriate tests. Pretty identical to the method above so you can copy the test from there.
https://github.com/noahhusby/aiorussound/blob/master/tests/test_util.py
There was a problem hiding this comment.
Noticed I missed the MBX models, so I added those here and in the new util test method
…til method get_max_zones_favorites to add 2 missing controller types and created test method.
Feature favorites after Rebase
Rebase to parent with favorite additions
|
@archiconda1976 Please run |
|
@archiconda1976 Can you show an example of the media browser implementation of your changes on the Home Assistant side? I'm trying to plan ahead to account for presets and such and want to see how if there's a better way to store the state. |
Here is an example: https://github.com/archiconda1976/core/tree/feat/favorites/homeassistant/components/russound_rio for the favorite browsing and playing. I removed my media browsing code from this commit, but did not have a chance to verify it with my system tonight before pushing up. I may have time tomorrow to test it, let me know if you have any issues if you try to run it. |
|
@archiconda1976 Awesome, this helps explain the full picture. Can you add methods to the ZoneControlSurface for adding, deleting, and recalling favorites? (these are just wrapper methods for SendEvent). That way, you can do the validation on the library side (like the 1 < f_id < 32, and raise a RussoundError if it's outside the range. The less validation we have to do on the HA side the better. The other way to look at it is we want to make sure this library only send safe data to the unit even if it was used independently of HA, so all calls should validate custom info. |
|
I've added the 6 methods and also updated and did some brief testing of the HA side to use the new methods. |
Updated to add a favorites class.
Updated to allow for json responses, which is needed for Russound Media Management menu responses.