cover position control by RealByron · Pull Request #402 · romasku/tuya-zigbee-switch

RealByron · 2026-04-17T10:44:04Z

Cover position control (GoToLiftPercentage)

Summary

Adds time-based position tracking and GoToLiftPercentage support for cover devices. Once calibrated with travel times, the cover can move to an absolute position (0–100%) and stop automatically.

Changes

1. Position tracking (`cover_cluster.c`, `cover_cluster.h`)

Two new writable attributes: OPEN_TIME (0xff02, uint16, ms) and CLOSE_TIME (0xff03, uint16, ms). Setting both to a non-zero value enables position tracking.
When moving, position is live-interpolated from elapsed time. When stopped, the last position is committed to the struct and notified to the coordinator.
GoToLiftPercentage (0x05): computes run time from current position and travel time, starts the motor, and schedules a stop task. Uses max(run_ms, RELAY_MIN_SWITCH_TIME_MS) to guarantee the stop is never deferred by motor protection.
When the scheduled stop fires, the exact target is applied (not time-interpolated) to avoid drift from polling jitter.
Manual open/close/stop commands cancel any active GoToLiftPercentage target.
All three values (open time, close time, last position) are persisted to NVM. Zero in NVM is treated as "unset" to keep backward compatibility with older firmware that never wrote these fields.

2. Config parser bug fixes (`config_parser.c`)

pressed_when_high was only set for pull-down buttons in the X handler (cover switch) and not set at all for B (reset button). Both are now derived consistently from the pull direction: pull-down → pressed when high, pull-up → pressed when low.
Relay on_high was hardcoded to 1 for both R (relay) and C (cover relay) handlers. Both now respect an i suffix in the config string to support inverted relay outputs.

3. Zigbee2MQTT converter template (`switch_custom.js.jinja`)

Replace all raw hex data-type literals (0x30, 0x20, etc.) with Zcl.DataType.* named constants throughout.
Remove deviceAddCustomCluster block for closuresWindowCovering — the windowCovering modernExtend already handles cluster setup; custom attributes are now accessed by explicit ID.
Add coverOpenTime and coverCloseTime converter helpers (numeric, 0–60000 ms, config category).
Add position (0x0008) to the configure-reporting block alongside the existing moving attribute.
Update moving and motorReversal attribute references from named strings to explicit {ID, type} objects, consistent with the new approach.

4. Defensive NULL guards (`switch_cluster.c`, `cover_switch_cluster.c`, `cover_cluster.c`)

Attribute-write trampolines now guard against being called for an endpoint that was never registered, preventing a NULL dereference if the Zigbee stack delivers a write to an unexpected endpoint.

5. New device entry (`device_db.yaml`)

Added MODULE_LORATAP_TS130F_1GANG (LoraTap SC500ZB-v4, 1-gang curtain module, status: in_progress).

Tests

10 new tests covering position tracking, GoToLiftPercentage (from open, from closed, already at target, stops at target, cancelled by manual stop, no-tracking guard).
Existing 184 tests unchanged and passing (194 total).

Verification

make tests              # 194 passed
make stub/build         # clean build

andrei-lazarov · 2026-04-17T13:47:40Z

Whoops, I think @LorandBiro was already working on this: LorandBiro/cover-position

Nearly 100 (public) forks to track! 🙂

LorandBiro · 2026-04-17T13:56:37Z

Thanks for tagging me!

Yeah, there’s a large overlap, but both branches have some extras. I added more features around position control, my branch supports delays at the end positions, which makes the position calculation a bit more complex. You also added bug fixes that look important.

I’m going to compare the two solutions in detail tonight.

RealByron · 2026-04-17T15:50:54Z

Thanks for tagging me!

Yeah, there’s a large overlap, but both branches have some extras. I added more features around position control, my branch supports delays at the end positions, which makes the position calculation a bit more complex. You also added bug fixes that look important.

I’m going to compare the two solutions in detail tonight.

I will do also that tomorrow ;-)

RealByron · 2026-04-18T06:35:04Z

Thanks for tagging me!

Yeah, there’s a large overlap, but both branches have some extras. I added more features around position control, my branch supports delays at the end positions, which makes the position calculation a bit more complex. You also added bug fixes that look important.

I’m going to compare the two solutions in detail tonight.

I think yours is more robust that mine ! I think mine can be dropped. I will test your branch on my device.

LorandBiro

Hey,
I took a look at your PR and left a few questions. I’d really appreciate your thoughts on those when you have a moment, as they could help improve my implementation as well.

Also, I’m sorry we ended up working on the same feature in parallel. I’ve had a branch for this for a while, but I was slow to open a PR. In hindsight, sharing it earlier as a draft might have made the overlap more visible.

LorandBiro · 2026-04-19T20:16:48Z

+| **`R`** | Relay / Triac | • Output <br> • Non-latching: `RC1` - 1 pin: on when high <br> • Latching: `RC2C3` - 2 pins: pulse on, pulse off <br> • Add `i` to invert (active-low): `RC1i`  |
 | **`X`** | Cover Switch  | • User input for cover control <br> • Format: `XA2B3u` - 2 pins + pull resistor: open button, close button        |
-| **`C`** | Cover         | • Motor control for curtains/blinds/shades <br> • Format: `CA2B3` - 2 pins: open relay, close relay               |
+| **`C`** | Cover         | • Motor control for curtains/blinds/shades <br> • Format: `CA2B3` - 2 pins: open relay, close relay <br> • Add `i` to invert both relays (active-low): `CA2B3i` |


