Cover position control#408
Conversation
|
I added an "overrun" feature, which lets you press open or close again at the end position to run the motor for another 3 seconds. I did this because calibration was frustrating when the controller thought it had reached the end position, but the cover hadn’t actually reached it physically. The only way to fix that was to send the cover all the way in the opposite direction and then back again. This helped a lot while I was calibrating the dead zones. I’m also considering adding a fixed 1-second overrun whenever we’re at an end position, to improve accuracy when the cover only moves between 0% and 50% without ever reaching 100%. A fixed overrun would ensure that 0% truly hits the physical end position. Any feedback/idea on how to improve this is welcome. |
LorandBiro
force-pushed
the
cover-position
branch
from
2b725e7 to
f7c1c86
Compare
|
Ok, so the question about the "overrun" feature seems to be clarified. I added a 3-second overrun each time the cover is sent to an end position. This allows for realignment in cases where the cover is only used at one end of its range. If both the open and close times are set to 0, manual mode is activated. In this mode, there is no position tracking or control. Based on testing so far, this provides an easy way to set up and calibrate the covers. |
|
I’ve finished the PR, but there are two points where I need help:
|
|
I performed an OTA test with the custom FW, and it loaded without any issues. The config string is correct. |
|
I did some investigation into why cmd_incoming_from_addr_info isn't working for you. I believe it's because of zcl_window_covering_custom.c. This memory stuff is a bit beyond me at the moment, so I don't really understand what's happening there. But I used zcl_level.c from the Telink library as an example and reimplemented it. That appears to have fixed it. I have a branch with the changes you can take a look at. I've run it on a device and it worked. It feels like Telink are doing something weird/wrong here, but it might be best to follow their lead to avoid confusion about why this one callback works differently. Also, have you done all this using only OTA? That's hardcore! I would of bricked so many devices. |
|
TL;DR In zcl_window_covering_custom.c pass a pointer to zclIncoming_t->addrInfo, instead of building your own zclIncomingAddrInfo_t. Slept on this, and I think I understand it a bit more now. The Telink cmd handlers all pass a pointer to the addrInfo (zclIncomingAddrInfo_t ) member of the zclIncoming_t struct they receive. cmd_incoming_from_addr_info is some C magic which gets you a pointer to the whole zclIncoming_t struct, from a pointer to a member of that struct. In your custom cmd handler, you were building a fresh zclIncomingAddrInfo_t struct. The pointer to this fresh struct is in no way connected to the incoming zclIncoming_t, thus cmd_incoming_from_addr_info doesn't work. Still not 100% clear on how cmd_incoming_from_addr_info works. It appears to also be known as the container_of macro and is used a lot in the linux kernel. Something to do with a struct being contiguous memory, therefore if you have a pointer to something in that struct you can travel along the memory to find the pointer to the whole thing. Very clever. |
|
@Haydend Thank you so much for looking into this, it felt impossible to figure out without serial access or access to the Telink source code. I tested the changes locally, and everything seems to work correctly.
Yeah, I did everything over OTA, the first update was nerve-wracking :D But I got lucky and didn’t brick any devices. |
|
Hello, Thank you very much for your help. I have six of these curtain modules purchased from AliExpress: https://alishort.com/IebOn I was wondering whether it would be possible to include the corresponding firmware support directly in the main build. I noticed that there are two links available: one for the OTA firmware and one for the Zigbee2MQTT converter. Could you please confirm whether these are already updated to the latest version? Thank you in advance for your support. Francesco |
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.
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.
- 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
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 <haydendunnicliffe@gmail.com>
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>
Hey, I also use those curtain modules, it should be working perfectly. Some functionality is already merged to main. If you can live without the position control, you can already update your devices by following the instructions in the documentation. This PR will be merged soon, then an official firmware upgrade will add position control. The converter and OTA files are not updated in this branch. If you want to test the changes before the merge, you can get the binaries and the converter from my build branch: cover-position-build You can get the binaries directly from there, or you can follow the documentation and change the URLs. Please note that you'll have to wait for this build before the files are updated on the build branch: https://github.com/LorandBiro/tuya-zigbee-switch/actions/runs/25492620252 |
|
Hello, I have not tested all the functions yet, but the switch already works much better with this new firmware. Thank you again for your work and support. EDIT: Probably it was me or something wrong with Z2M. This morning I opened Z2M again and the motor direction and cover position percentage were working very well. The position is updated automatically. EDIT2: Ok found the issue. I forgot to Reconfigure the devices after firmware upgrade. Now it works very well. Francesco |
|
Hi, I first updated my 2-gang switches and then came across your roller shutter branch, which I flashed immediately. I’m already looking forward to the position display in the Home Assistant frontend. Are you planning to implement direct binding? |
|
@FrancescoTalotta Thank you very much for testing the branch and sharing your solution. @Todelec Happy to hear that! Could you clarify which firmware version you deployed to your shutter devices? Home Assistant should already display the position properly if you're using the build from my branch. If the position is available in the Z2M UI but not in Home Assistant, please share the details so I can investigate further. Direct binding is already available in the official build. You can set up the switches of a cover module to control other cover modules. Is this what you meant by direct binding? |
|
Hi, Thank you for your quick reply! I’m using version 1.1.2-0564bee3. At first, there were no entries in the Z2M reporting tab, but after adding them, the position is now displayed correctly in the frontend. That was my mistake. Regarding binding: I was able to successfully bind two TS0002-GIR-zmy4lslw devices together. Thanks in advance! |
|
@Todelec if there were no entries in the reporting tab, you should probably re-interview and reconfigure the device, that usually helps. Also, binding has a really bad user interface in Z2M. I just tried binding between two Girier modules, and when I chose the second endpoint, I could choose the cluster as well. After that, binding worked properly. 
|
|
@LorandBiro thank you very much for your effort, great work!
|
Thank you very much for your support. What I was trying to do apparently cannot work. In the meantime, I found out that this cannot work because the switch does not have a closuresWindowCovering client cluster. If I use a cover module instead, I can select the closuresWindowCovering cluster as well, just like in your screenshot. So that’s how I’m going to rebuild it now — of course with your firmware ;) BR and thanks a lot! |
|
@yarfalksol Sorry I missed your message last week. The open and close time is currently limited to 300 seconds: Where did you see that the open and close time is limited to 60 seconds? I looked through the git history and the code, but I couldn't find it. I don't plan to add support for slat angle. This "small" contribution already turned out to be much bigger than I anticipated :) |
|
@LorandBiro seems cover_travel_time() returns uint16_t, causing overflow for travel times > 65 s cover_travel_time() in src/zigbee/cover_cluster.c (line 54) stores the result of open_time * 100 (or close_time * 100) in a uint16_t return value. Since open_time/close_time are stored in tenths of a second (zigbee2mqtt scale: 10), a 66 s travel time is stored as 660, and 660 × 100 = 66 000 silently wraps around to 464 (66000 mod 65536). This makes the motor stop after ~3 s instead of running the full travel time. The overflow threshold is 65535 / 100 = 655 tenths = 65.5 s. Values at or below 65 s work correctly; 66 s and above are broken. The same issue affects both open_time and close_time. Seams cover_travel_time() on line 54 have to be changed from uint16_t to uint32_t. All call sites (lines 113, 152, 284) already assign the result to uint32_t, so no other changes are needed. Once again thank you very much for your effort. |
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>
|
@yarfalksol Oh, I understand now. Thank you so much for investigating this issue in so much detail. I applied the fix and I marked you as co-author. The new build should be available soon. |
7 participants
This PR is the 3rd phase of the window covering feature started in issue #270. It contains position control and tracking.
The branch needs a little more work so it's created as a draft, but builds are available for testing: