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

Char Set value: setter_callback #63

Closed
wants to merge 1 commit into from
Closed

Char Set value: setter_callback #63

wants to merge 1 commit into from

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Mar 22, 2018

I believe that the callback method shouldn't be called if a char is set to the same value it has been before. For some this might be a breaking change.

What do you think?

@ikalchev
Copy link
Owner

Hey, it's been a while!

There are some chars for which setting the same value twice makes sense, but these are more of an exception. In that case, the optimisation you are proposing makes sense, however, I would like for users to be able to disable this check. Also, note that in the current design, this same functionality can be achieved in the setter_callback - implies that the setter_callback is stateful. The last bit can be removed if callbacks also had access to the self of course. What do you think?

Can you give me more context on the reason for this request, i.e. is it performance that bothers you or is it usability, so that we can work better towards a solution?

Best,
Ivan

@cdce8p
Copy link
Contributor Author

cdce8p commented Mar 22, 2018

It was more meant as a general suggestion and to prevent potential errors that might occur if setter_callback=false isn't set.

In a broader context: I'm still looking for a good solution to handle the case when both (HomeKit and the HAP server) can update a char value and and the setter method calls an action on the server which in turn would call an action on HomeKit, so on. My current solution using flag variables for each char feels overly complicated. An example for this would be: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/homekit/type_lights.py and how a brightness change is handled in the setter_callback and the update method.

@cdce8p
Copy link
Contributor Author

cdce8p commented Apr 2, 2018

See #73 for follow up

@cdce8p cdce8p closed this Apr 2, 2018
@cdce8p cdce8p deleted the patch-2 branch April 2, 2018 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants