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

Fix for hue logging issue #64

Merged
merged 3 commits into from
Feb 17, 2020
Merged

Fix for hue logging issue #64

merged 3 commits into from
Feb 17, 2020

Conversation

shred86
Copy link
Contributor

@shred86 shred86 commented Feb 17, 2020

When changing the color of a light, Abode will sometimes return a hue value that is off by 1 resulting in a warning log message, "Set color mismatch for device...". In an attempt to reduce unnecessary logging messages (mostly to reduce extraneous logging messages in Home Assistant), this pull request makes a slight modification to the comparison of the requested and response object hue value using the math standard library. The isclose method (added in Python 3.5) is used with abs_tol set to 1. This way, if the values are ever anything greater than 1.0, the warning log message will appear, which shouldn't happen unless something is really off. If the values are equal to or less than 1.0, the warning log message will not appear. I've only seen this "issue" with the hue value, hence the reason I only made the change there.

Edit: Just spent more time than I'd like to admit on confusion with the or operator. The parenthesis around the entire if statement led me down the wrong path, then trying to format the code so the line wasn't too long confused me even more. It should be correct now.

math.isclose(response_object["hue"], int(hue), abs_tol=1) will return True if the difference between the hue values are less than or equal to 1.

So basically my logic is, if not true (i.e. the difference between the hue values are greater than 1) or the saturation values don't match, then log the warning. Clear as mud.... I need some sleep.

@coveralls
Copy link

coveralls commented Feb 17, 2020

Coverage Status

Coverage increased (+0.03%) to 85.338% when pulling 760e6fa on shred86:light_fix into c4108ec on MisterWil:master.

@MisterWil
Copy link
Owner

So it only happens occasionally, and it happens because abode is rounding or sending a value 1 off from the value we're sending? I remember writing some code that was converting 0-255 to 0-100, so it's not because HASS is rounding incorrectly/differently?

@shred86
Copy link
Contributor Author

shred86 commented Feb 17, 2020

That was on the Home Assistant side where HA sends a brightness value of 0-255 which needs to be converted to a value of 0-99 (not 100), source code here. The issue was Abode would reject a brightness value of 100 for whatever reason.

This “issue” is with the hue value. When we send a hue value, the response from that request will sometimes come back with a value that is off by 1 from what we sent, which triggers the warning log message. My only guess is Abode is doing some sort of conversion of the value on their end and it gets converted back for the response message which results in a rounding error in some cases. I knew about this when adding the support for changing colors, but just accepted it at the time. At first I was thinking we should suppress the log message but I think this solution is better - if it’s performing as expected, don’t log.

@MisterWil MisterWil merged commit d294c57 into MisterWil:master Feb 17, 2020
@MisterWil
Copy link
Owner

Cool seems good to me!

@shred86 shred86 deleted the light_fix branch February 17, 2020 18:59
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.

3 participants