From 50871ba9cd745083fefdc8f77bcb2039b22d92a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lor=C3=A1nd=20Bir=C3=B3?= Date: Thu, 7 May 2026 13:13:31 +0200 Subject: [PATCH 1/6] Replace deviceAddCustomCluster with direct attribute IDs Using deviceAddCustomCluster for the cover switch and cover clusters caused the Zigbee2MQTT UI to crash when opening the binding and reporting tabs, and made the reconfigure process fail. The issue was reproducible on the main branch as well, so it was not a regression introduced here. Replacing the custom cluster registrations with direct numeric cluster IDs and inline {ID, type} attribute definitions resolves the crashes and restores reliable reconfiguration. The trade-off is reduced visibility of custom attributes in the Z2M reporting UI. --- .../templates/switch_custom.js.jinja | 57 ++++++------------- 1 file changed, 16 insertions(+), 41 deletions(-) diff --git a/helper_scripts/templates/switch_custom.js.jinja b/helper_scripts/templates/switch_custom.js.jinja index a70452ca03..a2b9d99ebb 100644 --- a/helper_scripts/templates/switch_custom.js.jinja +++ b/helper_scripts/templates/switch_custom.js.jinja @@ -6,7 +6,6 @@ const { text, binary, windowCovering, - deviceAddCustomCluster, } = require("zigbee-herdsman-converters/lib/modernExtend"); const {assertString} = require("zigbee-herdsman-converters/lib/utils"); const reporting = require("zigbee-herdsman-converters/lib/reporting"); @@ -271,8 +270,8 @@ const romasku = { name, endpointName, lookup: { toggle: 0, momentary: 1 }, - cluster: "manuSpecificTuyaCoverSwitchConfig", - attribute: "switchType", + cluster: 0xFC01, + attribute: {ID: 0x0000, type: Zcl.DataType.ENUM8}, description: "Type of cover switch: toggle (rocker) or momentary (push button)", entityCategory: "config", }), @@ -284,8 +283,8 @@ const romasku = { ['detached', 0], ...Array.from({ length: output_cnt || 2 }, (_, i) => [`cover_${i + 1}`, i + 1]) ]), - cluster: "manuSpecificTuyaCoverSwitchConfig", - attribute: "coverIndex", + cluster: 0xFC01, + attribute: {ID: 0x0001, type: Zcl.DataType.UINT8}, description: "Which cover to control locally (detached = no local control)", entityCategory: "config", }), @@ -295,8 +294,8 @@ const romasku = { endpointName, valueOn: ["ON", 1], valueOff: ["OFF", 0], - cluster: "manuSpecificTuyaCoverSwitchConfig", - attribute: "reversal", + cluster: 0xFC01, + attribute: {ID: 0x0002, type: Zcl.DataType.BOOLEAN}, description: "Inverts UP/DOWN direction for inputs", access: "ALL", entityCategory: "config", @@ -306,8 +305,8 @@ const romasku = { name, endpointName, lookup: { immediate: 0, short_press: 1, long_press: 2, hybrid: 3 }, - cluster: "manuSpecificTuyaCoverSwitchConfig", - attribute: "localMode", + cluster: 0xFC01, + attribute: {ID: 0x0003, type: Zcl.DataType.ENUM8}, description: "When to trigger local cover: immediate (start/stop on press), short_press (trigger on release), long_press (trigger after long press duration), hybrid (trigger on release or continuous movement while held). Only affects momentary switches", entityCategory: "config", }), @@ -316,8 +315,8 @@ const romasku = { name, endpointName, lookup: { immediate: 0, short_press: 1, long_press: 2, hybrid: 3 }, - cluster: "manuSpecificTuyaCoverSwitchConfig", - attribute: "bindedMode", + cluster: 0xFC01, + attribute: {ID: 0x0004, type: Zcl.DataType.ENUM8}, description: "When to send commands to bound devices: immediate (start/stop on press), short_press (trigger on release), long_press (trigger after long press duration), hybrid (trigger on release or continuous movement while held). Only affects momentary switches", entityCategory: "config", }), @@ -325,8 +324,8 @@ const romasku = { numeric({ name, endpointNames: [endpointName], - cluster: "manuSpecificTuyaCoverSwitchConfig", - attribute: "longPressDuration", + cluster: 0xFC01, + attribute: {ID: 0x0005, type: Zcl.DataType.UINT16}, description: "Threshold in milliseconds to distinguish short press from long press", valueMin: 0, valueMax: 5000, @@ -343,7 +342,7 @@ const romasku = { closing: 2 }, cluster: "closuresWindowCovering", - attribute: "moving", + attribute: {ID: 0xff00, type: Zcl.DataType.ENUM8}, description: "Cover movement status", entityCategory: "diagnostic", }), @@ -354,7 +353,7 @@ const romasku = { valueOn: [true, 1], valueOff: [false, 0], cluster: "closuresWindowCovering", - attribute: "motorReversal", + attribute: {ID: 0xff01, type: Zcl.DataType.BOOLEAN}, description: "Reverse motor direction (swap OPEN/CLOSE relays)", entityCategory: "config", }), @@ -376,31 +375,7 @@ const definitions = [ {% if device.has_battery_cluster %} romasku.batteryPercentage(), {% endif %} - {% if device.coverSwitchNames %} - deviceAddCustomCluster("manuSpecificTuyaCoverSwitchConfig", { - ID: 0xFC01, - manufacturerCode: 0x125D, - attributes: { - switchType: {ID: 0x0000, type: Zcl.DataType.ENUM8, write: true}, - coverIndex: {ID: 0x0001, type: Zcl.DataType.UINT8, write: true}, - reversal: {ID: 0x0002, type: Zcl.DataType.BOOLEAN, write: true}, - localMode: {ID: 0x0003, type: Zcl.DataType.ENUM8, write: true}, - bindedMode: {ID: 0x0004, type: Zcl.DataType.ENUM8, write: true}, - longPressDuration: {ID: 0x0005, type: Zcl.DataType.UINT16, write: true}, - }, - commands: {}, - commandsResponse: {}, - }), - {% endif %} - {% if device.coverNames %} - deviceAddCustomCluster("closuresWindowCovering", { - ID: 0x0102, - attributes: { - moving: {ID: 0xff00, type: Zcl.DataType.ENUM8}, - motorReversal: {ID: 0xff01, type: Zcl.DataType.BOOLEAN, write: true}, - }, - }), - {% endif %} + deviceEndpoints({ endpoints: { {%- for switchName in device.switchNames -%} "{{switchName}}": {{loop.index}},{{" "}} @@ -529,7 +504,7 @@ const definitions = [ await reporting.bind(cover{{loop.index}}, coordinatorEndpoint, ["closuresWindowCovering"]); await cover{{loop.index}}.configureReporting("closuresWindowCovering", [ { - attribute: "moving", + attribute: {ID: 0xff00, type: Zcl.DataType.ENUM8}, minimumReportInterval: 0, maximumReportInterval: constants.repInterval.MAX, reportableChange: 1, From 9aeae758f88a292ebe400f4593b93dc8711aacd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lor=C3=A1nd=20Bir=C3=B3?= Date: Thu, 7 May 2026 13:13:31 +0200 Subject: [PATCH 2/6] Fix MODULE_TUYA_NOVATO_TS130F converter manufacturer and OTA IDs Update stock_converter_manufacturer from Tuya to Lonsonho and stock_converter_model from TS130F to QS-Zigbee-C03 to match the actual Z2M converter. Add previously null stock_manufacturer_id (4417) and stock_image_type (54179) for correct OTA identification. --- device_db.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/device_db.yaml b/device_db.yaml index 199acb8352..98ddb4a59e 100644 --- a/device_db.yaml +++ b/device_db.yaml @@ -2536,8 +2536,8 @@ MODULE_TUYA_NOVATO_TS130F: device_type: router stock_model_name: TS130F stock_manufacturer_name: _TZ3210_ol1uhvza - stock_converter_manufacturer: Tuya - stock_converter_model: TS130F + stock_converter_manufacturer: Lonsonho + stock_converter_model: QS-Zigbee-C03 override_z2m_device: null tuya_module: ZTLC9 mcu_family: Telink @@ -2546,8 +2546,8 @@ MODULE_TUYA_NOVATO_TS130F: alt_config_str: ol1uhvza;TS130F-NOV;BC3u;LC4;SC2f;RB5;SB4f;RD2; old_manufacturer_names: null old_zb_models: null - stock_manufacturer_id: null - stock_image_type: null + stock_manufacturer_id: 4417 + stock_image_type: 54179 firmware_image_type: 43609 build: yes status: in_progress From 1b2768e3ede6e4ab0a917fdc7ab1d92fe7f4f017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lor=C3=A1nd=20Bir=C3=B3?= Date: Thu, 7 May 2026 13:13:31 +0200 Subject: [PATCH 3/6] Add cover position tracking and configurable travel/deadzone parameters - Split travel_time into separate open_time and close_time attributes (close_time falls back to open_time for symmetric covers) - Rename open/closed slack to deadzone with percentage-based config - Swap open/closed deadzone attribute order (0xff04=open, 0xff05=closed) - Add 3-second overrun when motor reaches an end position for reliable calibration - Add manual mode when both travel times are zero: position tracking is disabled, motor runs until stopped or 5-minute safety timeout - Configurable deadzones as percentage of total travel (previously fixed at 5%) - Remove minimum travel time constraint; always apply overrun at ends - Remove COVER_ACTION_OPEN and COVER_ACTION_CLOSE; use position-based control exclusively - Housekeeping: remove forward declarations, rename update task, simplify cover_dispatch_action, improve documentation - Add custom ZCL cluster handler for Telink window covering - Expand test coverage for all new position tracking behavior - Update changelog, supported devices doc, and devcontainer - Mark GIRIER_TS130F and TUYA_NOVATO_TS130F as fully_supported --- .devcontainer/devcontainer.json | 3 +- device_db.yaml | 12 +- docs/changelog_fw.md | 3 +- .../templates/switch_custom.js.jinja | 69 +- src/telink/Makefile | 1 + .../custom_zcl/zcl_window_covering_custom.c | 57 ++ .../custom_zcl/zcl_window_covering_custom.h | 10 + src/telink/hal/zigbee_zcl.c | 11 +- src/zigbee/consts.h | 11 +- src/zigbee/cover_cluster.c | 485 +++++++++++-- src/zigbee/cover_cluster.h | 34 +- tests/conftest.py | 49 +- tests/test_cover_cluster.py | 636 ++++++++++++++++++ tests/zcl_consts.py | 6 + 14 files changed, 1289 insertions(+), 98 deletions(-) create mode 100644 src/telink/custom_zcl/zcl_window_covering_custom.c create mode 100644 src/telink/custom_zcl/zcl_window_covering_custom.h diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 6c712000da..e952189637 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -11,7 +11,8 @@ "extensions": [ "ms-vscode.cpptools", "ms-vscode.hexeditor", - "redhat.vscode-yaml" + "redhat.vscode-yaml", + "ms-python.python" ] } } diff --git a/device_db.yaml b/device_db.yaml index 98ddb4a59e..76f6f1c583 100644 --- a/device_db.yaml +++ b/device_db.yaml @@ -1245,8 +1245,8 @@ MODULE_GIRIER_TS130F_1GANG: stock_image_type: 54179 firmware_image_type: 43616 build: yes - status: in_progress - info: Curtains not implemented! + status: fully_supported + info: Supported threads: https://github.com/romasku/tuya-zigbee-switch/issues/270 store: https://www.aliexpress.com/item/1005003864471089.html MODULE_GIRIER_TS130F_2GANG: @@ -1272,8 +1272,8 @@ MODULE_GIRIER_TS130F_2GANG: stock_image_type: 54179 firmware_image_type: 45597 build: yes - status: in_progress - info: Curtains in progress! Reset button on B4 pin, same as switch + status: fully_supported + info: Supported - Reset button on B4 pin, same as switch threads: https://github.com/romasku/tuya-zigbee-switch/issues/270 store: https://www.aliexpress.com/item/1005003864471089.html MODULE_GIRIER_ZTU_TS0001: @@ -2550,8 +2550,8 @@ MODULE_TUYA_NOVATO_TS130F: stock_image_type: 54179 firmware_image_type: 43609 build: yes - status: in_progress - info: Curtains in progress! + status: fully_supported + info: Supported threads: https://github.com/romasku/tuya-zigbee-switch/pull/231 store: https://www.aliexpress.com/item/1005007010573203.html MODULE_TUYA_OXT_A_TS0003: diff --git a/docs/changelog_fw.md b/docs/changelog_fw.md index d2a29cad95..3079ccef1b 100644 --- a/docs/changelog_fw.md +++ b/docs/changelog_fw.md @@ -13,7 +13,8 @@ Please describe what you are working on, under ## Upcoming ### Features - **Cover cluster** (window covering) for controlling the motor of curtains, blinds, and shutters. - Supports open, close, and stop commands with motor safety delays. + Supports open, close, stop, and position control commands with configurable opening/closing + travel times and deadzone configuration for precise calibration. - **Cover switch cluster** for handling user input from window covering switches. Supports toggle/momentary switches, stop-on-repeat, stop button, local control, and remote device binding. - Relays now respond to *MoveToLevelWithOnOff* diff --git a/helper_scripts/templates/switch_custom.js.jinja b/helper_scripts/templates/switch_custom.js.jinja index a2b9d99ebb..1d323a90a2 100644 --- a/helper_scripts/templates/switch_custom.js.jinja +++ b/helper_scripts/templates/switch_custom.js.jinja @@ -357,6 +357,68 @@ const romasku = { description: "Reverse motor direction (swap OPEN/CLOSE relays)", entityCategory: "config", }), + coverOpenTime: (name, endpointName) => + numeric({ + name, + endpointNames: [endpointName], + cluster: "closuresWindowCovering", + attribute: {ID: 0xff02, type: Zcl.DataType.UINT16}, + description: "Travel time for the OPENING direction (0.1 s precision). " + + "For symmetric covers you only need to set this value. " + + "Set both open_time and close_time to 0 to enter manual mode: " + + "position tracking is disabled, the motor runs until stopped or " + + "until a 5-minute safety timeout, and position commands are ignored.", + valueMin: 0, + valueMax: 300, + valueStep: 0.1, + scale: 10, + unit: "s", + entityCategory: "config", + }), + coverCloseTime: (name, endpointName) => + numeric({ + name, + endpointNames: [endpointName], + cluster: "closuresWindowCovering", + attribute: {ID: 0xff03, type: Zcl.DataType.UINT16}, + description: "Travel time for the CLOSING direction (0.1 s precision). " + + "Set to 0 to use the same value as open_time. " + + "Set explicitly only if closing speed differs from opening speed.", + valueMin: 0, + valueMax: 300, + valueStep: 0.1, + scale: 10, + unit: "s", + entityCategory: "config", + }), + coverOpenDeadzone: (name, endpointName) => + numeric({ + name, + endpointNames: [endpointName], + cluster: "closuresWindowCovering", + attribute: {ID: 0xff04, type: Zcl.DataType.UINT16}, + description: "Mechanical deadzone at the OPEN end (100%) as a percentage of " + + "total travel. Motor movement within this zone does not change the " + + "reported position.", + valueMin: 0, + valueMax: 50, + unit: "%", + entityCategory: "config", + }), + coverClosedDeadzone: (name, endpointName) => + numeric({ + name, + endpointNames: [endpointName], + cluster: "closuresWindowCovering", + attribute: {ID: 0xff05, type: Zcl.DataType.UINT16}, + description: "Mechanical deadzone at the CLOSED end (0%) as a percentage of " + + "total travel. Motor movement within this zone does not change the " + + "reported position.", + valueMin: 0, + valueMax: 50, + unit: "%", + entityCategory: "config", + }), }; const definitions = [ @@ -375,7 +437,6 @@ const definitions = [ {% if device.has_battery_cluster %} romasku.batteryPercentage(), {% endif %} - deviceEndpoints({ endpoints: { {%- for switchName in device.switchNames -%} "{{switchName}}": {{loop.index}},{{" "}} @@ -419,11 +480,15 @@ const definitions = [ windowCovering({ controls: ["lift"], coverInverted: true, - configureReporting: false, + configureReporting: true, endpointNames: ["{{coverName}}"] }), romasku.coverMoving("{{coverName}}_moving", "{{coverName}}"), romasku.coverMotorReversal("{{coverName}}_motor_reversal", "{{coverName}}"), + romasku.coverOpenTime("{{coverName}}_open_time", "{{coverName}}"), + romasku.coverCloseTime("{{coverName}}_close_time", "{{coverName}}"), + romasku.coverOpenDeadzone("{{coverName}}_open_deadzone", "{{coverName}}"), + romasku.coverClosedDeadzone("{{coverName}}_closed_deadzone", "{{coverName}}"), {% endfor %} {% for coverSwitchName in device.coverSwitchNames %} romasku.coverSwitchPressAction("{{coverSwitchName}}_press_action", "{{coverSwitchName}}"), diff --git a/src/telink/Makefile b/src/telink/Makefile index 1bccff07fe..797ea637ae 100644 --- a/src/telink/Makefile +++ b/src/telink/Makefile @@ -140,6 +140,7 @@ TELINK_SOURCES := \ custom_zcl/zcl_multistate_input.c \ custom_zcl/zcl_onoff_configuration.c \ custom_zcl/zcl_cover_switch_config.c \ + custom_zcl/zcl_window_covering_custom.c \ patch_sdk/drv_nv.c # Common source files (shared with Silicon Labs build) diff --git a/src/telink/custom_zcl/zcl_window_covering_custom.c b/src/telink/custom_zcl/zcl_window_covering_custom.c new file mode 100644 index 0000000000..d83957ee1a --- /dev/null +++ b/src/telink/custom_zcl/zcl_window_covering_custom.c @@ -0,0 +1,57 @@ +#pragma pack(push, 1) +#include "zcl_include.h" +#pragma pack(pop) + +/* + * Custom Window Covering Cluster Implementation + * + * This custom implementation is required because the SDK's default + * zcl_windowCovering_register() only handles UP_OPEN, DOWN_CLOSE, and STOP + * commands. When a GO_TO_LIFT_PERCENTAGE command is received, the SDK returns + * ZCL_STA_UNSUP_CLUSTER_COMMAND even though the command is defined in the spec. + * + * This custom handler adds support for GO_TO_LIFT_PERCENTAGE by properly parsing + * the single-byte percentage payload and passing it to the application callback. + */ + +static status_t zcl_windowCovering_custom_cmdHandler(zclIncoming_t *incoming); + +_CODE_ZCL_ status_t zcl_windowCovering_custom_register(u8 endpoint, u16 manuCode, u8 attrNum, + const zclAttrInfo_t attrTbl[], + cluster_forAppCb_t cb) { + return(zcl_registerCluster(endpoint, ZCL_CLUSTER_CLOSURES_WINDOW_COVERING, manuCode, attrNum, + attrTbl, zcl_windowCovering_custom_cmdHandler, cb)); +} + +_CODE_ZCL_ static status_t zcl_windowCovering_custom_cmdHandler(zclIncoming_t *incoming) { + if (incoming->hdr.frmCtrl.bf.dir != ZCL_FRAME_CLIENT_SERVER_DIR) { + return(ZCL_STA_UNSUP_CLUSTER_COMMAND); + } + + void *payload = NULL; + switch (incoming->hdr.cmd) { + case ZCL_CMD_UP_OPEN: + case ZCL_CMD_DOWN_CLOSE: + case ZCL_CMD_STOP: + break; + case ZCL_CMD_GO_TO_LIFT_PERCENTAGE: + payload = (void *)incoming->pData; + break; + default: + return(ZCL_STA_UNSUP_CLUSTER_COMMAND); + } + + if (!incoming->clusterAppCb) { + return(ZCL_STA_FAILURE); + } + + zclIncomingAddrInfo_t addr; + addr.dirCluster = incoming->hdr.frmCtrl.bf.dir; + addr.profileId = incoming->msg->indInfo.profile_id; + addr.srcAddr = incoming->msg->indInfo.src_short_addr; + addr.dstAddr = incoming->msg->indInfo.dst_addr; + addr.srcEp = incoming->msg->indInfo.src_ep; + addr.dstEp = incoming->msg->indInfo.dst_ep; + + return(incoming->clusterAppCb(&addr, incoming->hdr.cmd, payload)); +} diff --git a/src/telink/custom_zcl/zcl_window_covering_custom.h b/src/telink/custom_zcl/zcl_window_covering_custom.h new file mode 100644 index 0000000000..dd36ecf78d --- /dev/null +++ b/src/telink/custom_zcl/zcl_window_covering_custom.h @@ -0,0 +1,10 @@ +#pragma once + +#pragma pack(push, 1) +#include "zcl_include.h" +#pragma pack(pop) + +status_t zcl_windowCovering_custom_register(u8 endpoint, u16 manuCode, + u8 attrNum, + const zclAttrInfo_t attrTbl[], + cluster_forAppCb_t cb); diff --git a/src/telink/hal/zigbee_zcl.c b/src/telink/hal/zigbee_zcl.c index d0e6850ec7..948f848c88 100644 --- a/src/telink/hal/zigbee_zcl.c +++ b/src/telink/hal/zigbee_zcl.c @@ -4,6 +4,7 @@ #include "zcl_cover_switch_config.h" #include "zcl_include.h" #include "zcl_multistate_input.h" +#include "zcl_window_covering_custom.h" #include "zcl_onoff_configuration.h" #pragma pack(pop) @@ -54,9 +55,9 @@ static cluster_registerFunc_t get_register_func_by_cluster_id(u16 cluster_id) { return zcl_multistate_input_register; } if (cluster_id == ZCL_CLUSTER_CLOSURES_WINDOW_COVERING) { - return zcl_windowCovering_register; + return zcl_windowCovering_custom_register; } - if (cluster_id == 0xFC01) { // Cover Switch Config + if (cluster_id == ZCL_CLUSTER_COVER_SWITCH_CONFIG) { return zcl_cover_switch_config_register; } if (cluster_id == ZCL_CLUSTER_GEN_POLL_CONTROL) { @@ -104,10 +105,10 @@ static status_t cmd_callback_on_off(zclIncomingAddrInfo_t *pAddrInfo, u8 cmdId, static status_t cmd_callback_window_covering(zclIncomingAddrInfo_t *pAddrInfo, u8 cmdId, void *cmdPayload) { - zclIncoming_t *pInMsg = cmd_incoming_from_addr_info(pAddrInfo); - + // cmd_incoming_from_addr_info didn't work here for some reason and I had to use + // the cmdPayload directly. Not sure why, I don't have serial logs to investigate. return cmd_callback(pAddrInfo->dstEp, ZCL_CLUSTER_CLOSURES_WINDOW_COVERING, - cmdId, pInMsg->pData, pInMsg->dataLen); + cmdId, cmdPayload, 1); } static status_t cmd_callback_level_control(zclIncomingAddrInfo_t *pAddrInfo, diff --git a/src/zigbee/consts.h b/src/zigbee/consts.h index 14f117c30e..ce74492bfe 100644 --- a/src/zigbee/consts.h +++ b/src/zigbee/consts.h @@ -93,6 +93,10 @@ #define ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE 0x0008 #define ZCL_ATTR_WINDOW_COVERING_MOVING 0xff00 #define ZCL_ATTR_WINDOW_COVERING_MOTOR_REVERSAL 0xff01 +#define ZCL_ATTR_WINDOW_COVERING_OPEN_TIME 0xff02 +#define ZCL_ATTR_WINDOW_COVERING_CLOSE_TIME 0xff03 +#define ZCL_ATTR_WINDOW_COVERING_OPEN_DEADZONE 0xff04 +#define ZCL_ATTR_WINDOW_COVERING_CLOSED_DEADZONE 0xff05 // Cover Switch Configuration cluster #define ZCL_ATTR_COVER_SWITCH_CONFIG_SWITCH_TYPE 0x0000 @@ -205,9 +209,10 @@ // WindowCovering Cluster -#define ZCL_CMD_WINDOW_COVERING_UP_OPEN 0x00 -#define ZCL_CMD_WINDOW_COVERING_DOWN_CLOSE 0x01 -#define ZCL_CMD_WINDOW_COVERING_STOP 0x02 +#define ZCL_CMD_WINDOW_COVERING_UP_OPEN 0x00 +#define ZCL_CMD_WINDOW_COVERING_DOWN_CLOSE 0x01 +#define ZCL_CMD_WINDOW_COVERING_STOP 0x02 +#define ZCL_CMD_WINDOW_COVERING_GO_TO_LIFT_PERCENTAGE 0x05 // OTA Cluster diff --git a/src/zigbee/cover_cluster.c b/src/zigbee/cover_cluster.c index 30ed262e62..2a3782da88 100644 --- a/src/zigbee/cover_cluster.c +++ b/src/zigbee/cover_cluster.c @@ -18,6 +18,21 @@ // shock to motor/gears when reversing direction. #define RELAY_MIN_SWITCH_TIME_MS 200 +// Extra duration appended to every move that targets a physical end position (fully open or +// fully closed). Window covers drift over time when operated in a sub-range (e.g. 0–50%): +// small timing errors accumulate until the reported position no longer matches reality. +// Applying an overrun on every end-position arrival forces the motor past the reported end, +// re-aligning the physical cover with the tracker on each use. The position is clamped to +// 0% / 100% as soon as the tracker reaches the end, so the reported position is always +// correct during the overrun period. +#define OVERRUN_DURATION_MS 3000 + +// Safety timeout used in manual mode: the maximum time a relay can remain +// energized when position tracking is disabled. Long enough to accommodate even +// very slow covers, and ensures the relay is eventually de-energized if the user +// walks away without stopping the motor. +#define MANUAL_MODE_TIMEOUT_MS 300000 + static zigbee_cover_cluster * cover_cluster_by_endpoint[10]; static zigbee_cover_cluster_config nv_config_buffer; @@ -25,106 +40,360 @@ static zigbee_cover_cluster_config nv_config_buffer; // Value 0 = Rollershade (liftable cover, not tiltable). static uint8_t window_covering_type = 0; -// Current lift position percentage - required by ZCL spec for liftable covers. -// Hardcoded to 50 until position calculation/control is implemented. -static uint8_t cover_position = 50; +// ============================================================================ +// Position Tracking Engine +// ============================================================================ + +#define MAX_MOTOR_POSITION 10000 + +/** + * Returns the travel time in milliseconds for the given direction. If close_time is not + * configured, the open_time is used for both directions. This allows a single open_time value to + * cover both directions for symmetric covers. + */ +static uint16_t cover_travel_time(zigbee_cover_cluster *cluster, uint8_t direction) { + uint16_t travel_time = cluster->open_time; + + if (direction == ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING && cluster->close_time > 0) { + travel_time = cluster->close_time; + } + + return travel_time * 100; // Convert from tenth of seconds to milliseconds +} + +/** + * Converts the internal motor position (0–10000 basis points) to the ZCL + * cover position percentage (0–100), applying open/closed deadzones. + * + * Positions at or above the open deadzone clamp to 100. + * Positions at or below the closed deadzone clamp to 0. + * Values in between are linearly mapped across the effective range. + */ +uint8_t motor_to_cover_position(zigbee_cover_cluster *cluster) { + uint32_t open_deadzone_bp = (uint32_t)cluster->open_deadzone * 100; + uint32_t closed_deadzone_bp = (uint32_t)cluster->closed_deadzone * 100; + + if (cluster->motor_position >= (10000 - open_deadzone_bp)) { + return 100; + } + + if (cluster->motor_position <= closed_deadzone_bp) { + return 0; + } + + uint32_t effective_bp = 10000 - open_deadzone_bp - closed_deadzone_bp; + uint32_t travel_bp = cluster->motor_position - closed_deadzone_bp; + return (travel_bp * 100) / effective_bp; +} + +/** + * Converts a ZCL cover position percentage (0–100) to the internal motor + * position in basis points (0–10000), applying open/closed deadzones. + * + * 0 maps to 0, 100 maps to 10000. Intermediate values are linearly mapped + * across the effective range between the deadzones. + */ +uint16_t cover_to_motor_position(zigbee_cover_cluster *cluster, uint8_t cover_pos) { + if (cover_pos == 0) return 0; + + if (cover_pos >= 100) return 10000; + + uint32_t open_deadzone_bp = (uint32_t)cluster->open_deadzone * 100; + uint32_t closed_deadzone_bp = (uint32_t)cluster->closed_deadzone * 100; + uint32_t effective_bp = 10000 - open_deadzone_bp - closed_deadzone_bp; + return closed_deadzone_bp + (cover_pos * effective_bp) / 100; +} + +void cover_update_position(zigbee_cover_cluster *cluster) { + if (cluster->moving == ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED) { + return; + } + + uint32_t elapsed_ms = hal_millis() - cluster->movement_start_time; + uint32_t travel_time_ms = cover_travel_time(cluster, cluster->moving); + uint32_t motor_position_delta; + if (travel_time_ms == 0) { + motor_position_delta = MAX_MOTOR_POSITION; + } else { + motor_position_delta = (elapsed_ms * MAX_MOTOR_POSITION) / travel_time_ms; + } + + if (cluster->moving == ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING) { + uint32_t new_motor_position = cluster->start_motor_position + motor_position_delta; + if (new_motor_position > MAX_MOTOR_POSITION) { + new_motor_position = MAX_MOTOR_POSITION; + } + cluster->motor_position = (uint16_t)new_motor_position; + } else { + if (motor_position_delta > cluster->start_motor_position) { + cluster->motor_position = 0; + } else { + cluster->motor_position = cluster->start_motor_position - + (uint16_t)motor_position_delta; + } + } + + uint8_t new_cover_pos = motor_to_cover_position(cluster); + if (new_cover_pos != cluster->position) { + cluster->position = new_cover_pos; + hal_zigbee_notify_attribute_changed(cluster->endpoint, + ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE); + } +} + +void cover_cancel_movement_tasks(zigbee_cover_cluster *cluster) { + hal_tasks_unschedule(&cluster->stop_task); + hal_tasks_unschedule(&cluster->update_task); +} + +void cover_schedule_next_update(zigbee_cover_cluster *cluster) { + // Update position every 1% of travel, but not more often than every 100ms. + uint32_t update_interval_ms = cover_travel_time(cluster, cluster->moving) / 100; + + if (update_interval_ms < 100) { + update_interval_ms = 100; + } + + hal_tasks_schedule(&cluster->update_task, update_interval_ms); +} + +void cover_update_handler(void *arg) { + zigbee_cover_cluster *cluster = (zigbee_cover_cluster *)arg; + + cover_update_position(cluster); + + if (cluster->moving != ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED) { + cover_schedule_next_update(cluster); + } +} // ============================================================================ // Movement Control // ============================================================================ /** - * Immediately applies the requested movement state to the relays. - * - * This is a low-level function that directly controls the relay hardware - * without any safety timing checks. Should only be called by cover_request_movement() - * after verifying timing constraints are satisfied. + * Manual mode is active when both open_time and close_time are zero. In this + * mode position tracking is disabled: the position attribute snaps immediately to + * 0% or 100% on move commands, GoToLiftPercentage commands are ignored, and a + * fixed MANUAL_MODE_TIMEOUT_MS safety timeout replaces the travel-time-based stop. */ -void cover_apply_movement(zigbee_cover_cluster *cluster, uint8_t moving) { - relay_t *open_relay = cluster->motor_reversal ? cluster->close_relay : cluster->open_relay; - relay_t *close_relay = cluster->motor_reversal ? cluster->open_relay : cluster->close_relay; - - cluster->last_switch_time = hal_millis(); - if (moving == ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING) { - relay_on(open_relay); - relay_off(close_relay); - cluster->moving = ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING; - }else if (moving == ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING) { - relay_off(open_relay); - relay_on(close_relay); - cluster->moving = ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING; - }else { - relay_off(open_relay); - relay_off(close_relay); - cluster->moving = ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED; - } - - hal_zigbee_notify_attribute_changed(cluster->endpoint, - ZCL_CLUSTER_WINDOW_COVERING, - ZCL_ATTR_WINDOW_COVERING_MOVING); -} - -void cover_schedule_movement(zigbee_cover_cluster *cluster, uint8_t moving, uint32_t delay) { - cluster->pending_movement = moving; - cluster->has_pending_movement = 1; - hal_tasks_schedule(&cluster->delay_task, delay); +static bool cover_is_manual_mode(const zigbee_cover_cluster *cluster) { + return cluster->open_time == 0 && cluster->close_time == 0; } /** - * Requests a movement state change with motor protection timing enforcement. + * Immediately executes an action: sets the relays and schedules the stop_task. + * COVER_ACTION_STOP stops the motor. Returns early if action equals the + * current motor position (already at target). * - * This is the safe, high-level function for all movement requests. It enforces - * minimum time between relay state changes to protect the motor and relay contacts. + * In manual mode (both open_time and close_time are zero), position tracking + * is disabled: the position attribute snaps immediately to 0%/100%, and a fixed + * MANUAL_MODE_TIMEOUT_MS safety timeout is used instead of the travel-time-based stop. * - * If timing constraints aren't met, the movement is scheduled for delayed execution. + * This is a low-level function that directly controls the relay hardware + * without any safety timing checks. Should only be called by cover_dispatch_action() + * or timer callbacks that already know the timing constraints are satisfied. */ -void cover_request_movement(zigbee_cover_cluster *cluster, uint8_t moving) { - // Ignore duplicate requests and cancel any pending delayed operation. This is especially - // important for some cover switches with stop buttons. Their stop button closes both contacts, - // so pressing it while moving might initially appear as a repeated/reversal command before the - // stop command arrives. Canceling pending operations ensures we handle this sequence correctly. - if (moving == cluster->moving) { - if (cluster->has_pending_movement) { - hal_tasks_unschedule(&cluster->delay_task); +void cover_execute_action(zigbee_cover_cluster *cluster, cover_action_t action) { + // COVER_ACTION_NONE is invalid here. + if (action == COVER_ACTION_NONE) { + return; + } + + // If we're already at the target position, there's no need to move or update attributes. + if (action == cluster->motor_position) { + return; + } + + // If we're already stopped, no need to toggle relays or update attributes. + if (action == COVER_ACTION_STOP && cluster->moving == ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED) { + return; + } + + // If we're moving let's cancel any pending stop/position-update tasks and update the position. + // We will either stop the movement completely or reschedule the necessary tasks. + if (cluster->moving != ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED) { + cover_cancel_movement_tasks(cluster); + cover_update_position(cluster); + } + + // Handle the stop case + if (action == COVER_ACTION_STOP) { + relay_off(cluster->close_relay); + relay_off(cluster->open_relay); + cluster->last_switch_time = hal_millis(); + cluster->moving = ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED; + hal_zigbee_notify_attribute_changed(cluster->endpoint, + ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_MOVING); + return; + } + + // Update relays if necessary. + uint8_t direction = (action > cluster->motor_position) + ? ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING + : ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING; + if (direction != cluster->moving) { + relay_t *open_relay = cluster->motor_reversal ? cluster->close_relay : cluster->open_relay; + relay_t *close_relay = cluster->motor_reversal ? cluster->open_relay : cluster->close_relay; + if (direction == ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING) { + relay_on(open_relay); + relay_off(close_relay); + } else if (direction == ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING) { + relay_off(open_relay); + relay_on(close_relay); + } else { + relay_off(open_relay); + relay_off(close_relay); } + cluster->last_switch_time = hal_millis(); + cluster->moving = direction; + hal_zigbee_notify_attribute_changed(cluster->endpoint, + ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_MOVING); + } + + cluster->target_motor_position = action; + // In manual mode, position tracking is disabled. Snap the position attribute + // immediately to the endpoint and schedule a fixed safety timeout instead of the + // travel-time-based stop. + if (cover_is_manual_mode(cluster)) { + cluster->motor_position = (direction == ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING) + ? MAX_MOTOR_POSITION : 0; + cluster->position = (direction == ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING) ? 100 : 0; + hal_zigbee_notify_attribute_changed(cluster->endpoint, + ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE); + hal_tasks_schedule(&cluster->stop_task, MANUAL_MODE_TIMEOUT_MS); return; } - // Enforce motor protection delay. Minimum time must elapse between relay state changes. - uint32_t elapsed = hal_millis() - cluster->last_switch_time; - if (elapsed < RELAY_MIN_SWITCH_TIME_MS) { - cover_schedule_movement(cluster, moving, RELAY_MIN_SWITCH_TIME_MS - elapsed); + // Schedule the stop task for the travel duration plus OVERRUN_DURATION_MS when the target + // is a physical end position. + cluster->start_motor_position = cluster->motor_position; + cluster->movement_start_time = hal_millis(); + uint32_t position_delta; + if (direction == ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING) { + position_delta = cluster->target_motor_position - cluster->motor_position; + } else { + position_delta = cluster->motor_position - cluster->target_motor_position; + } + + uint32_t travel_time_ms = cover_travel_time(cluster, direction); + uint32_t duration_ms = (position_delta * travel_time_ms) / MAX_MOTOR_POSITION; + if (cluster->target_motor_position == 0 || + cluster->target_motor_position == MAX_MOTOR_POSITION) { + duration_ms += OVERRUN_DURATION_MS; + } + + hal_tasks_schedule(&cluster->stop_task, duration_ms); + cover_schedule_next_update(cluster); +} + +void cover_auto_stop_handler(void *arg) { + zigbee_cover_cluster *cluster = (zigbee_cover_cluster *)arg; + + cover_execute_action(cluster, COVER_ACTION_STOP); + + // Snap exactly to target + cluster->motor_position = cluster->target_motor_position; + + uint8_t new_cover_pos = motor_to_cover_position(cluster); + if (new_cover_pos != cluster->position) { + cluster->position = new_cover_pos; + hal_zigbee_notify_attribute_changed(cluster->endpoint, + ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE); + } +} + +void cover_defer_action(zigbee_cover_cluster *cluster, cover_action_t action, uint32_t delay) { + hal_tasks_unschedule(&cluster->delay_task); + cluster->pending_action = action; + hal_tasks_schedule(&cluster->delay_task, delay); +} + +/** + * Dispatches an action with motor protection timing. + * + * Safe, high-level entry point for all movement commands. Enforces + * RELAY_MIN_SWITCH_TIME_MS between relay state changes to protect the + * motor and relay contacts from arc damage and mechanical shock. + */ +void cover_dispatch_action(zigbee_cover_cluster *cluster, cover_action_t action) { + if (action == COVER_ACTION_NONE) { return; } - // Direct transitions to/from STOP can be applied immediately. Direction reversals require - // stopping first to avoid damage to the motor and the relays. - if (moving == ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED || - cluster->moving == ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED) { - cover_apply_movement(cluster, moving); - }else { - cover_apply_movement(cluster, ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED); - cover_schedule_movement(cluster, moving, RELAY_MIN_SWITCH_TIME_MS); + uint32_t elapsed = hal_millis() - cluster->last_switch_time; + if (action == COVER_ACTION_STOP) { + // If already stopped, no action is needed. Cancel any pending deferred action. + if (cluster->moving == ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED) { + if (cluster->pending_action != COVER_ACTION_NONE) { + hal_tasks_unschedule(&cluster->delay_task); + cluster->pending_action = COVER_ACTION_NONE; + } + return; + } + + // If within the minimum switch time, delay the stop. Otherwise, stop immediately. + if (elapsed < RELAY_MIN_SWITCH_TIME_MS) { + cover_defer_action(cluster, COVER_ACTION_STOP, RELAY_MIN_SWITCH_TIME_MS - elapsed); + } else { + cover_execute_action(cluster, COVER_ACTION_STOP); + } + } else { + uint8_t direction = action > cluster->motor_position + ? ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING + : ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING; + + // If already moving in the correct direction, cancel any pending deferred action + // (e.g. a reversal that has not yet executed) and delegate to cover_execute_action, + // which handles all task rescheduling. + if (cluster->moving == direction) { + if (cluster->pending_action != COVER_ACTION_NONE) { + hal_tasks_unschedule(&cluster->delay_task); + cluster->pending_action = COVER_ACTION_NONE; + } + cover_execute_action(cluster, action); + return; + } + + if (elapsed < RELAY_MIN_SWITCH_TIME_MS) { + cover_defer_action(cluster, action, RELAY_MIN_SWITCH_TIME_MS - elapsed); + } else if (cluster->moving == ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED) { + // Already stopped and outside the minimum switch time, so start moving immediately. + cover_execute_action(cluster, action); + } else { + // Moving in the opposite direction and outside the minimum switch time, + // so stop immediately and defer the new action. + cover_execute_action(cluster, COVER_ACTION_STOP); + cover_defer_action(cluster, action, RELAY_MIN_SWITCH_TIME_MS); + } } } void cover_open(zigbee_cover_cluster *cluster) { - cover_request_movement(cluster, ZCL_ATTR_WINDOW_COVERING_MOVING_OPENING); + cover_dispatch_action(cluster, MAX_MOTOR_POSITION); } void cover_close(zigbee_cover_cluster *cluster) { - cover_request_movement(cluster, ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING); + cover_dispatch_action(cluster, 0); } void cover_stop(zigbee_cover_cluster *cluster) { - cover_request_movement(cluster, ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED); + cover_dispatch_action(cluster, COVER_ACTION_STOP); } void cover_delay_handler(void *arg) { zigbee_cover_cluster *cluster = (zigbee_cover_cluster *)arg; - cover_request_movement(cluster, cluster->pending_movement); + cover_action_t action = cluster->pending_action; + + cluster->pending_action = COVER_ACTION_NONE; + cover_dispatch_action(cluster, action); } // ============================================================================ @@ -132,7 +401,12 @@ void cover_delay_handler(void *arg) { // ============================================================================ void cover_cluster_store_attrs_to_nv(zigbee_cover_cluster *cluster) { - nv_config_buffer.motor_reversal = cluster->motor_reversal; + nv_config_buffer.motor_reversal = cluster->motor_reversal; + nv_config_buffer.open_time = cluster->open_time; + nv_config_buffer.close_time = cluster->close_time; + nv_config_buffer.open_deadzone = cluster->open_deadzone; + nv_config_buffer.closed_deadzone = cluster->closed_deadzone; + nv_config_buffer.motor_position = cluster->motor_position; hal_nvm_write(NV_ITEM_COVER_CONFIG(cluster->cover_idx), sizeof(zigbee_cover_cluster_config), @@ -148,7 +422,14 @@ void cover_cluster_load_attrs_from_nv(zigbee_cover_cluster *cluster) { return; } - cluster->motor_reversal = nv_config_buffer.motor_reversal; + cluster->motor_reversal = nv_config_buffer.motor_reversal; + cluster->open_time = nv_config_buffer.open_time; + cluster->close_time = nv_config_buffer.close_time; + cluster->open_deadzone = nv_config_buffer.open_deadzone; + cluster->closed_deadzone = nv_config_buffer.closed_deadzone; + cluster->motor_position = nv_config_buffer.motor_position; + + cluster->position = motor_to_cover_position(cluster); } // ============================================================================ @@ -157,9 +438,14 @@ void cover_cluster_load_attrs_from_nv(zigbee_cover_cluster *cluster) { void cover_cluster_on_write_attr(zigbee_cover_cluster *cluster, uint16_t attribute_id) { if (attribute_id == ZCL_ATTR_WINDOW_COVERING_MOTOR_REVERSAL) { - cover_request_movement(cluster, ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED); + cover_dispatch_action(cluster, COVER_ACTION_STOP); } + // Deadzone values are percentages: cap each at 50% so we don't exceed 100% + // even if both are set to their maximum. + if (cluster->open_deadzone > 50) cluster->open_deadzone = 50; + if (cluster->closed_deadzone > 50) cluster->closed_deadzone = 50; + cover_cluster_store_attrs_to_nv(cluster); } @@ -181,6 +467,21 @@ hal_zigbee_cmd_result_t cover_cluster_callback(zigbee_cover_cluster *cluster, case ZCL_CMD_WINDOW_COVERING_STOP: cover_stop(cluster); break; + case ZCL_CMD_WINDOW_COVERING_GO_TO_LIFT_PERCENTAGE: + if (cover_is_manual_mode(cluster)) { + break; + } + if (cmd_payload == NULL || cmd_payload_len < 1) { + return HAL_ZIGBEE_MALFORMED_COMMAND; + } + + uint8_t target_percentage = *((uint8_t *)cmd_payload); + if (target_percentage > 100) { + return HAL_ZIGBEE_INVALID_VALUE; + } + + cover_dispatch_action(cluster, cover_to_motor_position(cluster, target_percentage)); + break; default: printf("Unknown cover command: %d\r\n", command_id); return(HAL_ZIGBEE_CMD_SKIPPED); @@ -204,17 +505,33 @@ hal_zigbee_cmd_result_t cover_cluster_callback_trampoline(uint8_t endpoint, void cover_cluster_init(zigbee_cover_cluster *cluster) { // Attributes - cluster->moving = ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED; - cluster->motor_reversal = 0; + cluster->moving = ZCL_ATTR_WINDOW_COVERING_MOVING_STOPPED; + cluster->motor_reversal = 0; + cluster->open_time = 300; + cluster->close_time = 0; + cluster->open_deadzone = 0; + cluster->closed_deadzone = 0; + cluster->position = 50; // State - cluster->last_switch_time = 0; - cluster->pending_movement = 0; - cluster->has_pending_movement = 0; + cluster->last_switch_time = 0; + cluster->pending_action = COVER_ACTION_NONE; + cluster->motor_position = MAX_MOTOR_POSITION / 2; + cluster->movement_start_time = 0; + cluster->start_motor_position = 0; + cluster->target_motor_position = 0; hal_tasks_init(&cluster->delay_task); cluster->delay_task.handler = cover_delay_handler; cluster->delay_task.arg = cluster; + + hal_tasks_init(&cluster->stop_task); + cluster->stop_task.handler = cover_auto_stop_handler; + cluster->stop_task.arg = cluster; + + hal_tasks_init(&cluster->update_task); + cluster->update_task.handler = cover_update_handler; + cluster->update_task.arg = cluster; } void cover_cluster_add_to_endpoint(zigbee_cover_cluster *cluster, hal_zigbee_endpoint *endpoint) { @@ -232,7 +549,7 @@ void cover_cluster_add_to_endpoint(zigbee_cover_cluster *cluster, hal_zigbee_end ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE, ZCL_DATA_TYPE_UINT8, ATTR_READONLY, - cover_position); + cluster->position); SETUP_ATTR(2, ZCL_ATTR_WINDOW_COVERING_MOVING, ZCL_DATA_TYPE_ENUM8, @@ -243,9 +560,29 @@ void cover_cluster_add_to_endpoint(zigbee_cover_cluster *cluster, hal_zigbee_end ZCL_DATA_TYPE_BOOLEAN, ATTR_WRITABLE, cluster->motor_reversal); + SETUP_ATTR(4, + ZCL_ATTR_WINDOW_COVERING_OPEN_TIME, + ZCL_DATA_TYPE_UINT16, + ATTR_WRITABLE, + cluster->open_time); + SETUP_ATTR(5, + ZCL_ATTR_WINDOW_COVERING_CLOSE_TIME, + ZCL_DATA_TYPE_UINT16, + ATTR_WRITABLE, + cluster->close_time); + SETUP_ATTR(6, + ZCL_ATTR_WINDOW_COVERING_OPEN_DEADZONE, + ZCL_DATA_TYPE_UINT16, + ATTR_WRITABLE, + cluster->open_deadzone); + SETUP_ATTR(7, + ZCL_ATTR_WINDOW_COVERING_CLOSED_DEADZONE, + ZCL_DATA_TYPE_UINT16, + ATTR_WRITABLE, + cluster->closed_deadzone); endpoint->clusters[endpoint->cluster_count].cluster_id = ZCL_CLUSTER_WINDOW_COVERING; - endpoint->clusters[endpoint->cluster_count].attribute_count = 4; + endpoint->clusters[endpoint->cluster_count].attribute_count = 8; endpoint->clusters[endpoint->cluster_count].attributes = cluster->attr_infos; endpoint->clusters[endpoint->cluster_count].is_server = 1; endpoint->clusters[endpoint->cluster_count].cmd_callback = cover_cluster_callback_trampoline; diff --git a/src/zigbee/cover_cluster.h b/src/zigbee/cover_cluster.h index a135cb664b..094661dd9c 100644 --- a/src/zigbee/cover_cluster.h +++ b/src/zigbee/cover_cluster.h @@ -6,8 +6,24 @@ #include "hal/tasks.h" #include +#define COVER_ACTION_NONE 0xFFFF // No pending operation +#define COVER_ACTION_STOP 0xFFFE // Stop the motor + +/** + * Represents an action to be performed on a cover. The action can be either: + * - The target motor position between 0 and 10000 + * - COVER_ACTION_STOP to stop the motor immediately + * - COVER_ACTION_NONE to indicate no pending action + */ +typedef uint16_t cover_action_t; + typedef struct { - uint8_t motor_reversal; + uint8_t motor_reversal; + uint16_t open_time; + uint16_t close_time; + uint16_t open_deadzone; + uint16_t closed_deadzone; + uint16_t motor_position; } zigbee_cover_cluster_config; typedef struct { @@ -20,13 +36,23 @@ typedef struct { // Attributes uint8_t moving; uint8_t motor_reversal; - hal_zigbee_attribute attr_infos[4]; + uint16_t open_time; + uint16_t close_time; + uint16_t open_deadzone; + uint16_t closed_deadzone; + uint8_t position; + hal_zigbee_attribute attr_infos[8]; // State uint32_t last_switch_time; - uint8_t has_pending_movement; - uint8_t pending_movement; + cover_action_t pending_action; + uint16_t motor_position; + uint16_t target_motor_position; + uint32_t movement_start_time; + uint16_t start_motor_position; hal_task_t delay_task; + hal_task_t stop_task; + hal_task_t update_task; } zigbee_cover_cluster; void cover_cluster_add_to_endpoint(zigbee_cover_cluster *cluster, hal_zigbee_endpoint *endpoint); diff --git a/tests/conftest.py b/tests/conftest.py index 5d52a79ee7..5e4704c8b5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,8 +18,13 @@ ZCL_ATTR_ONOFF_CONFIGURATION_SWITCH_BINDING_MODE, ZCL_ATTR_ONOFF_CONFIGURATION_SWITCH_MODE, ZCL_ATTR_ONOFF_CONFIGURATION_SWITCH_RELAY_MODE, + ZCL_ATTR_WINDOW_COVERING_CLOSED_DEADZONE, + ZCL_ATTR_WINDOW_COVERING_CLOSE_TIME, + ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE, ZCL_ATTR_WINDOW_COVERING_MOTOR_REVERSAL, ZCL_ATTR_WINDOW_COVERING_MOVING, + ZCL_ATTR_WINDOW_COVERING_OPEN_DEADZONE, + ZCL_ATTR_WINDOW_COVERING_OPEN_TIME, ZCL_CLUSTER_COVER_SWITCH_CONFIG, ZCL_CLUSTER_MULTISTATE_INPUT_BASIC, ZCL_CLUSTER_ON_OFF, @@ -28,6 +33,7 @@ ZCL_CMD_ONOFF_OFF, ZCL_CMD_ONOFF_ON, ZCL_CMD_WINDOW_COVERING_DOWN_CLOSE, + ZCL_CMD_WINDOW_COVERING_GO_TO_LIFT_PERCENTAGE, ZCL_CMD_WINDOW_COVERING_STOP, ZCL_CMD_WINDOW_COVERING_UP_OPEN, ) @@ -204,7 +210,7 @@ def write_zigbee_attr( return res.payload def _exec_zigbee_cmd( - self, endpoint: int, cluster: int, cmd: int, payload: bytes | None = None + self, endpoint: int, cluster: int, cmd: int, payload: bytes | list[int] | None = None ) -> dict[str, str]: cmd_str = f"zcl_cmd {endpoint} 0x{cluster:04X} 0x{cmd:02X}" if payload: @@ -219,9 +225,10 @@ def call_zigbee_cmd( cluster: int, cmd: int, payload: bytes | None = None, + payload_bytes: list[int] | None = None, expected_result: str = "PROCESSED", ) -> dict[str, str]: - result = self._exec_zigbee_cmd(endpoint, cluster, cmd, payload) + result = self._exec_zigbee_cmd(endpoint, cluster, cmd, payload or payload_bytes) assert result.get("result") == expected_result, result return result @@ -497,6 +504,44 @@ def zcl_cover_stop(self, endpoint: int) -> None: ) + def zcl_cover_goto_position(self, endpoint: int, percentage: int) -> None: + self.call_zigbee_cmd( + endpoint, ZCL_CLUSTER_WINDOW_COVERING, + ZCL_CMD_WINDOW_COVERING_GO_TO_LIFT_PERCENTAGE, + payload_bytes=[percentage], + ) + + def zcl_cover_get_position(self, endpoint: int) -> int: + return int(self.read_zigbee_attr( + endpoint, + ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE, + )) + + def zcl_cover_set_open_time(self, endpoint: int, value: int) -> None: + self.write_zigbee_attr( + endpoint, ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_OPEN_TIME, value, + ) + + def zcl_cover_set_close_time(self, endpoint: int, value: int) -> None: + self.write_zigbee_attr( + endpoint, ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_CLOSE_TIME, value, + ) + + def zcl_cover_set_closed_deadzone(self, endpoint: int, value: int) -> None: + self.write_zigbee_attr( + endpoint, ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_CLOSED_DEADZONE, value, + ) + + def zcl_cover_set_open_deadzone(self, endpoint: int, value: int) -> None: + self.write_zigbee_attr( + endpoint, ZCL_CLUSTER_WINDOW_COVERING, + ZCL_ATTR_WINDOW_COVERING_OPEN_DEADZONE, value, + ) + def wait_for( condition_fn: Callable[[], bool], timeout: float = 2.0, interval: float = 0.1 ) -> None: diff --git a/tests/test_cover_cluster.py b/tests/test_cover_cluster.py index ca28f97d84..f4752de0e3 100644 --- a/tests/test_cover_cluster.py +++ b/tests/test_cover_cluster.py @@ -132,3 +132,639 @@ def test_cover_immediate_reversal(cover_device: Device): assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED d.step_time(MINIMUM_SWITCH_TIME_MS) assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + +# ============================================================================ +# Position tracking tests +# ============================================================================ + +# Default travel time is 30000 ms. +# Default initial position is 50%. +TRAVEL_TIME_MS = 30_000 +OVERRUN_DURATION_MS = 3_000 +MANUAL_MODE_TIMEOUT_MS = 300_000 + + +def test_cover_goto_position(cover_device: Device): + d = cover_device + endpoint = 1 + + # Arrange: First fully open the cover + d.zcl_cover_open(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 100 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Act: Go to 50% + d.zcl_cover_goto_position(endpoint, 50) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + # Wait for it to arrive + d.step_time(TRAVEL_TIME_MS) + + # Assert: Cover is at 50% and stopped + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 50 + + +def test_cover_position_with_slack(cover_device: Device): + d = cover_device + endpoint = 1 + + # Arrange: Set closed slack to 10% of total travel — a 10% dead zone near closed. + d.zcl_cover_set_closed_deadzone(endpoint, 10) + + # Move to fully closed first + d.zcl_cover_close(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Act: Open cover and advance ~5% of travel (within the 10% slack zone) + d.zcl_cover_open(endpoint) + # 10% slack = 3000ms with default open_time=300 (30s). Moving 1500ms (5%) stays inside. + d.step_time(1500) + d.zcl_cover_stop(endpoint) + + # Assert: Position should still be 0 since we're within the closed slack zone + assert d.zcl_cover_get_position(endpoint) == 0 + + +def test_cover_auto_stop_at_target(cover_device: Device): + d = cover_device + endpoint = 1 + + # Arrange: Fully open, then go to 25% + d.zcl_cover_open(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 100 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Act: Go to 25% — should auto-stop after 75% of travel_time + d.zcl_cover_goto_position(endpoint, 25) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + # Wait half the expected travel — should still be moving + d.step_time(TRAVEL_TIME_MS // 4) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + # Wait until the full expected travel completes + d.step_time(TRAVEL_TIME_MS) + + # Assert: Cover auto-stopped at target + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 25 + + +def test_cover_sequential_goto_positions(cover_device: Device): + """Reproduce the bug: second goto_position command is ignored after first completes.""" + d = cover_device + endpoint = 1 + + # Arrange: Fully open the cover first + d.zcl_cover_open(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 100 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # First goto: 50% — should work + d.zcl_cover_goto_position(endpoint, 50) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 50 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Second goto: 25% — must also start the motor and reach target + d.zcl_cover_goto_position(endpoint, 25) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING, \ + "Second goto_position was ignored — motor did not start" + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 25 + + # Third goto: back to 75% (reversal direction) + d.step_time(MINIMUM_SWITCH_TIME_MS) + d.zcl_cover_goto_position(endpoint, 75) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING, \ + "Third goto_position was ignored — motor did not start" + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 75 + + +def test_cover_goto_position_target_change_while_moving(cover_device: Device): + """ + Reproduce the bug: changing the position target while already moving in the same direction. + + When cover_goto_position is called while the motor is moving and the new target is in the + SAME direction as the current movement, cover_dispatch_action detects a 'duplicate' and + returns early. target_motor_position is updated but the stop_task is NOT rescheduled. + The auto-stop fires at the OLD target time, then cover_auto_stop_handler snaps + motor_position to the NEW (un-reached) target. + + Concrete scenario: cover is closing from 100% toward 50%. Before that stop fires, we + redirect to 25% (still closing, further away). The fixed firmware must reschedule the + stop for the longer travel and reach 25%. The buggy firmware stops at the 50%-time and + then snaps to 25%, so a subsequent goto_position(25) sees motor_position == target and + silently does nothing. + """ + d = cover_device + endpoint = 1 + + # Arrange: fully open + d.zcl_cover_open(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 100 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Start closing toward 50% (duration = 15 000 ms) + d.zcl_cover_goto_position(endpoint, 50) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + # Immediately redirect to 25% (same direction, but further away → 22 500 ms total) + d.zcl_cover_goto_position(endpoint, 25) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + # After the original 50% stop time (15 000 ms) the cover must STILL be moving + # (it has 7 500 ms left to reach 25%). The bug causes it to stop here instead. + d.step_time(15000 + 1000) # a bit past the original 50% stop + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING, \ + "Motor stopped too early — stop_task was not rescheduled for the new target" + + # Let the remaining travel complete + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 25 + + +# ============================================================================ +# Bug regression tests +# ============================================================================ + + +def test_cover_has_pending_movement_cleared_after_delay(cover_device: Device): + """ + Regression: has_pending_movement was never cleared after cover_delay_handler fired. + + Scenario: open → stop immediately (triggers delayed stop) → wait for delay → + re-open → stop immediately again. If has_pending_movement is stale from the first + delay, the second stop's duplicate-detection path will try to cancel an already-fired + delay_task instead of scheduling a new delayed stop, leaving the motor running. + """ + d = cover_device + endpoint = 1 + + # 1. Open the cover + d.zcl_cover_open(endpoint) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # 2. Stop immediately — within RELAY_MIN_SWITCH_TIME_MS, so stop is delayed + d.zcl_cover_stop(endpoint) + # Motor should NOT have stopped yet (delay is pending) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # 3. Let the delay fire — motor should now be stopped + d.step_time(MINIMUM_SWITCH_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + + # 4. Open again — must wait for motor protection delay since the stop just happened + d.zcl_cover_open(endpoint) + # Motor protection delay is active, so open is pending + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + d.step_time(MINIMUM_SWITCH_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # 5. Stop immediately again — same pattern as step 2 + d.zcl_cover_stop(endpoint) + # Motor should NOT have stopped yet (delay is pending) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + d.step_time(MINIMUM_SWITCH_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED, \ + "Motor did not stop — has_pending_movement was not cleared after first delay" + + +def test_cover_rapid_direction_changes(cover_device: Device): + """ + Regression: cover_defer_action did not unschedule the existing delay_task + before scheduling a new one, causing double task execution. + + Rapid command sequence: open → close → open (all within RELAY_MIN_SWITCH_TIME_MS). + The final state should reflect the last command (opening). + """ + d = cover_device + endpoint = 1 + + # Open + d.zcl_cover_open(endpoint) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # Close immediately — triggers reversal (stop + delayed close) + d.zcl_cover_close(endpoint) + + # Open immediately — should override the pending close + d.zcl_cover_open(endpoint) + + # Let all delays resolve + d.step_time(MINIMUM_SWITCH_TIME_MS * 3) + + # The cover should ultimately be opening (last command wins) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING, \ + "Last command did not win after rapid direction changes" + + +def test_cover_goto_position_after_open_stop_cycle(cover_device: Device): + """ + Reproduce the user's real-world bug: open → stop → goto_position from Z2M. + + The user observed that after open+stop, a goto_position command caused the + relay to actuate for only a very short time (matching the duration of the + previous manual open/stop), then the position snapped back. + """ + d = cover_device + endpoint = 1 + + # Open briefly, then stop + d.zcl_cover_open(endpoint) + d.step_time(2000) # open for 2 seconds + d.zcl_cover_stop(endpoint) + d.step_time(MINIMUM_SWITCH_TIME_MS) # wait for stop to take effect + + pos_after_stop = d.zcl_cover_get_position(endpoint) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + + # Now send goto_position(80) — like Z2M would + d.zcl_cover_goto_position(endpoint, 80) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING, \ + "goto_position did not start motor after open+stop cycle" + + # Motor should still be running after a short time (not immediately stopping) + d.step_time(1000) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING, \ + "Motor stopped too early after goto_position — possible stale timing" + + # Let it complete + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 80 + + +# ============================================================================ +# Asymmetric open_time / close_time tests +# ============================================================================ + +def test_cover_asymmetric_open_faster(cover_device: Device): + """open_time=150 (15 s) is half the close_time=300 (30 s). + Starting from 0% (fully closed), opening to 100% takes 15 000 ms travel + + OVERRUN_DURATION_MS overrun = 18 000 ms total.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 150) + d.zcl_cover_set_close_time(endpoint, 300) + + # Move to fully closed first + d.zcl_cover_close(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Open — 15 000 ms travel to 100% + 3 000 ms overrun = 18 000 ms total + d.zcl_cover_open(endpoint) + d.step_time(15000 + OVERRUN_DURATION_MS) + + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 100 + + +def test_cover_asymmetric_close_faster(cover_device: Device): + """close_time=150 (15 s) is half the open_time=300 (30 s). + Starting from 100% (fully open), closing to 0% takes 15 000 ms travel + + OVERRUN_DURATION_MS overrun = 18 000 ms total.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 300) + d.zcl_cover_set_close_time(endpoint, 150) + + # Move to fully open first + d.zcl_cover_open(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 100 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Close — 15 000 ms travel to 0% + 3 000 ms overrun = 18 000 ms total + d.zcl_cover_close(endpoint) + d.step_time(15000 + OVERRUN_DURATION_MS) + + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 0 + + +def test_cover_asymmetric_goto_position_open(cover_device: Device): + """With open_time=150 (15 000 ms full travel), goto_position(50) from 0% + should auto-stop at 50% after exactly 7 500 ms.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 150) + d.zcl_cover_set_close_time(endpoint, 300) + + # Start from fully closed + d.zcl_cover_close(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + d.zcl_cover_goto_position(endpoint, 50) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # Still moving just before the expected stop time + d.step_time(7000) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # Let the remaining 500 ms elapse + d.step_time(1000) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 50 + + +def test_cover_asymmetric_goto_position_close(cover_device: Device): + """With close_time=150 (15 000 ms full travel), goto_position(50) from 100% + should auto-stop at 50% after exactly 7 500 ms.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 300) + d.zcl_cover_set_close_time(endpoint, 150) + + # Start from fully open + d.zcl_cover_open(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 100 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + d.zcl_cover_goto_position(endpoint, 50) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + # Still moving just before the expected stop time + d.step_time(7000) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + + # Let the remaining 500 ms elapse + d.step_time(1000) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 50 + + +def test_cover_asymmetric_position_tracking_mid_stop(cover_device: Device): + """open_time=200 (20 000 ms), close_time=400 (40 000 ms). + Opening for 10 000 ms (50% of open travel) should land at 50%. + Then closing for 20 000 ms (50% of close travel) should return to 0%.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 200) + d.zcl_cover_set_close_time(endpoint, 400) + + # Start from fully closed + d.zcl_cover_close(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Open for 10 000 ms — half of 20 000 ms → 50% + d.zcl_cover_open(endpoint) + d.step_time(10000) + d.zcl_cover_stop(endpoint) + d.step_time(MINIMUM_SWITCH_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 50 + + # Close for 20 000 ms — half of 40 000 ms → back to 0% + d.zcl_cover_close(endpoint) + d.step_time(20000) + d.zcl_cover_stop(endpoint) + d.step_time(MINIMUM_SWITCH_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + + +def test_cover_asymmetric_slack(cover_device: Device): + """closed_deadzone is a percentage of physical travel, independent of directional speed. + With closed_deadzone=10 (10%), the dead zone is 1 000 basis points. + Opening at open_time=150 (15 000 ms full travel), 1 500 ms covers + 1 500/15 000 * 10 000 = 1 000 bp — exactly at the edge of the slack zone + (motor_position <= closed_deadzone_bp is True) → position stays at 0%. + With the old time-based formula this would have required different values for + each direction; now one percentage works regardless of speed asymmetry.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 150) + d.zcl_cover_set_close_time(endpoint, 300) + d.zcl_cover_set_closed_deadzone(endpoint, 10) # 10% = 1000 bp, independent of direction + + # Start from fully closed + d.zcl_cover_close(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Open for 1 500 ms — within the slack zone + d.zcl_cover_open(endpoint) + d.step_time(1500) + d.zcl_cover_stop(endpoint) + d.step_time(MINIMUM_SWITCH_TIME_MS) + + assert d.zcl_cover_get_position(endpoint) == 0, \ + "Position should remain 0 while inside the closed slack zone" + + +def test_cover_close_time_fallback_to_open_time(cover_device: Device): + """close_time=0 (the default sentinel) means fall back to open_time. + With open_time=150 and close_time=0, closing takes the same 15 000 ms travel + as opening, plus OVERRUN_DURATION_MS overrun at the end position.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 150) + # close_time left at 0 (default — falls back to open_time) + + # Move to fully open first (from default 50%: 7 500 ms travel + 3 000 ms overrun = 10 500 ms) + d.zcl_cover_open(endpoint) + d.step_time(15000) + assert d.zcl_cover_get_position(endpoint) == 100 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Close — 15 000 ms travel to 0% + 3 000 ms overrun = 18 000 ms total + d.zcl_cover_close(endpoint) + d.step_time(15000 + OVERRUN_DURATION_MS) + + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED, \ + "Motor did not stop — close_time fallback to open_time did not work" + assert d.zcl_cover_get_position(endpoint) == 0 + + +def test_cover_both_times_default(cover_device: Device): + """When neither open_time nor close_time is explicitly set (open_time=300, + close_time=0), the built-in default of 30 seconds applies to both directions. + A full 0% → 100% open takes TRAVEL_TIME_MS + OVERRUN_DURATION_MS total.""" + d = cover_device + endpoint = 1 + + # Start from fully closed (from 50%: 15 000 ms travel + 3 000 ms overrun = 18 000 ms) + d.zcl_cover_close(endpoint) + d.step_time(TRAVEL_TIME_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Open from 0% — 30 000 ms travel + 3 000 ms overrun = 33 000 ms total + d.zcl_cover_open(endpoint) + d.step_time(TRAVEL_TIME_MS + OVERRUN_DURATION_MS) + + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 100 + + +# ============================================================================ +# Overrun tests +# ============================================================================ + +def test_cover_overrun_on_open_from_partial(cover_device: Device): + """Opening from 50% to 100% runs the motor for travel_time + OVERRUN_DURATION_MS. + The position is clamped to 100% as soon as the tracker reaches the end, but the motor + continues for OVERRUN_DURATION_MS to re-align drift accumulated from sub-range usage.""" + d = cover_device + endpoint = 1 + + # Start from the default 50%, open toward 100%. + # With open_time=300 (30 000 ms full travel): 15 000 ms travel + 3 000 ms overrun = 18 000 ms total. + d.zcl_cover_open(endpoint) + + # At travel completion the position is already clamped to 100%, but the motor is still running. + d.step_time(TRAVEL_TIME_MS // 2) # 15 000 ms + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + assert d.zcl_cover_get_position(endpoint) == 100 + + # Motor stops after the additional overrun period. + d.step_time(OVERRUN_DURATION_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 100 + + +def test_cover_overrun_on_close_from_partial(cover_device: Device): + """Closing from 50% to 0% runs the motor for travel_time + OVERRUN_DURATION_MS. + The position is clamped to 0% as soon as the tracker reaches the end, but the motor + continues for OVERRUN_DURATION_MS to re-align drift accumulated from sub-range usage.""" + d = cover_device + endpoint = 1 + + # Start from the default 50%, close toward 0%. + # With open_time=300 (30 000 ms full travel): 15 000 ms travel + 3 000 ms overrun = 18 000 ms total. + d.zcl_cover_close(endpoint) + + # At travel completion the position is already clamped to 0%, but the motor is still running. + d.step_time(TRAVEL_TIME_MS // 2) # 15 000 ms + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + assert d.zcl_cover_get_position(endpoint) == 0 + + # Motor stops after the additional overrun period. + d.step_time(OVERRUN_DURATION_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 0 + + +def test_cover_overrun_total_duration_open_full(cover_device: Device): + """A full 0% \u2192 100% travel takes exactly TRAVEL_TIME_MS + OVERRUN_DURATION_MS. + The motor is still running just before the total duration ends and stops exactly + at the boundary, confirming the overrun is always appended after the travel time.""" + d = cover_device + endpoint = 1 + + # Move to fully closed first (from 50%: 15 000 ms travel + 3 000 ms overrun = 18 000 ms) + d.zcl_cover_close(endpoint) + d.step_time(TRAVEL_TIME_MS // 2 + OVERRUN_DURATION_MS) + assert d.zcl_cover_get_position(endpoint) == 0 + d.step_time(MINIMUM_SWITCH_TIME_MS) + + # Open from 0% \u2192 100%: 30 000 ms travel + 3 000 ms overrun = 33 000 ms total + d.zcl_cover_open(endpoint) + + # Still opening just before the total duration ends + d.step_time(TRAVEL_TIME_MS + OVERRUN_DURATION_MS - 100) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # Stopped after the full duration + d.step_time(200) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 100 + + +# ============================================================================ +# Manual mode tests +# Manual mode is active when both open_time and close_time are zero. +# close_time defaults to 0, so setting open_time=0 is all that is needed. +# ============================================================================ + + +def test_cover_manual_mode_open_and_close(cover_device: Device): + """In manual mode the position attribute snaps immediately to 100% on open + and to 0% on close, without waiting for the travel time to elapse.""" + d = cover_device + endpoint = 1 + + # Enter manual mode: close_time is already 0, only need to zero open_time. + d.zcl_cover_set_open_time(endpoint, 0) + + # Open — position must snap to 100% immediately. + d.zcl_cover_open(endpoint) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + assert d.zcl_cover_get_position(endpoint) == 100 + + # Reverse to close. Motor-protection reversal: stop immediately, then close after + # MINIMUM_SWITCH_TIME_MS. Both steps happen through the normal reversal path. + d.step_time(MINIMUM_SWITCH_TIME_MS) + d.zcl_cover_close(endpoint) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + + # After the protection delay the motor is closing and position snaps to 0%. + d.step_time(MINIMUM_SWITCH_TIME_MS) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_CLOSING + assert d.zcl_cover_get_position(endpoint) == 0 + + +def test_cover_manual_mode_safety_timeout(cover_device: Device): + """In manual mode the motor auto-stops after MANUAL_MODE_TIMEOUT_MS (5 minutes) + instead of the normal travel-time-based stop, ensuring the relay is always + de-energized even if the user walks away.""" + d = cover_device + endpoint = 1 + + d.zcl_cover_set_open_time(endpoint, 0) + + d.zcl_cover_open(endpoint) + assert d.zcl_cover_get_position(endpoint) == 100 + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # Still running just before the safety timeout. + d.step_time(MANUAL_MODE_TIMEOUT_MS - 100) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_OPENING + + # Auto-stopped after the full safety timeout. + d.step_time(200) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 100 + + +def test_cover_manual_mode_goto_position_ignored(cover_device: Device): + """In manual mode, GoToLiftPercentage commands are silently ignored so the + controller cannot auto-stop the motor before it reaches the physical end stop.""" + d = cover_device + endpoint = 1 + + # Enter manual mode. Default position is 50%. + d.zcl_cover_set_open_time(endpoint, 0) + + d.zcl_cover_goto_position(endpoint, 80) + assert d.zcl_cover_get_moving(endpoint) == ZCL_WINDOW_COVERING_MOVING_STOPPED + assert d.zcl_cover_get_position(endpoint) == 50 + diff --git a/tests/zcl_consts.py b/tests/zcl_consts.py index 2181f84d60..514f99d8d7 100644 --- a/tests/zcl_consts.py +++ b/tests/zcl_consts.py @@ -73,8 +73,13 @@ ZCL_ATTR_GROUP_NAME_SUPPORT = 0x0000 # Attributes - Window Covering cluster +ZCL_ATTR_WINDOW_COVERING_CURRENT_POSITION_LIFT_PERCENTAGE = 0x0008 ZCL_ATTR_WINDOW_COVERING_MOVING = 0xFF00 ZCL_ATTR_WINDOW_COVERING_MOTOR_REVERSAL = 0xFF01 +ZCL_ATTR_WINDOW_COVERING_OPEN_TIME = 0xFF02 +ZCL_ATTR_WINDOW_COVERING_CLOSE_TIME = 0xFF03 +ZCL_ATTR_WINDOW_COVERING_OPEN_DEADZONE = 0xFF04 +ZCL_ATTR_WINDOW_COVERING_CLOSED_DEADZONE = 0xFF05 # Attributes - Cover Switch Configuration cluster ZCL_ATTR_COVER_SWITCH_SWITCH_TYPE = 0x0000 @@ -163,6 +168,7 @@ ZCL_CMD_WINDOW_COVERING_UP_OPEN = 0x00 ZCL_CMD_WINDOW_COVERING_DOWN_CLOSE = 0x01 ZCL_CMD_WINDOW_COVERING_STOP = 0x02 +ZCL_CMD_WINDOW_COVERING_GO_TO_LIFT_PERCENTAGE = 0x05 # Data types (not currently used in tests, provided for completeness) ZCL_DATA_TYPE_NO_DATA = 0x00 From 2b0a54e5a4c961d8a9931668d8f44d17d1311da6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lor=C3=A1nd=20Bir=C3=B3?= Date: Thu, 7 May 2026 13:13:31 +0200 Subject: [PATCH 4/6] Use addrInfo pointer in window covering handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The custom zcl_windowCovering_custom_cmdHandler was building a fresh stack-local zclIncomingAddrInfo_t and passing its address to the app callback. The callback (cmd_callback_window_covering) uses cmd_incoming_from_addr_info — a container_of / offsetof macro — to recover the enclosing zclIncoming_t* from the addrInfo pointer. Since the stack-local copy has no enclosing struct, container_of computed a garbage pointer, making cmd_incoming_from_addr_info fail silently. Fix this by passing &(incoming->addrInfo) — the actual member of the real zclIncoming_t struct — matching how all other Telink cluster handlers invoke their app callbacks. With the correct pointer, cmd_callback_window_covering can use cmd_incoming_from_addr_info to recover pInMsg->pData and pInMsg->dataLen, removing the old workaround that passed cmdPayload with a hardcoded length of 1. Co-authored-by: Hayden --- src/telink/custom_zcl/zcl_window_covering_custom.c | 10 +--------- src/telink/hal/zigbee_zcl.c | 8 ++++---- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/telink/custom_zcl/zcl_window_covering_custom.c b/src/telink/custom_zcl/zcl_window_covering_custom.c index d83957ee1a..e20888ef5a 100644 --- a/src/telink/custom_zcl/zcl_window_covering_custom.c +++ b/src/telink/custom_zcl/zcl_window_covering_custom.c @@ -45,13 +45,5 @@ _CODE_ZCL_ static status_t zcl_windowCovering_custom_cmdHandler(zclIncoming_t *i return(ZCL_STA_FAILURE); } - zclIncomingAddrInfo_t addr; - addr.dirCluster = incoming->hdr.frmCtrl.bf.dir; - addr.profileId = incoming->msg->indInfo.profile_id; - addr.srcAddr = incoming->msg->indInfo.src_short_addr; - addr.dstAddr = incoming->msg->indInfo.dst_addr; - addr.srcEp = incoming->msg->indInfo.src_ep; - addr.dstEp = incoming->msg->indInfo.dst_ep; - - return(incoming->clusterAppCb(&addr, incoming->hdr.cmd, payload)); + return(incoming->clusterAppCb(&(incoming->addrInfo), incoming->hdr.cmd, payload)); } diff --git a/src/telink/hal/zigbee_zcl.c b/src/telink/hal/zigbee_zcl.c index 948f848c88..518d1f3260 100644 --- a/src/telink/hal/zigbee_zcl.c +++ b/src/telink/hal/zigbee_zcl.c @@ -105,10 +105,10 @@ static status_t cmd_callback_on_off(zclIncomingAddrInfo_t *pAddrInfo, u8 cmdId, static status_t cmd_callback_window_covering(zclIncomingAddrInfo_t *pAddrInfo, u8 cmdId, void *cmdPayload) { - // cmd_incoming_from_addr_info didn't work here for some reason and I had to use - // the cmdPayload directly. Not sure why, I don't have serial logs to investigate. - return cmd_callback(pAddrInfo->dstEp, ZCL_CLUSTER_CLOSURES_WINDOW_COVERING, - cmdId, cmdPayload, 1); + zclIncoming_t *pInMsg = cmd_incoming_from_addr_info(pAddrInfo); + + return cmd_callback(pAddrInfo->dstEp, ZCL_CLUSTER_CLOSURES_WINDOW_COVERING, cmdId, + pInMsg->pData, pInMsg->dataLen); } static status_t cmd_callback_level_control(zclIncomingAddrInfo_t *pAddrInfo, From 0564bee3190394ceb3b2070ba04e24d2bc4981c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lor=C3=A1nd=20Bir=C3=B3?= Date: Thu, 7 May 2026 13:13:31 +0200 Subject: [PATCH 5/6] Expose cover moving state properly to Home Assistant The cover moving property was not being reported to Home Assistant because the property name was prefixed (e.g. "cover_left__moving") and Zigbee2MQTT looks it up by the exact name "moving". Also change the access mode from STATE_GET to STATE so the value is properly exposed as a state attribute. See https://github.com/Koenkk/zigbee2mqtt/blob/master/lib/extension/homeassistant.ts#L844 This fixes cover.toggle behaviour: previously HA would toggle between open and closed based on position > 0, and could not detect or stop an in-progress movement. Co-authored-by: DawidLBD <112850302+DawidLBD@users.noreply.github.com> --- helper_scripts/templates/switch_custom.js.jinja | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helper_scripts/templates/switch_custom.js.jinja b/helper_scripts/templates/switch_custom.js.jinja index 1d323a90a2..9f2aaa6dbc 100644 --- a/helper_scripts/templates/switch_custom.js.jinja +++ b/helper_scripts/templates/switch_custom.js.jinja @@ -335,7 +335,7 @@ const romasku = { enumLookup({ name, endpointName, - access: "STATE_GET", + access: "STATE", lookup: { stopped: 0, opening: 1, @@ -483,7 +483,7 @@ const definitions = [ configureReporting: true, endpointNames: ["{{coverName}}"] }), - romasku.coverMoving("{{coverName}}_moving", "{{coverName}}"), + romasku.coverMoving("moving", "{{coverName}}"), romasku.coverMotorReversal("{{coverName}}_motor_reversal", "{{coverName}}"), romasku.coverOpenTime("{{coverName}}_open_time", "{{coverName}}"), romasku.coverCloseTime("{{coverName}}_close_time", "{{coverName}}"), From f3db5ff8fee22dfb2663c2fb84104a28923014d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lor=C3=A1nd=20Bir=C3=B3?= Date: Tue, 26 May 2026 09:30:47 +0200 Subject: [PATCH 6/6] Fix cover_travel_time() overflow for travel times > 65 s cover_travel_time() returned uint16_t but multiplied open_time/close_time (which are stored in tenths of a second) by 100 to convert to milliseconds. For travel times above 65.5 s (655 tenths), the result exceeded 65535 and silently wrapped. A 66 s travel time (660 tenths) would produce 464 ms instead of 66000 ms. Co-authored-by: yarfalksol <285298507+yarfalksol@users.noreply.github.com> --- src/zigbee/cover_cluster.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/zigbee/cover_cluster.c b/src/zigbee/cover_cluster.c index 2a3782da88..ece856c9bb 100644 --- a/src/zigbee/cover_cluster.c +++ b/src/zigbee/cover_cluster.c @@ -51,14 +51,14 @@ static uint8_t window_covering_type = 0; * configured, the open_time is used for both directions. This allows a single open_time value to * cover both directions for symmetric covers. */ -static uint16_t cover_travel_time(zigbee_cover_cluster *cluster, uint8_t direction) { +static uint32_t cover_travel_time(zigbee_cover_cluster *cluster, uint8_t direction) { uint16_t travel_time = cluster->open_time; if (direction == ZCL_ATTR_WINDOW_COVERING_MOVING_CLOSING && cluster->close_time > 0) { travel_time = cluster->close_time; } - return travel_time * 100; // Convert from tenth of seconds to milliseconds + return (uint32_t)travel_time * 100; // Convert from tenth of seconds to milliseconds } /**