search

bazwilliams / openhomedevice

Python library to access services on Linn Ds and other Openhome devices
MIT License
7 stars 3 forks source link

Added guard for volume service not existing #15

Closed bazwilliams closed 5 years ago

bazwilliams commented 5 years ago

Guard against volume control on systems where the volume service is not available.

Fixes #14

Will also need to publish updated library once merged and update the Home Assistant integration.

von-Chaps commented 5 years ago

Change looks good and is pretty much what I did to check my understanding before raising the issue. The only comment I would make is stylistic. I am not a Python expert, but would have expected;

if service is None:
    return

rather than

if service is None:
    return None

for the methods that don't ever return anything, but that's just me being pedantic! Tests should still pass I think.

von-Chaps commented 5 years ago

Slightly off topic, and I know IsMuted is already protected, but I wonder if it should return False rather than None if volume is not enabled...hmmm, your call!

bazwilliams commented 5 years ago

Thanks for the comments; you've convinced me with the return rather than return None. I'm not so sure about the isMuted() returning False. I think this should operate in a similar fashion to VolumeLevel() where reporting a volume would not be appropriate.