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
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.
| end | ||
| ) | ||
|
|
||
| test.register_coroutine_test( |
There was a problem hiding this comment.
Please add a few more tests for the rotation_monitor_per_handler handler. We should have some edge case rotate amounts tested as well as both the heldRotateAmount and the rotateAmount events.
There was a problem hiding this comment.
Thank you for the detailed feedback! I have updated the code and test cases to address your suggestions.
3 participants
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests