Merge pull request romasku#405 from RealByron/few_fixes

Some fixes and generalisations

Author: eakorolev

docs/contribute/porting.md

@@ -90,9 +90,9 @@ The pinout is stored in the **device config string**.
 | **`B`** | Reset button  | • Puts device in pairing                                                                                          |
 | **`L`** | Network led   | • Blinks while pairing <br> • Is the backlight sometimes                                                          |
 | **`S`** | Switch        | • User input <br> • Tactile/touch button or external switch <br> • Spam to put in pairing mode                    |
-| **`R`** | Relay / Triac | • Output <br> • Non-latching: `RC1` - 1 pin: on when high <br> • Latching: `RC2C3` - 2 pins: pulse on, pulse off  |
+| **`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` |
 | **`I`** | Indicator LED | • 1 per relay, follows state <br> • Briefly flashes on button press (binding confirmation) <br> • Blinks while pairing if there is no network led |
 
 For buttons (`B`), switches (`S`), and cover switches (`X`), the next character chooses the internal pull-up/down resistor:  
@@ -101,7 +101,7 @@ For buttons (`B`), switches (`S`), and cover switches (`X`), the next character
 Usually, pressing the button bridges the GPIO pin to Ground (active low).  
 ⤷ So we need a pull-up resistor `u`: to hold it at VCC (high) while not-pressed.
 
-For LEDs, add `i` to invert the state.
+For LEDs (`L`, `I`), relays (`R`), and cover relays (`C`), add `i` to invert the output (active-low).
 
 Additional options: 
 | Format       | Option                       | Function                                                                          |

docs/updating.md

