[Aqara] Cube T1 Pro #930
[Aqara] Cube T1 Pro #930
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Channel deleted. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against a5c0857 |
| delete_timer(device) | ||
| device:set_field(CUBEACTION_TIMER, device.thread:call_with_delay(CUBEACTION_TIME, callback_timer(driver, device))) |
There was a problem hiding this comment.
These two lines always appear together. I would suggest you pull all of this logic out into one function rather than defining two functions that are always called together in the same order.
| local capabilities = require "st.capabilities" | ||
| local data_types = require "st.zigbee.data_types" | ||
| local clusters = require "st.zigbee.zcl.clusters" | ||
| local PowerConfiguration = clusters.PowerConfiguration |
There was a problem hiding this comment.
unused: this is the luacheck error
There was a problem hiding this comment.
maybe you should write a configuration test
| local timer = device:get_field(CUBEACTION_TIMER) | ||
| if timer then | ||
| device.thread:cancel_timer(timer) | ||
| device:set_field(CUBEACTION_TIMER, nil, { persist = true }) |
There was a problem hiding this comment.
this doesn't need to persist
it takes more than 3s to reboot the hub anyway
| device.thread:cancel_timer(timer) | ||
| device:set_field(CUBEACTION_TIMER, nil, { persist = true }) | ||
| end | ||
| device:set_field(CUBEACTION_TIMER, device.thread:call_with_delay(CUBEACTION_TIME, callback_timer(driver, device))) |
There was a problem hiding this comment.
but isn't used in callback_timer, so it can be removed there too
There was a problem hiding this comment.
I applied the changes you requested.
|
Should the firmwareUpdate capability also be included? |
| version: 1 | ||
| - id: battery | ||
| version: 1 | ||
| - id: firmwareUpdate |
Apologies for the delay. In addition to no shared functionality with zigbee-button, it also doesn't use the button capability (although it does use the RemoteController category). I wouldn't be opposed to putting this in its own driver, similar to the Pet Feeder. Is that what you were thinking too? |
|
@tpmanley that's correct |
|
Hey @seojune79 We would like to see this device use its own driver due to the uniqueness of the device. Can you please refactor this PR to be an independent driver in the Aqara folder please? Also just so your aware I attempted this driver with a T1 Pro device on my hub and it was not responsive to the rotation of the cube. |
I have changed the folder of the cube. |
| init = device_init, | ||
| added = device_added | ||
| }, | ||
| can_handle = is_aqara_products |
There was a problem hiding this comment.
as a base driver, you do not need to supply a can_handle function
There was a problem hiding this comment.
It was removed can_handle.
add Aqara Cube T1 Pro