Initial support for zone control #42
Conversation
songpal/containers.py
Outdated
| @@ -263,7 +263,7 @@ def is_muted(self): | |||
| return self.mute == "on" | |||
|
|
|||
| def __str__(self): | |||
| s = "Volume: %s/%s" % (self.volume, self.maxVolume) | |||
| s = "Zone %s Volume: %s/%s" % (self.output[self.output.rfind("=")+1:], self.volume, self.maxVolume) | |||
There was a problem hiding this comment.
This feels potentially error-prone when it comes to devices with no zones, doesn't it?
There was a problem hiding this comment.
I was assuming that there was always at least one zone, but I guess there are maybe some devices where that isn't the case?
Ultimately, there are a couple of places that feel a little brittle like this because, as you mentioned below, Zones are not cached in the device, which they probably should be. I had thought about it, but seemed like a bigger change to the overall structure of the library.
For now, I just changed this to see if self.output contained a string that contains '=', but happy to do something more substantial.
There was a problem hiding this comment.
I'll have to think about it a bit, but maybe the proper solution is to create a default zone (with output="") and replace Input with Zone altogether, as the functionality in both of those containers is very much the same?
There was a problem hiding this comment.
What kind of features each zone has? Source and volume control, but are there more? After this PR is merged, I'm going to do a heavy cleanup on the codebase to make it easier to maintain, and at that point it maybe makes sense to create a real zone class having methods related to controlling all zone-specific functionality.
There was a problem hiding this comment.
- Input source
- Volume
- Activate (on/off)
These are different from Inputs (I think), because Inputs don't have a volume or a source associated with them (in fact, Inputs are the sources)
Inputs for Zones are a little funny because it seems that not all inputs can be activated for a zone, but this is listed on the inputs, not the zones, e.g.:
{ 'active':'inactive', 'connection':'unknown', 'iconUrl':'', 'label':'', 'meta':'meta:bd-dvd', 'outputs':[ 'extOutput:zone?zone=1', 'extOutput:zone?zone=4' ], 'title':'cast ', 'uri':'extInput:bd-dvd' },
|
I guess one question here is if there are additional functions that will be required to integrate this smoothly into Home Assistant. I don't have experience contributing to Home Assistant yet, so I'm not sure how that is going to go. This maybe gets back to @rytilahti's question about whether some of the zone logic should be cached in the device. |
See my comment above about "default" zone, I think that's the way to go and will make integrating with homeassistant easier. The current songpal platform has to be converted into a component (I have some code around, but haven't yet managed to finish the conversion), which shall then request the available zones from the device and initialize a |
|
I think I got 'em all |
|
Looks good, let's get it merged! :-) edit: I released 0.10 containing this, thanks again for your contribution! 🥇 |
No description provided.