Repository navigation
WWST: add Zemismart KES606 four-gang relay/scene switch - #3239
zemismart-dev wants to merge 9 commits into
Conversation
|
|
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Profile category check: ✅ Passed - all profiles have a category defined. |
|
Invitation URL: |
|
zigbee-switch_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against ee1aa8d |
cjswedes
left a comment
cjswedes
left a comment
There was a problem hiding this comment.
A refresh-only parent with four independently switchable relay/scene children. Both the existing implementation and candidate tests pass the child automation-presentation checks, while the parent fails the generic requirement for an automation action or condition. We are requesting review of that design; no exception approval is assumed.
I am not sure what to do about this; it may need an exception. My only idea is that the parent could have a switch and button capability on its main component:
- button is pressed whenever a child device button is pressed (this is inline with functionality of other zigbee button devices)
- switch controls all child devices that are in the non-scene mode, and if 1 child is on then the parent is on, otherwise it is off.
| if device == nil then return false end | ||
| local parent = device | ||
| if device.network_type == device_lib.NETWORK_TYPE_CHILD then | ||
| local gang = tonumber((device.parent_assigned_child_key or ""):match("^gang([1-4])$")) | ||
| if gang == nil then return false end | ||
| parent = device:get_parent_device() | ||
| elseif device.network_type ~= device_lib.NETWORK_TYPE_ZIGBEE then | ||
| return false | ||
| end | ||
| if parent ~= nil and parent:get_manufacturer() == "_TZ3000_hurauima" | ||
| and parent:get_model() == "TS0726" then | ||
| return true, require("zemismart-kes606") | ||
| end |
There was a problem hiding this comment.
this can_handle is quite confusing:
- The
get_manufacturerandget_modelcommands when called on a zigbee child device are overwritten to lookup the mfr/model on the parent device. - The checking of if the parent_assigned_child_key parses correctly should not be done in this function. If that fails here, there are no other subdrivers/handlers where it will succeed, so we should just let it fail in this subdriver so that any errors (which shouldnt happen) are actually associated with the code that created the child devices
| if device == nil then return false end | |
| local parent = device | |
| if device.network_type == device_lib.NETWORK_TYPE_CHILD then | |
| local gang = tonumber((device.parent_assigned_child_key or ""):match("^gang([1-4])$")) | |
| if gang == nil then return false end | |
| parent = device:get_parent_device() | |
| elseif device.network_type ~= device_lib.NETWORK_TYPE_ZIGBEE then | |
| return false | |
| end | |
| if parent ~= nil and parent:get_manufacturer() == "_TZ3000_hurauima" | |
| and parent:get_model() == "TS0726" then | |
| return true, require("zemismart-kes606") | |
| end | |
| if device ~= nil and device:get_manufacturer() == "_TZ3000_hurauima" | |
| and device:get_model() == "TS0726" then | |
| return true, require("zemismart-kes606") | |
| end | |
| return false |
There was a problem hiding this comment.
Applied in 09a4655. The KES606 matcher now uses the exact manufacturer/model check you suggested, relying on the SDK's child-to-parent lookup. Child-key validation no longer affects subdriver selection; malformed keys remain within the KES606 handler and are rejected by its gang-specific command/preference guards. Added regression coverage for those cases.
| local function onoff_report(_, device, value, zb_rx) | ||
| local gang = endpoint_from_rx(zb_rx) | ||
| local parent = parent_from_device(device) | ||
| if gang == nil or parent == nil or gang_mode(parent, gang) == "scene" then return end |
There was a problem hiding this comment.
Can you clarify why gang_mode (the persisted field) and child:supports_capability are both consulted in some paths (onoff_report) but only the field is consulted in others (relay_command)?
There was a problem hiding this comment.
The two checks serve different purposes. Within gang_mode, child capabilities are only a fallback when no tracked mode is available. The tracked mode is updated from device readback and governs whether an endpoint is eligible for relay control or scene handling. Pending transitions suppress routing until D020 confirms the actual mode.
Handler-level capability checks separately prevent emitting an event into an incompatible child profile while its cloud update is asynchronous. Relay commands therefore use the confirmed mode/pending guard; child switch-event emission additionally requires a switch-capable profile. A valid relay report still updates the parent cache during a profile transition, and profile completion triggers another read. I added a source comment to make the fallback distinction explicit.
| categories: | ||
| - name: RemoteController | ||
| preferences: | ||
| - title: Indicator Light |
There was a problem hiding this comment.
Regarding your review request:
The embedded preference definitions, or the approved shared preferences that should replace them without changing their meaning.
We would prefer you use the shared preferences where possible
There was a problem hiding this comment.
I audited the current standard, stse and certifiedpreferences catalogs (90 definitions). The three-state power-on and indicator settings and the 0–43200-second toggle countdown do not have equivalent shared definitions. For example, restorePowerState is Boolean; powerOffMemory adds unsupported reverse behavior; turnOffDelay lacks zero cancellation, the full range and delayed-on behavior.
stse.changeToWirelessSwitch matches the basic mode function, but replacing our existing switchMode enum also changes the saved key/type and introduces a false default for existing Scene children. I did not find a documented alias or supported per-device preference-value migration in the current profile/SDK interfaces. Ignoring the new default would leave the displayed setting inconsistent; treating it as an ordinary change risks switching a Scene gang back to relay mode.
I have therefore retained the inline definitions and saved settings in this revision. Could you advise whether there is a preferred compatible shared enum or supported migration mechanism for this case? The catalog/schema audit is complete, but I am not claiming a shared-preference conversion.
There was a problem hiding this comment.
switchMode should be replaced with stse.changeToWirelessSwitch. The default is false which lines up with relay mode, which is what children start with currently.
|
Thanks @cjswedes. The parent switch/button suggestion was implemented in 192595d, and the physical failed-case retests are complete:
These are targeted operational reports with documented exclusions, not a new Full Integration Test. A separate mixed-mode regression confirmed master commands skip Scene gangs and aggregate state follows eligible relay reports. All four gangs were restored to Switch; the final parent on/off regression and operator confirmation ended with all loads off. The submitted certification report's notes now reference the new evidence. I have also pushed 09a4655 for the matcher/nested-dispatch feedback and replied to all four inline comments. Focused API 19/API 21 suites each pass 140/140; lint and independent review passed. Full local zigbee-switch runs have the same pre-existing failing-case sets as the clean 192595d baseline; they are not reported as all-green. This new routing revision is locally validated only; the physical reports above remain associated with 192595d and package 2026-10-13T07:18:46.468510907. The shared-preference comment has a compatibility explanation and a request for guidance on safe mode-preference migration. No certification approval is assumed. |
|
Follow-up on the October 15 CI results: firmware 59, 60, 61 and dev passed, while 57 and 58 reported six failures. I reproduced those same six failures with the official API 14/15 runtimes: the tests expected v2 module-path dispatcher names, but the legacy loader retains each template's Fixed in 5fd6d98. Only the two test files changed. The assertions now use the appropriate loader naming convention; no tests are skipped, and the topology, lazy-loading, generic-Tuya isolation and protocol checks are preserved. No production driver or profile changes are included. Validation: API 14, 15, 19 and 21 each pass all 140 focused tests; lint and independent review pass. For the legacy SDK archives, a separate fallback directory supplies only two missing generated capability definitions from official API 21 ( The new upstream CI run is pending. The October 15 routing revision still requires physical Hub validation; the October 13 hardware reports remain evidence for 192595d, not this revision. The shared-preference migration question in the inline review remains open. |
cjswedes
left a comment
cjswedes
left a comment
There was a problem hiding this comment.
There is more I could review due to all the extra complexity in this PR; please simplify this implementation. Your AI really overcomplicated this, and I wont approve in its current state.
| local function child_gang(device) | ||
| if device.network_type ~= device_lib.NETWORK_TYPE_CHILD then return nil end | ||
| return tonumber((device.parent_assigned_child_key or ""):match("^gang([1-4])$")) | ||
| end | ||
|
|
||
| local function get_child(parent, gang) | ||
| return parent:get_child_by_parent_assigned_key(string.format("gang%d", gang)) | ||
| end |
There was a problem hiding this comment.
Please use the driver's existing child key convention (%02X endpoint) and device:set_find_child(require 'lifecycle_handlers.find_child'). The default OnOff attribute and DefaultResponse handlers already route to children by endpoint, and child commands already target the right endpoint. That removes most of the manual endpoint plumbing and the no-op DefaultResponse handler. Override only what really differs: the parent aggregate event emission, scene presses, and mode changes.
There was a problem hiding this comment.
Updated in 8bda664. Children now use 01–04 and the stock lifecycle_handlers.find_child mapping. Child on/off commands, OnOff attributes and DefaultResponses delegate to the SDK switch handlers; the wrappers only check mode eligibility and update the parent aggregate. The empty DefaultResponse handler and manual child endpoint addressing are removed. Successful On/Off acknowledgements now follow the standard SDK switch-state behavior. The wrappers pass false for native registration so subsequent messages cannot bypass the Lua parent aggregation.
| local gang = endpoint_from_rx(zb_rx) | ||
| local parent = parent_from_device(device) | ||
| if gang ~= nil and parent ~= nil then | ||
| parent:set_field(string.format("relay_status_gang_%d", gang), value.value, { persist = true }) |
There was a problem hiding this comment.
The relay_status_gang_N, light_mode, relay_status, and countdown_gang_N` fields are written but never read. Please remove the three handlers, their configure_reporting/read calls, and the countdown_gang_%d field, unless you have a real consumer for them.
There was a problem hiding this comment.
Removed in 8bda664: the three unused attribute handlers, relay_status_gang_N, light_mode, relay_status, countdown_gang_N, and their unused reads/reporting configuration. Indicator, global/per-gang power-on and countdown preference writes remain. D020 is the only additional configured attribute used by the mode logic.
There was a problem hiding this comment.
Please remove the ts0601- from the profile names to make them generic.
There was a problem hiding this comment.
Renamed in 8bda664 to scene-switch-4-gang, scene-switch-child and scene-switch-child-scene, including the filenames, profile name fields, fingerprint and test references. Capability and preference definitions are otherwise unchanged.
| return mode == "scene" and SCENE_PROFILE or SWITCH_PROFILE | ||
| end | ||
|
|
||
| local function ensure_parent_profile(parent) |
There was a problem hiding this comment.
No devices in the field use this fingerprint yet, so profile migration isn't needed (it would only ever be used by your test devices). Please remove ensure_parent_profile, profile_has_capability, the old-profile branch in info_changed, and the legacy-parent tests.
There was a problem hiding this comment.
Removed in 8bda664: ensure_parent_profile, profile_has_capability, the legacy-parent infoChanged branch and both legacy-parent tests. The development sample will be cleanly re-paired for hardware validation of the new names/child keys. The remaining child profile-completion read is for normal Switch/Scene operation, not migration of an old parent profile.
| initialize_button(device) | ||
| end | ||
|
|
||
| local function do_configure(driver, device) |
There was a problem hiding this comment.
This doConfigure duplicates device:configure()/device:refresh() (which cover all endpoints) and tuya-multi's magic spell. Add the D020 mode attribute as a configured attribute in device_init and let the inherited doConfigure and refresh handle it.
There was a problem hiding this comment.
D020 is now added once as a configured attribute in parent init. The KES606-specific doConfigure, refresh, magic-packet builder and manual bind/report/read loops have been removed in 8bda664. The inherited Tuya doConfigure and SDK refresh/configure handle the endpoint traversal. I added a child early return to the Tuya handler because a dispatched Edge-child doConfigure would otherwise send refresh/magic traffic through that child; generic Tuya parent behavior remains covered by the existing tests.
There was a problem hiding this comment.
Can mode be a single source of truth? The preference writes D020, the D020 report switches the child profile, and the aggregate uses the children's latest switch state. Using persisted pending state plus a mode field plus a capability fallback plus cached relay state is a lot of machinery. What failure did you see on hardware that required it?
There was a problem hiding this comment.
I did not establish a hardware failure that justified all of those layers. The recorded cloud-state stall cleared after a Hub restart, but its cause was not isolated; it was not evidence that the full pending/cache machinery was necessary.
Simplified in 8bda664: only the persisted D020-confirmed mode remains. Preferences write/read D020, reports derive the child profile, and aggregation uses the children's SDK latest switch states. The pending state, capability fallback, independent relay cache and aggregate cache are removed. This deliberately uses last-known state semantics, including standard successful acknowledgements, rather than claiming transition-time freshness. Reads on return to Switch/profile completion update that state.
A small profile synchronization helper handles two asynchronous ordering cases: a child appearing after its D020 report, and an old profile update arriving after a newer D020 report. Both have focused regressions and need no extra state machine. This source revision still needs clean-pair physical validation; the existing October13 reports are not presented as validation of it.
|
Simplified in 8bda664 and replied to all six inline comments. The dedicated sub-driver is now 284 lines instead of 484, with standard child routing/handlers, inherited configuration/refresh and one D020-confirmed mode source. Unused fields/read/report handlers and legacy-parent migration are removed; profiles use generic names. Official SDK API 14/15/19/21 each pass all 82 focused tests. Lint, independent review and build-only packaging pass; no SDK runtime patches were used. The PR description now reflects the final implementation and its last-known aggregate-state semantics. The new revision has not been installed on the physical Hub. Clean-pair hardware validation is still required, and the October13 reports remain tied to their original commit/package. Upstream Driver Tests for this exact head all passed: 57: 4037 passed, 0 failed, 58: 4834 passed, 0 failed, 59: 5213 passed, 0 failed, 60: 5310 passed, 0 failed, 61: 5326 passed, 0 failed, dev: 5330 passed, 0 failed. Could you re-review this simplification, @cjswedes? Please let me know if further changes are needed. |
|
@cjswedes @KKlimczukS, could you help confirm the STTS validation plan before we arrange the final on-device run? The certification team has requested a complete Full Integration Test report. The currently linked 66VJTV2QBH is a targeted retest (33 passed, 0 failed, 18 user-excluded), not complete integration evidence. We will not treat the combined operational retests as a substitute for the required full flow. This device's children change between Switch and Button profiles according to their relay/scene mode, while the parent exposes both. In our previous STTS run, the parent Button case initialization expected the parent Switch to turn on. An all-Scene configuration has no eligible relay targets, so that setup could not complete initialization; a mixed configuration worked. Could you confirm:
If the certification team must decide the first point, please help route this specific coverage question to them. We will record the exact installed driver package and profile configuration for the eventual reports. |
@zemismart-dev
I'd like to take a look first. |
|
@KKlimczukS The two requested mixed-mode and complementary-mode Full Integration reports have not been generated yet. The three October 13 reports remain targeted operational evidence, not substitutes for those full sessions. October 27 update: we installed commit We are currently blocked by the Test Suite. After sign-in and reloads, the device list returns HTTP 503 from The sample has been paired back to the original test Hub and all four relays are verified Off. We will share the actual downloaded reports once the two complete sessions can run. We have not canceled or resubmitted the certification request during this attempt. |
|
@zemismart-dev |
cjswedes
left a comment
cjswedes
left a comment
There was a problem hiding this comment.
Thank you for the simplification. This is getting much closer, but still a few things.
| local function is_multi_endpoint(device) | ||
| local main_endpoint = device:get_endpoint(0x0006) | ||
| for _, ep in ipairs(device.zigbee_endpoints) do | ||
| for _, ep in pairs(device.zigbee_endpoints) do |
There was a problem hiding this comment.
Please revert ipairs → pairs and the sparse-endpoint test. KES606 endpoints are contiguous, so this isn't needed for your device, and it changes tuya-multi matching for existing Tuya devices. If it's a real bug, please send it as a separate PR.
| if device == nil then return false end | ||
| local parent = device | ||
| if device.network_type == device_lib.NETWORK_TYPE_CHILD then | ||
| parent = device:get_parent_device() | ||
| end | ||
| if parent == nil or parent.zigbee_endpoints == nil then return false end | ||
| if device.network_type == device_lib.NETWORK_TYPE_CHILD | ||
| and not require("tuya-multi.zemismart-kes606.can_handle")(opts, driver, device) then | ||
| return false | ||
| end |
There was a problem hiding this comment.
| if device == nil then return false end | |
| local parent = device | |
| if device.network_type == device_lib.NETWORK_TYPE_CHILD then | |
| parent = device:get_parent_device() | |
| end | |
| if parent == nil or parent.zigbee_endpoints == nil then return false end | |
| if device.network_type == device_lib.NETWORK_TYPE_CHILD | |
| and not require("tuya-multi.zemismart-kes606.can_handle")(opts, driver, device) then | |
| return false | |
| end | |
| if require("tuya-multi.zemismart-kes606.can_handle")(opts, driver, device) then | |
| return true, require("tuya-multi") | |
| end | |
| if device.network_type == device_lib.NETWORK_TYPE_CHILD then return false end |
I believe this is equivalent. (tests pass when I run these changes locally)
| if changed(preferences, old_preferences, "countdownSeconds") then | ||
| local seconds = tonumber(preferences.countdownSeconds) | ||
| if seconds ~= nil and seconds == seconds then | ||
| seconds = math.max(0, math.min(43200, math.floor(seconds))) | ||
| device:send(OnOff.server.commands.OnWithTimedOff( | ||
| device, data_types.Uint8(0), data_types.Uint16(seconds), data_types.Uint16(seconds))) | ||
| end | ||
| end |
There was a problem hiding this comment.
In ZCL, OnWithTimedOff turns the relay on, and its times are in tenths of a second. The preference is in seconds and is passed through unscaled. You said countdowns and cancellation were checked on hardware on Oct 27, so this may be how the device actually behaves. Please confirm that: changing the preference turns the gang on, and what unit does the device use?
| [OnOff.server.commands.On.ID] = scene_command, | ||
| [OnOff.server.commands.Off.ID] = scene_command, | ||
| [OnOff.server.commands.Toggle.ID] = scene_command, | ||
| [SCENE_PRESS_COMMAND] = scene_command, |
There was a problem hiding this comment.
Does the device send On/Off/Toggle commands in scene mode, or only 0xFD? If only 0xFD, remove the other three. I dont think I have ever seen a device send these commands to the hub.
| categories: | ||
| - name: RemoteController | ||
| preferences: | ||
| - title: Indicator Light |
There was a problem hiding this comment.
switchMode should be replaced with stse.changeToWirelessSwitch. The default is false which lines up with relay mode, which is what children start with currently.
Type of Change
Checklist
Description of Change
Add exact-fingerprint support for the Zemismart KES-606US-L4 (
_TZ3000_hurauima / TS0726) to Zigbee Switch. A parent exposes a master switch, an aggregate pushed button and refresh. Four Edge children expose independent relay controls or a single-press scene button, selected through each child's Mode preference.The parent on/off command targets confirmed Switch-mode children. Real scene commands emit the corresponding child's pushed event and the parent aggregate pushed event. Initialization and relay acknowledgements do not synthesize button presses. Countdown, indicator and global/per-gang power-on preferences retain their existing values and protocol writes.
October 21 review simplification
Commit
8bda6645b15deb7c727b682b75f945482b54bb73addresses the six comments in review 5587652083. The dedicated implementation is reduced from 484 to 284 lines.%02Xendpoint keys andlifecycle_handlers.find_child. Child commands, OnOff reports and successful On/Off DefaultResponses delegate to the SDK switch handlers. Thin wrappers retain mode eligibility and parent aggregation; native handling is disabled for these wrappers so it cannot bypass aggregation.scene-switch-4-gang,scene-switch-childandscene-switch-child-scene. The fingerprint and tests use these names. Parent-profile migration and legacy-parent tests are removed. A small local driverSwitched handler remains because the SDK checks the root supported-capability list, which does not include Button.The exact KES606 handler remains nested under
tuya-multi; generic Tuya child routing is unchanged. Inline preferences remain pending agreement on semantically compatible shared definitions. The earlier catalog audit and compatibility explanation remain in the review thread.Validation of the October 21 revision
Package directory alignment
Commit
ee1aa8d1c986b62e198edad5ce38439a8c46844amoves the KES606 package intosrc/tuya-multi/zemismart-kes606/as requested in review 5650074428. All module loads and the newer-SDK dispatcher assertions now usetuya-multi.zemismart-kes606. This is a directory/import update only; the device behavior, fingerprint, profiles and preferences are unchanged.Validation on this commit: official SDK API 14, 15, 19 and 21 each pass 82/82 focused tests; Luacheck reports zero warnings/errors/fatals in all six affected Lua files; diff checks and independent source review pass. The official SDK copies are unmodified, with the same two generated-capability fallbacks for API 14/15 documented above. Upstream Driver Tests for this exact commit have now passed: 57: 4037 passed, 0 failed; 58: 4834 passed, 0 failed; 59: 5213 passed, 0 failed; 60: 5310 passed, 0 failed; 61: 5326 passed, 0 failed; dev: 5330 passed, 0 failed. The current-commit CLA check also passes. The other fork GitHub Actions workflows have since been authorized and completed successfully on the same head (driver tests, lint, profile checks, fingerprint validation and package upload). Successful Jenkins trigger jobs are not counted as the actual Driver Tests results above.
KKlimczukS has requested the downloaded mixed-mode and complementary-mode Full Integration reports for inspection. These are still planned sessions, not existing reports or certification approval; onsite execution has not started.
Hardware and certification status
Commit
ee1aa8dwas installed on October 27 as development package2026-10-27T03:37:11.497994669on the exact_TZ3000_hurauima / TS0726sample. Fresh pairing produced the parent and four children with standard endpoint keys 01–04. On this unloaded sample, current-package pre-session checks covered global and per-gang power-on behavior across seven power cycles, indicator modes, four countdowns and countdown cancellation. Device reports/reads, camera indicator observations and onsite relay confirmation were preserved separately; this does not establish loaded-contact performance. L3/L4 mode/profile transitions were observed; full current-revision scene-event coverage remains part of the planned complementary sessions.The two new Full Integration reports are still outstanding. The Test Suite device list returned HTTP 503 on October 27, and two attempts to create an onboarding session ended with “Service Temporarily Unavailable” without displaying a new session ID. The unit was restored to the original test Hub, with four Switch children, countdown preferences at 0 and all four relays verified Off. After that restoration pairing, the new parent detail page would not open; its default indicator/power-on preference text has not been independently confirmed by new D010/8001/8002 hardware reports. No certification approval or completed Full Integration test is claimed.
The earlier downloadable STTS reports belong specifically to commit
192595d7af85bb0d2b56c4aeeedc07b323bc6877and package2026-10-13T07:18:46.468510907:66VJTV2QBH: 33 passed, 0 failed, 18 excluded; former parent automation-presentation failure and parent/four-child relay control.47NKZ9SQDD: 28 passed, 0 failed, 14 excluded; parent aggregate press, L2/L3/L4 real scene presses and mixed-mode switch checks.7YSI12M2VD: 20 passed, 0 failed, 19 excluded; L1 real scene press. All four children were subsequently restored to Switch with D020 readback, and the final master on/off operation was confirmed by protocol/cloud evidence and the onsite operator, ending all-off.These are targeted operational reports, not a new Full Integration Test. Canceled all-Scene initialization session
94I3MFKV7Dremains 10 passed/1 failed/13 skipped/15 excluded. Historical September17 full report6MUUP3G0YHremains 46 passed/1 failed on its earlier package; scene attemptXQA0I9YQF8remains 17 passed/5 failed/13 excluded. Later continuous-log checks do not relabel those reports as passing. The earlier cloud-state stall cleared after a Hub restart, but its cause was never established.On October20, request
20e3a431-1aa0-4f3e-9936-491c440acccbwas canceled with the applicant's authorization and the same product was resubmitted as request7b8d62b2-561f-4ee9-826d-c313dd49da9e(product version 2). The portal confirmed submission, formally linking66VJTV2QBH; both companion report links and their tested-version/exclusion limits are in the reviewer notes and testing instructions. This submission supplies October13 targeted evidence, not hardware validation of the later routing changes. No certification approval is claimed.