Is this feature intended to invert the relay states, or to invert the signal that controls the relay? Interpreting it as inverting the relay states sounds like flipping the open/closed states, which doesn’t really make sense for motors.

Further down, it mentions that it inverts the output. That wording is a bit ambiguous to me, I instinctively think of the physical output of the device, rather than the firmware’s logic. I’m not entirely sure how to phrase this better, but I wanted to share my concern.

LorandBiro · 2026-04-19T20:33:59Z

-                    motorReversal: {ID: 0xff01, type: Zcl.DataType.BOOLEAN, write: true},
-                },
-            }),
-            {% endif %}


I noticed you moved away from this custom cluster definition and unified a few things in the file. Did you run into any issues with it?

I’d need to retest to be certain, but from what I remember, the reporting UI didn’t work perfectly without these custom cluster definitions. Reporting itself was functioning, but the UI wasn’t aware of it and couldn’t display the attributes properly.

LorandBiro · 2026-04-19T20:39:24Z

+            valueMax: 60000,
+            unit: "ms",
+            entityCategory: "config",
+        }),


I also considered supporting separate open and close times, but after testing my own shutters, I found the difference to be negligible. Because of that, I decided to simplify the scope of my PR.

Do you personally see a need for this? Do you think it’s something others would find useful? My concern is that it might unnecessarily bloat the UI.

That said, if you and others would like this in the firmware, I’d be happy to add it back to my PR.

On my roller blinds, the difference between opening and closing is over two seconds depending on the window size. They actually close faster.

Then I’ll add this back to my version and open a draft PR with details on how to test it.

LorandBiro · 2026-04-19T20:47:43Z

                                                         uint16_t attribute_id) {
+    if (cover_switch_cluster_by_endpoint[endpoint] == NULL) {
+        return;
+    }


Could you help me understand the purpose of this guard? In what situations would this actually occur?

I noticed that the switch_cluster_callback_attr_write_trampoline and relay_cluster_callback_attr_write_trampoline functions don’t include this guard, which made me wonder why it’s needed here.

LorandBiro · 2026-04-19T21:01:00Z

+            return HAL_ZIGBEE_CMD_SKIPPED;
+        }
+        cover_go_to_lift_percentage(cluster, ((uint8_t *)cmd_payload)[0]);
+        break;


Have you tested this with Zigbee2Mqtt? When I worked on this part, I had to make two changes in zigbee_zcl.c to get the payload through. I just want to make sure I’m not introducing unnecessary complexity if it’s not needed.

RealByron force-pushed the cover_position branch 2 times, most recently from caa3e83 to 0efd35e Compare April 17, 2026 10:53

cover position control

154c89c

RealByron force-pushed the cover_position branch from 0efd35e to 154c89c Compare April 17, 2026 10:57

some doc updates

e362676

RealByron marked this pull request as draft April 18, 2026 07:29

LorandBiro reviewed Apr 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cover position control#402

cover position control#402
RealByron wants to merge 2 commits into
romasku:mainfrom
RealByron:cover_position

RealByron commented Apr 17, 2026

Uh oh!

andrei-lazarov commented Apr 17, 2026 •

edited

Loading

Uh oh!

LorandBiro commented Apr 17, 2026

Uh oh!

RealByron commented Apr 17, 2026

Uh oh!

RealByron commented Apr 18, 2026

Uh oh!

LorandBiro left a comment

Uh oh!

LorandBiro Apr 19, 2026

Uh oh!

LorandBiro Apr 19, 2026 •

edited

Loading

Uh oh!

LorandBiro Apr 19, 2026

Uh oh!

DawidLBD Apr 20, 2026

Uh oh!

LorandBiro Apr 21, 2026

Uh oh!

LorandBiro Apr 19, 2026

Uh oh!

LorandBiro Apr 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

RealByron commented Apr 17, 2026

Cover position control (GoToLiftPercentage)

Summary

Changes

1. Position tracking (cover_cluster.c, cover_cluster.h)

2. Config parser bug fixes (config_parser.c)

3. Zigbee2MQTT converter template (switch_custom.js.jinja)

4. Defensive NULL guards (switch_cluster.c, cover_switch_cluster.c, cover_cluster.c)

5. New device entry (device_db.yaml)

Tests

Verification

Uh oh!

andrei-lazarov commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LorandBiro commented Apr 17, 2026

Uh oh!

RealByron commented Apr 17, 2026

Uh oh!

RealByron commented Apr 18, 2026

Uh oh!

LorandBiro left a comment

Choose a reason for hiding this comment

Uh oh!

LorandBiro Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

LorandBiro Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LorandBiro Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

DawidLBD Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

LorandBiro Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

LorandBiro Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

LorandBiro Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

1. Position tracking (`cover_cluster.c`, `cover_cluster.h`)

2. Config parser bug fixes (`config_parser.c`)

3. Zigbee2MQTT converter template (`switch_custom.js.jinja`)

4. Defensive NULL guards (`switch_cluster.c`, `cover_switch_cluster.c`, `cover_cluster.c`)

5. New device entry (`device_db.yaml`)

andrei-lazarov commented Apr 17, 2026 •

edited

Loading

LorandBiro Apr 19, 2026 •

edited

Loading