Aqara Wireless Knob Switch H1#2844
Aqara Wireless Knob Switch H1#2844haedo-doo wants to merge 3 commits intoSmartThingsCommunity:mainfrom
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
Test Results 72 files 496 suites 0s ⏱️ Results for commit f251b28. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against f251b28 |
| @@ -0,0 +1,149 @@ | |||
| -- Copyright 2024 SmartThings | |||
There was a problem hiding this comment.
Fixed the copyright year. Thanks for the review!
cjswedes
left a comment
There was a problem hiding this comment.
Looks pretty good. Just a few small optimizations and a couple extra tests requested to be added.
| - name: Button | ||
| preferences: | ||
| - preferenceId: stse.knobSensitivity | ||
| explicit: true No newline at end of file |
|
|
||
| device:emit_event(capabilities.button.supportedButtonValues({ "pushed", "held", "double" }, { visibility = { displayed = false } })) | ||
| device:emit_event(capabilities.button.numberOfButtons({ value = 1 })) | ||
| if device:get_latest_state("main", capabilities.button.ID, capabilities.button.button.NAME) == nil then |
There was a problem hiding this comment.
nit: this should use the button_utils.emit_event_if_latest_state_missing(device, "main", capabilities.button, capabilities.button.button.NAME, capabilities.button.button.pushed({state_change = false})).
| local configuration = { | ||
| { | ||
| cluster = PowerConfiguration.ID, | ||
| attribute = PowerConfiguration.attributes.BatteryVoltage.ID, | ||
| minimum_interval = 30, | ||
| maximum_interval = 3600, | ||
| data_type = PowerConfiguration.attributes.BatteryVoltage.base_type, | ||
| reportable_change = 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
Move this definition into the device_init handler. This is a small improvement for memory since the configuration is only used there. You can also remove the nil check in that handler since it is guaranteed to be non nil.
| local SENSITIVITY = "stse.knobSensitivity" | ||
| local SENSITIVITY_FACTORS = {0.5, 1.0, 2.0} |
There was a problem hiding this comment.
Move these constant definitions into the rotation_monitor_per_handler handler. That is the only place they are used, so it is unnecessary to have them at the top level driver.
|
|
||
| local function do_configure(driver, device) | ||
| device:configure() | ||
| device:send(cluster_base.write_manufacturer_specific_attribute(device, |
There was a problem hiding this comment.
nit: a comment here about what this attribute write is doing would be nice to understand why it is needed.
| local sensitivity = tonumber(device.preferences[SENSITIVITY_KEY]) | ||
| local factor = SENSITIVITY_FACTORS[sensitivity] or 1.0 | ||
| local intermediate_val = raw_val * factor | ||
| local sign = (intermediate_val > 0 and 1) or (intermediate_val < 0 and -1) or 0 |
| return | ||
| elseif end_point == 0x47 then -- normal | ||
| device:emit_event(capabilities.knob.rotateAmount({value = val, unit = "%"}, {state_change = true})) | ||
| elseif end_point == 0x48 then -- press |
There was a problem hiding this comment.
indentation should be fixed
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests