espressif / esp-homekit-sdk

541 stars 98 forks source link

Adds step argument to fan rotation speed api - fixes #92 #93

Closed ahharvey closed 1 year ago

ahharvey commented 1 year ago

HAP specification defines a step parameter on the rotation speed characteristic.

esp-homekit-sdk does not expose the step parameter on hap_char_rotation_speed_create.

This commit exposes the step parameter, while also maintain backward compatibility with existing implementations.

This commit fixes issue #92

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

shahpiyushv commented 1 year ago

All standard service/characteristic functions create the data with values as specified in the specs. Step value for rotation speed has a default value of 1 in the spec, which is what we set internally. Your use case is regarding overriding a default. This would be applicable for all characteristics for all 3 bounds (min/max/step) and so adding a separate API for one specific characteristic for one specific bound is not something we would include.

The recommended approach in such cases is to create the characteristic using the API hap_char_rotation_speed_create() and then use hap_char_float_set_constraints() to override the defaults.

shahpiyushv commented 1 year ago

Closing this PR as a solution has been suggested in the comment https://github.com/espressif/esp-homekit-sdk/pull/93#issuecomment-1209661254

ahharvey commented 1 year ago

thank you @shahpiyushv for this very useful guidance. it does indeed solve the problem.