Add 'get_battery_info' method to the SoCo class #756
Conversation
|
I think I have a mixed S1 and S2, but I don't have any devices with battery :( In any case. Since the code reaches fairly wide, when the feature is not supported, I would be ok to merge it even without testing on S1 and then improve if anyone opens up an issue saying it doesn't work on S1 but should. There isn't much more we can do without more data or testing. What do you think @pwt ? |
I factory reset my Move and attempted to add it to my S1 system for testing. However, it looks like there's a bug in the current Sonos S1 iOS/iPadOS controller apps, meaning the process gets stuck when adding WiFi creds. Fortunately, I was able to add it back to my S2 system without any issues.
Yes, I agree. It's not as if it's a mission-critical feature :) |
pwt
force-pushed
the
battery_info
branch
2 times, most recently
from
452dead
to
7c8ede5
Compare
|
Do you want to add a software test for it or should I review as is? |
I'm not planning to add a test for this one. |
|
Note that I could test explicitly for whether the target speaker is a Move, and avoid the network call if it isn't. I chose not to do this because if additional battery-powered speakers arrive, there's a good chance that this call will work without change for these new speakers. |
This PR addresses #723.
I've only tested against a Move on my S2 system, and there are indications that this approach doesn't work with S1 systems. It would be useful if someone could test this.