@@ -57,11 +57,11 @@ This page describes **converting** and **updating** supported devices **wireless
 8. **Start** the update (red download button) 
 9. **Re-download** the custom [# Quirks / Converters / Extensions](#quirks--converters--extensions) and restart ZHA / Z2M
 10. **Interview** the device **`i`**  
-⤷ option missing from ZHA, remove and re-pair if needed  
-(updates endpoints, clusters and identifiers)
-11.  **Reconfigure** the device **`🗘`**  
-(resets reporting and stuff?, keeps user binds and settings)
-12.   Re-do user settings if needed
+  ⤷ option missing from ZHA, remove and re-pair if needed  
+  (updates endpoints, clusters and identifiers)
+11. **Reconfigure** the device **`🗘`**  
+  (resets reporting intervals, keeps user binds and settings)
+12. Re-do user settings if needed
 
 > *If your device is several versions behind, it will update directly to the latest version.*
 
@@ -120,7 +120,7 @@ zha:
 > - e.g. 10W dumb bulb is safe 
 > - estimated values: >4W for EndDevice, >8W for Router  
 > - **not recommended: no-Neutral switch + smart bulb ( <1W when brightness=0 )**  
->   ⤷ dummy load (capactior) may be required
+>   ⤷ dummy load (capacitor) may be required
 
 <details>
 <summary> <b> Index link format </b> </summary>  

helper_scripts/templates/switch_custom.js.jinja

@@ -34,7 +34,7 @@ const romasku = {
             endpointName,
             lookup: { on_off: 0, off_on: 1, toggle_simple: 2, toggle_smart_sync: 3, toggle_smart_opposite: 4 },
             cluster: "genOnOffSwitchCfg",
-            attribute: {ID: 0x0010, type: 0x30, required: true, write: true, min: 0, max: 4}, // Enum8
+            attribute: {ID: 0x0010, type: Zcl.DataType.ENUM8, required: true, write: true, min: 0, max: 4},
             description: `Select how switch should work:
             - on_off: When switch physically moved to position 1 it always generates ON command, and when moved to position 2 it generates OFF command
             - off_on: Same as on_off, but positions are swapped
@@ -49,7 +49,7 @@ const romasku = {
             endpointName,
             lookup: { toggle: 0, momentary: 1, momentary_nc: 2 },
             cluster: "genOnOffSwitchCfg",
-            attribute: { ID: 0xff00, type: 0x30 }, // Enum8
+            attribute: { ID: 0xff00, type: Zcl.DataType.ENUM8 },
             description: "Select the type of switch connected to the device",
             entityCategory: "config",
         }),
@@ -59,7 +59,7 @@ const romasku = {
             endpointName,
             lookup: { detached: 0, press_start: 1, short_press: 3, long_press: 2},
             cluster: "genOnOffSwitchCfg",
-            attribute: { ID: 0xff01, type: 0x30 }, // Enum8
+            attribute: { ID: 0xff01, type: Zcl.DataType.ENUM8 },
             description: "When to turn on/off internal relay",
             entityCategory: "config",
         }),
@@ -71,7 +71,7 @@ const romasku = {
                 Array.from({ length: relay_cnt || 2 }, (_, i) => [`relay_${i + 1}`, i + 1])
             ),
             cluster: "genOnOffSwitchCfg",
-            attribute: { ID: 0xff02, type: 0x20 }, // uint8
+            attribute: { ID: 0xff02, type: Zcl.DataType.UINT8 },
             description: "Which internal relay it should trigger",
             entityCategory: "config",
         }),
@@ -81,7 +81,7 @@ const romasku = {
             endpointName,
             lookup: { press_start: 1, short_press: 3, long_press: 2},
             cluster: "genOnOffSwitchCfg",
-            attribute: { ID: 0xff05, type: 0x30 }, // Enum8
+            attribute: { ID: 0xff05, type: Zcl.DataType.ENUM8 },
             description: "When turn on/off binded device",
             entityCategory: "config",
         }),
@@ -90,7 +90,7 @@ const romasku = {
             name,
             endpointNames: [endpointName],
             cluster: "genOnOffSwitchCfg",
-            attribute: { ID: 0xff03, type: 0x21 }, // uint16
+            attribute: { ID: 0xff03, type: Zcl.DataType.UINT16 },
             description: "What duration is considerd to be long press",
             valueMin: 0,
             valueMax: 5000,
@@ -101,7 +101,7 @@ const romasku = {
             name,
             endpointNames: [endpointName],
             cluster: "genOnOffSwitchCfg",
-            attribute: { ID: 0xff04, type: 0x20 }, // uint8
+            attribute: { ID: 0xff04, type: Zcl.DataType.UINT8 },
             description: "Level (dim) move rate in steps per ms",
             valueMin: 1,
             valueMax: 255,
@@ -124,7 +124,7 @@ const romasku = {
             endpointName,
             lookup: { same: 0, opposite: 1, manual: 2 },
             cluster: "genOnOff",
-            attribute: { ID: 0xff01, type: 0x30 }, // Enum8
+            attribute: { ID: 0xff01, type: Zcl.DataType.ENUM8 },
             description: "Mode for the relay indicator LED",
             entityCategory: "config",
         }),
@@ -135,7 +135,7 @@ const romasku = {
             valueOn: ["ON", 1],
             valueOff: ["OFF", 0],
             cluster: "genOnOff",
-            attribute: {ID: 0xff02, type: 0x10},  // Boolean
+            attribute: {ID: 0xff02, type: Zcl.DataType.BOOLEAN},
             description: "State of the relay indicator LED",
             access: "ALL",
             entityCategory: "config",
@@ -170,17 +170,18 @@ const romasku = {
             valueOn: ["ON", 1],
             valueOff: ["OFF", 0],
             cluster: "genBasic",
-            attribute: {ID: 0xff01, type: 0x10},  // Boolean
+            attribute: {ID: 0xff01, type: Zcl.DataType.BOOLEAN},
             description: "State of the network indicator LED",
             access: "ALL",
             entityCategory: "config",
         }),
+
     multiPressResetCount: (name, endpointName) =>
         numeric({
             name,
             endpointNames: [endpointName],
             cluster: "genBasic",
-            attribute: { ID: 0xff02, type: 0x20 }, // uint8
+            attribute: { ID: 0xff02, type: Zcl.DataType.UINT8 },
             description: "Number of consecutive presses to trigger factory reset (0 = disabled)",
             valueMin: 0,
             valueMax: 255,
@@ -192,7 +193,7 @@ const romasku = {
             endpointName,
             access: "ALL",
             cluster: "genBasic",
-            attribute:  { ID: 0xff00, type: 0x44 }, // long str
+            attribute:  { ID: 0xff00, type: Zcl.DataType.LONG_CHAR_STR },
             description: "Current configuration of the device",
             zigbeeCommandOptions: {timeout: 30_000},
             validate: (value) => {
@@ -441,7 +442,7 @@ const definitions = [
             romasku.relayIndicator("{{relayName}}_indicator", "{{relayName}}"),
             {% endfor %}
             {% for coverName in device.coverNames %}
-            windowCovering({ 
+            windowCovering({
                 controls: ["lift"],
                 coverInverted: true,
                 configureReporting: false,
@@ -470,7 +471,7 @@ const definitions = [
             // switch action:
             await endpoint{{loop.index}}.configureReporting("genMultistateInput", [
                 {
-                    attribute: {ID: 0x0055 /* presentValue */, type: 0x21}, // uint16
+                    attribute: {ID: 0x0055 /* presentValue */, type: Zcl.DataType.UINT16},
                     minimumReportInterval: 0,
                     maximumReportInterval: constants.repInterval.MAX,
                     reportableChange: 1,
@@ -483,7 +484,7 @@ const definitions = [
             await reporting.bind(batteryEndpoint, coordinatorEndpoint, ["genPowerCfg"]);
             await batteryEndpoint.configureReporting("genPowerCfg", [
                 {
-                    attribute: {ID: 0x0021, type: 0x20}, // BatteryPercentageRemaining
+                    attribute: {ID: 0x0021, type: Zcl.DataType.UINT8}, // BatteryPercentageRemaining
                     minimumReportInterval: 0,
                     maximumReportInterval: constants.repInterval.HOUR,
                     reportableChange: 2, // 1% (2 in ZCL 0-200 format)
@@ -503,7 +504,7 @@ const definitions = [
             {% for relayName in device.relayIndicatorNames %}
             await endpoint{{loop.index  + (device.switchNames | length)}}.configureReporting("genOnOff", [
                 {
-                    attribute: {ID: 0xff02, type: 0x10}, // Boolean
+                    attribute: {ID: 0xff02, type: Zcl.DataType.BOOLEAN},
                     minimumReportInterval: 0,
                     maximumReportInterval: constants.repInterval.MAX,
                     reportableChange: 1,

src/device_config/config_parser.c

@@ -133,12 +133,14 @@ void parse_config() {
             hal_gpio_pin_t  pin  = hal_gpio_parse_pin(entry + 1);
             hal_gpio_pull_t pull = hal_gpio_parse_pull(entry + 3);
             hal_gpio_init(pin, 1, pull);
+            bool pressed_when_high = (pull == HAL_GPIO_PULL_DOWN) ? 1 : 0;
 
             buttons[buttons_cnt].pin = pin;
             buttons[buttons_cnt].long_press_duration_ms  = 2000;
             buttons[buttons_cnt].multi_press_duration_ms = 800;
             buttons[buttons_cnt].debounce_delay_ms       = debounce_ms;
             buttons[buttons_cnt].on_long_press           = on_reset_clicked;
+            buttons[buttons_cnt].pressed_when_high       = pressed_when_high;
             buttons_cnt++;
         } else if (entry[0] == 'L') {
             hal_gpio_pin_t pin = hal_gpio_parse_pin(entry + 1);
@@ -188,15 +190,14 @@ void parse_config() {
             hal_gpio_pin_t  pin  = hal_gpio_parse_pin(entry + 1);
             hal_gpio_pull_t pull = hal_gpio_parse_pull(entry + 3);
             hal_gpio_init(pin, 1, pull);
+            bool pressed_when_high = (pull == HAL_GPIO_PULL_DOWN) ? 1 : 0;
 
             buttons[buttons_cnt].pin = pin;
-            buttons[buttons_cnt].long_press_duration_ms  = 800;
-            buttons[buttons_cnt].multi_press_duration_ms = 800;
-            buttons[buttons_cnt].debounce_delay_ms       = debounce_ms;
-            buttons[buttons_cnt].on_multi_press          = on_multi_press_reset;
-
-            if (entry[3] == 'd')
-                buttons[buttons_cnt].pressed_when_high = 1;
+            buttons[buttons_cnt].long_press_duration_ms     = 800;
+            buttons[buttons_cnt].multi_press_duration_ms    = 800;
+            buttons[buttons_cnt].debounce_delay_ms          = debounce_ms;
+            buttons[buttons_cnt].on_multi_press             = on_multi_press_reset;
+            buttons[buttons_cnt].pressed_when_high          = pressed_when_high;
             switch_clusters[switch_clusters_cnt].switch_idx = switch_clusters_cnt;
             switch_clusters[switch_clusters_cnt].mode       =
                 ZCL_ONOFF_CONFIGURATION_SWITCH_TYPE_TOGGLE;
@@ -212,13 +213,14 @@ void parse_config() {
             buttons_cnt++;
             switch_clusters_cnt++;
         } else if (entry[0] == 'R') {
-            hal_gpio_pin_t pin = hal_gpio_parse_pin(entry + 1);
+            hal_gpio_pin_t pin     = hal_gpio_parse_pin(entry + 1);
+            bool           on_high = entry[3] != 'i';
             hal_gpio_init(pin, 0, HAL_GPIO_PULL_NONE);
 
             relays[relays_cnt].pin     = pin;
-            relays[relays_cnt].on_high = 1;
+            relays[relays_cnt].on_high = on_high;
 
-            if (entry[3] != '\0') {
+            if (entry[3] != '\0' && entry[3] != 'i') {
                 pin = hal_gpio_parse_pin(entry + 3);
                 hal_gpio_init(pin, 0, HAL_GPIO_PULL_NONE);
                 relays[relays_cnt].off_pin     = pin;
@@ -231,9 +233,10 @@ void parse_config() {
             relays_cnt++;
             relay_clusters_cnt++;
         } else if (entry[0] == 'X') {
-            hal_gpio_pin_t  open_pin  = hal_gpio_parse_pin(entry + 1);
-            hal_gpio_pin_t  close_pin = hal_gpio_parse_pin(entry + 3);
-            hal_gpio_pull_t pull      = hal_gpio_parse_pull(entry + 5);
+            hal_gpio_pin_t  open_pin          = hal_gpio_parse_pin(entry + 1);
+            hal_gpio_pin_t  close_pin         = hal_gpio_parse_pin(entry + 3);
+            hal_gpio_pull_t pull              = hal_gpio_parse_pull(entry + 5);
+            bool            pressed_when_high = (pull == HAL_GPIO_PULL_DOWN) ? 1 : 0;
 
             hal_gpio_init(open_pin, 1, pull);
             hal_gpio_init(close_pin, 1, pull);
@@ -243,13 +246,15 @@ void parse_config() {
             buttons[buttons_cnt].multi_press_duration_ms = 800;
             buttons[buttons_cnt].debounce_delay_ms       = debounce_ms;
             buttons[buttons_cnt].on_multi_press          = on_multi_press_reset;
+            buttons[buttons_cnt].pressed_when_high       = pressed_when_high;
             button_t *open_button = &buttons[buttons_cnt++];
 
             buttons[buttons_cnt].pin = close_pin;
             buttons[buttons_cnt].long_press_duration_ms  = 800;
             buttons[buttons_cnt].multi_press_duration_ms = 800;
             buttons[buttons_cnt].debounce_delay_ms       = debounce_ms;
             buttons[buttons_cnt].on_multi_press          = on_multi_press_reset;
+            buttons[buttons_cnt].pressed_when_high       = pressed_when_high;
             button_t *close_button = &buttons[buttons_cnt++];
 
             cover_switch_clusters[cover_switch_clusters_cnt].open_button =
@@ -262,17 +267,18 @@ void parse_config() {
         } else if (entry[0] == 'C') {
             hal_gpio_pin_t open_pin  = hal_gpio_parse_pin(entry + 1);
             hal_gpio_pin_t close_pin = hal_gpio_parse_pin(entry + 3);
+            bool           on_high   = entry[5] != 'i';
 
             hal_gpio_init(open_pin, 0, HAL_GPIO_PULL_NONE);
             hal_gpio_init(close_pin, 0, HAL_GPIO_PULL_NONE);
 
             relays[relays_cnt].pin         = open_pin;
-            relays[relays_cnt].on_high     = 1;
+            relays[relays_cnt].on_high     = on_high;
             relays[relays_cnt].is_latching = 0;
             relay_t *open_relay = &relays[relays_cnt++];
 
             relays[relays_cnt].pin         = close_pin;
-            relays[relays_cnt].on_high     = 1;
+            relays[relays_cnt].on_high     = on_high;
             relays[relays_cnt].is_latching = 0;
             relay_t *close_relay = &relays[relays_cnt++];
 

src/zigbee/cover_cluster.c

@@ -164,6 +164,9 @@ void cover_cluster_on_write_attr(zigbee_cover_cluster *cluster, uint16_t attribu
 }
 
 void cover_cluster_callback_attr_write_trampoline(uint8_t endpoint, uint16_t attribute_id) {
+    if (cover_cluster_by_endpoint[endpoint] == NULL) {
+        return;
+    }
     cover_cluster_on_write_attr(cover_cluster_by_endpoint[endpoint], attribute_id);
 }
 

src/zigbee/cover_switch_cluster.c

@@ -324,6 +324,9 @@ void cover_switch_cluster_on_write_attr(zigbee_cover_switch_cluster *cluster,
 
 void cover_switch_cluster_callback_attr_write_trampoline(uint8_t endpoint,
                                                          uint16_t attribute_id) {
+    if (cover_switch_cluster_by_endpoint[endpoint] == NULL) {
+        return;
+    }
     cover_switch_cluster_on_write_attr(cover_switch_cluster_by_endpoint[endpoint],
                                        attribute_id);
 }

src/zigbee/switch_cluster.c

@@ -82,6 +82,9 @@ void switch_cluster_report_action(zigbee_switch_cluster *cluster);
 
 void switch_cluster_callback_attr_write_trampoline(uint8_t endpoint,
                                                    uint16_t attribute_id) {
+    if (switch_cluster_by_endpoint[endpoint] == NULL) {
+        return;
+    }
     switch_cluster_on_write_attr(switch_cluster_by_endpoint[endpoint],
                                  attribute_id);
 }

tests/test_active_low_cover.py

@@ -0,0 +1,28 @@
+"""Tests for active-low cover relay support via the 'i' inversion flag (CA0A1i config)."""
+import pytest
+
+from client import StubProc
+from conftest import Device, MINIMUM_SWITCH_TIME_MS
+
+
+@pytest.fixture
+def cover_device_inverted():
+    p = StubProc(device_config="X;Y;CA0A1i;").start()
+    try:
+        d = Device(p)
+        d.step_time(MINIMUM_SWITCH_TIME_MS)
+        yield d
+    finally:
+        p.stop()
+
+
+def test_active_low_cover_open_drives_open_relay_low(cover_device_inverted: Device):
+    cover_device_inverted.zcl_cover_open(1)
+    assert not cover_device_inverted.get_gpio("A0", refresh=True)
+    assert cover_device_inverted.get_gpio("A1", refresh=True)
+
+
+def test_active_low_cover_close_drives_close_relay_low(cover_device_inverted: Device):
+    cover_device_inverted.zcl_cover_close(1)
+    assert cover_device_inverted.get_gpio("A0", refresh=True)
+    assert not cover_device_inverted.get_gpio("A1", refresh=True)

tests/test_active_low_relay.py

@@ -0,0 +1,25 @@
+"""Tests for active-low relay support via the 'i' inversion flag (RB0i config)."""
+import pytest
+
+from client import StubProc
+from conftest import Device
+
+
+@pytest.fixture
+def relay_inverted_device():
+    p = StubProc(device_config="Mfg;Model;SA0u;RB0i;").start()
+    try:
+        yield Device(p)
+    finally:
+        p.stop()
+
+
+def test_active_low_relay_on_gpio_is_low(relay_inverted_device: Device):
+    relay_inverted_device.zcl_relay_on(2)
+    assert not relay_inverted_device.get_gpio("B0", refresh=True)
+
+
+def test_active_low_relay_off_gpio_is_high(relay_inverted_device: Device):
+    relay_inverted_device.zcl_relay_on(2)
+    relay_inverted_device.zcl_relay_off(2)
+    assert relay_inverted_device.get_gpio("B0", refresh=True)