Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow characteristics to indicate set value failure #75

Open
ikalchev opened this issue Mar 31, 2018 · 4 comments
Open

Allow characteristics to indicate set value failure #75

ikalchev opened this issue Mar 31, 2018 · 4 comments
Labels

Comments

@ikalchev
Copy link
Owner

This should propagate in AccessoryDriver.set-characteristics. FYI @thomaspurchas @cdce8p

@cdce8p
Copy link
Contributor

cdce8p commented Mar 31, 2018

I think we need to differentiate here. Setting a value on its on (just for HAP-python) can't fail, because HomeKit can only send valid values. However the callback could. Maybe fall down from there, through return False?

@cdce8p
Copy link
Contributor

cdce8p commented Mar 31, 2018

Didn't read the comments below #73 😅
Good that we thought of the same thing here.

@ikalchev
Copy link
Owner Author

ikalchev commented Apr 1, 2018

I should have been more concise. FYI I label “protocol” issues that implement pieces of the HAP spec.

@thomaspurchas
Copy link

To summarise from #73:

@thomaspurchas
Rather than returning True/False I think the callback should raise an exception on failure. The exception could be different depending on what went wrong.

We can then map the exceptions to the HAP protocol status values.

Using True/False feels a little bit like err values in C.

@cdce8p
I would think that True / False should be enough for our proposes, but you are right in that exceptions would allow for more options. @ikalchev What do you think?

@thomaspurchas
I would point out that True/False is not backwards compatible, and requires that users remember to return a True value on success (forgetting to return a value would result in None). This does not strike me as very pythonic.

@ikalchev
I just looked at Apple's spec - there are 11 error codes (which I need to add btw). If we use exceptions, we will force the Driver from understanding these exceptions and mapping them to error codes - in this case, why not set_value to return one of the error codes instead? What is more, returning an error code would allow "internal" errors to more "voicefully" propagate with an exception.

@thomaspurchas
There might be 11 error codes, but only some would be valid for a callback i.e. Request denied due to insufficient privileges. Unable to communicate with requested service, e.g. the power to the accessory was turned off. Resource is busy, try again. Accessory received an invalid value in a write request.

The rest are errors that should be managed by py-HAP. i.e. Operation timed out. could be thrown by setting a timeout on the callback (either via threads or asyncio).

Additionally exceptions can be nested so the following would work:

class AccessoryError(Exception):
  pass

class AccessoryBusyError(AccessoryError):
  pass

try:
  throw AccessoryBusyError
except AccessoryError as e:
  print(e)

@ikalchev
True, but let's keep in mind that tomorrow Apple may add 10 more, 4 of which would make sense (for example).

@thomaspurchas
@ikalchev still doesn't feel pythonic. We would then need a bunch of static vars somewhere to match the docs, or rely on users parsing the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants