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

Invalid value outside constraints appear causes complete accessory failure #13

Open
mbuckaway opened this issue Nov 28, 2020 · 2 comments

Comments

@mbuckaway
Copy link

mbuckaway commented Nov 28, 2020

IDF Version: v4.2-beta1-227-gf0e87c933
HomeKit Version: current (checked out this week from git from master)

I've built a few homekit devices over the last month and had great success. The SDK is one of the easier ones to work with that I have used. So, in creating another device, I ran into an issue where the accessory would refuse to pair IF I added an ambient light sensor. Additional, if I removed the light sensor and got it to pair, enabling the light sensor against with the now pair accessory would cause the entire sensor would be reported as unresponsive in the home app. It took some effort, but I tracked the issue down to the initial value I set on the light sensor being outside the default expected range defined in esp_hap_apple_profiles/src/hap_apple_chars.c. Not sure this is an issue or it is it working as designed, but setting the value outside the expected range causes the entire accessory to be rendered useless. The problem is my light sensor will read 0 when there is no light...and I really don't want it to display 1 or 0.001. Additionally, I would expect that if the value is outside the correct range, it would continued to work or the SDK would possibly reset the value to the minimum value in this case. For testing purposes, I set the initial value to 0 which causes the pairing error. If I set the initial value to 1.0, it worked.

Not sure if this bug in the ESP Homekit SDK or this is the way the Apple Homekit works when the value is outside the range and the accessory won't work.

The quick fix to the issue was either to set the value to a number inside the range or change esp_hap_apple_profiles/src/hap_apple_chars.c to allow a different range.

The code in esp_hap_apple_profiles/src/hap_apple_chars.c around line 502 has:

 hap_char_float_set_constraints(hc, 0.0001, 100000.0, 0.0);

If I changed line to:

 hap_char_float_set_constraints(hc, 0.0, 100000.0, 0.0);

...the sensor worked correctly when I initialized it to 0.

Questions:

  • if an accessory tries to set a value outside the specified range, should it become unresponsive in the home app?
  • should the value for the light sensor be set 0.0 for a minimum? Or am I expected to make sure the value is within the range?

Reviewing the other issues, it appears similar to #4.

I attached a log of the failed pairing if that is any help.

error_ls.log

@shahpiyushv
Copy link
Collaborator

@mbuckaway this is indeed an expected behaviour and that is how the sdk and iOS side is designed.

If we internally change the value to fit within the constraints, modifying constraints of some standard characteristics becomes tricky. For example, the code below is perfectly valid

hap_char_t *light_level = hap_char_current_ambient_light_level_create(0);
hap_char_float_set_constraints(light_level, 0, 100000.0, 0.0);

In the first call, if we just change the value from 0 to 0.0001 to fit within the constraints, even if you override the constraints using the next call, the value will stay at 0.0001, which is not what you may have wanted. If we decide to adjust the value while actually reporting, it complicates the code and moreover, since that will happen asynchronously, it will not even be reported to your application code and may cause undesired behaviour.

The SDK only checks the values received from the controller, but lets the application code decide the values set through the firmware.

Now, for any characteristic, if iOS finds that the value is outside the constraints, it is considered as an error and so, the controller either discards the pairing or closes the connection (depending on when the error was received). For some characteristics, it may not even allow changing the constraints(Eg. A value of 400 for hue would be wrong even if you modify the constraints. I haven't tested this, but just a possibility).

If the Home app is working fine with the constraints changed to what you want in this particular case (starting with 0), it could be acceptable.

This is not related to #4 , as there, the JSON formatting itself was getting distorted because ESP8266 does not have floating point support enabled by default. You seem to be using ESP32.

@mbuckaway
Copy link
Author

Thanks for the quick response. Its unfortunate Apple has choose to cause an entire device to go offline when there is an "error" in a sensor read value. In my case, the light sensor does return 0 when it's night....which Apple believes is not valid.

My work around was to create a custom light sensor based on the ESP HomeKit SDK version, and set to the limit to start from 0 and set int constraints since the sensor returns an int and not a float. This works. The Home app shows 0lux at night and shows the correct value during the day.

This is different for a light or a switch where the values are limited. You can set a switch a value of 2 and it only supports 0 or 1. Sensors on the other hand are read-only devices, and return what they return.

That, I have a working device. So, the case can be closed. The issue should probably appear in a FAQ for the SDK.

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

No branches or pull requests

2 participants