hap-java / HAP-Java

Java implementation of the HomeKit Accessory Protocol
MIT License
153 stars 82 forks source link

Returning value out of range made accessory disappear #19

Closed gdombiak closed 7 years ago

gdombiak commented 8 years ago

Andy,

After some detective work I was able to narrow down the reason the accessory was disappearing. I'm still running more tests to confirm this was the reason but so far after 24 hours it is holding fine. The problem was not in the code but in the data the accessory was returning. To be more precise, AmbientLightLevelCharacteristic said that values were in the range between 0.0001 and 100000. My zwave sensor was returning 0.0 when it was dark. Seems like HK runs some background check at some moment and if an accessory is returning some invalid data (e.g. value out of range) then it is declared as "incompatible" (I guess) and is silently removed.

I added a simple check in the library to detect out of range values (for Floats) and return closest value within range. The problem is also logged like this:

2016-08-27 22:53:41.755 WARN 84874 --- [onPool-worker-5] c.b.h.c.FloatCharacteristic : Detected value out of range 0.0. Returning min value instead. Characteristic com.beowulfe.hap.impl.characteristics.light.AmbientLightLevelCharacteristic@3a6d0e82

It would be nice to also check range for IntegerCharacteristic but a bigger refactoring is needed that could break compatibility so I left that one alone.

Columbo can now take a break from this long investigation. :)

andylintner commented 8 years ago

Wow! Good find! I've had to work through a few similar issues - the error handling in iOS could be better :/

Can you squash your commits? Looks like it pulled in some old stuff.

gdombiak commented 8 years ago

Good catch. Code rebased now so only 2 commits appear now.

Thanks, Gaston

gdombiak commented 7 years ago

Hi Andy,

Fix has been rock solid for over a week now. Problem was returning a value out of range. This PR is now good to go. 😀

Thanks, Gaston

andylintner commented 7 years ago

Oh, sorry - this fell off my radar! Merging.