-
Notifications
You must be signed in to change notification settings - Fork 517
Use PercentSetting to set fanSpeedPercent #2425
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
base: add-fan-speed-percent-for-thermostats
Are you sure you want to change the base?
Use PercentSetting to set fanSpeedPercent #2425
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
Test Results 71 files 466 suites 0s ⏱️ Results for commit e6561f4. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against e6561f4 |
df06872 to
ae3bd7f
Compare
| local function fan_speed_percent_attr_handler(driver, device, ib, response) | ||
| local speed = 0 | ||
| if ib.data.value ~= nil then | ||
| speed = utils.clamp_value(ib.data.value, MIN_ALLOWED_PERCENT_VALUE, MAX_ALLOWED_PERCENT_VALUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably shouldn't remove the clamp, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restored in latest force push
|
|
||
| local function fan_speed_setting_attr_handler(driver, device, ib, response) | ||
| if ib.data.value == nil then return end | ||
| device:emit_event_for_endpoint(ib.endpoint_id, capabilities.fanSpeedPercent.percent(ib.data.value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want to clamp this as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in latest force push
ae3bd7f to
aec6e9f
Compare
This change adds the fanSpeedPercent capability to the thermostat-modular profile. Additionally, PercentSetting is subscribed in addition to PercentCurrent to set this capability, since it provides an accurate representation of the speed of the fan while helping avoid the following situation: (1) The fan speed is changed in the app and PercentSetting is routed to the device (2) The fan reports back a value of PercentCurrent that doesn't match PercentSetting because the speed takes a little while to change (3) The fanSpeedPercent capability jumps to the value reported by PercentCurrent PercentCurrent is still subscribed to, but its attribute handler is gated on the current fan mode being AUTO, in which case PercentSetting is NULL on the device side and PercentCurrent is used as a fallback for setting the capability.
cd97c74 to
e6561f4
Compare
PercentSetting is subscribed in addition to PercentCurrent to set
fanSpeedPercent, since it provides an accurate representation of the speed of the fan while helping avoid the following situation:(1) The fan speed is changed in the app and PercentSetting is routed to the device
(2) The fan reports back a value of PercentCurrent that doesn't match PercentSetting because the speed takes a little while to change
(3) The fanSpeedPercent capability jumps to the value reported by PercentCurrent
PercentCurrent is still subscribed to, but its attribute handler is gated on the current fan mode being AUTO, in which case PercentSetting is NULL on the device side and PercentCurrent is used as a fallback for setting the capability.