fix(zigbee): Add missing locks + allow printBoundDevices to Serial

Author: P-R-O-C-H-Y

Diff for: libraries/Zigbee/examples/Zigbee_On_Off_Switch/Zigbee_On_Off_Switch.ino

@@ -137,18 +137,17 @@ void setup() {
     Serial.printf(".");
     delay(500);
   }
-
-  // Optional: read manufacturer and model name from the bound light
+  
+  // Optional: List all bound devices and read manufacturer and model name
   std::list<zb_device_params_t *> boundLights = zbSwitch.getBoundDevices();
-  //List all bound lights
   for (const auto &device : boundLights) {
-    Serial.printf("Device on endpoint %d, short address: 0x%x\n", device->endpoint, device->short_addr);
+    Serial.printf("Device on endpoint %d, short address: 0x%x\r\n", device->endpoint, device->short_addr);
     Serial.printf(
-      "IEEE Address: %02X:%02X:%02X:%02X:%02X:%02X:%02X:%02X\n", device->ieee_addr[0], device->ieee_addr[1], device->ieee_addr[2], device->ieee_addr[3],
-      device->ieee_addr[4], device->ieee_addr[5], device->ieee_addr[6], device->ieee_addr[7]
+      "IEEE Address: %02X:%02X:%02X:%02X:%02X:%02X:%02X:%02X\r\n", device->ieee_addr[7], device->ieee_addr[6], device->ieee_addr[5], 
+      device->ieee_addr[4], device->ieee_addr[3], device->ieee_addr[2], device->ieee_addr[1], device->ieee_addr[0]
     );
-    Serial.printf("Light manufacturer: %s", zbSwitch.readManufacturer(device->endpoint, device->short_addr, device->ieee_addr));
-    Serial.printf("Light model: %s", zbSwitch.readModel(device->endpoint, device->short_addr, device->ieee_addr));
+    Serial.printf("Light manufacturer: %s\r\n", zbSwitch.readManufacturer(device->endpoint, device->short_addr, device->ieee_addr));
+    Serial.printf("Light model: %s\r\n", zbSwitch.readModel(device->endpoint, device->short_addr, device->ieee_addr));
   }
 
   Serial.println();
@@ -191,6 +190,6 @@ void loop() {
   static uint32_t lastPrint = 0;
   if (millis() - lastPrint > 10000) {
     lastPrint = millis();
-    zbSwitch.printBoundDevices();
+    zbSwitch.printBoundDevices(&Serial);
   }
-}
+}

Diff for: libraries/Zigbee/src/ZigbeeCore.cpp

@@ -411,8 +411,6 @@ void ZigbeeCore::bindingTableCb(const esp_zb_zdo_binding_table_info_t *table_inf
     esp_zb_zdo_binding_table_record_t *record = table_info->record;
     for (int i = 0; i < table_info->count; i++) {
       log_d("Binding table record: src_endp %d, dst_endp %d, cluster_id 0x%04x, dst_addr_mode %d", record->src_endp, record->dst_endp, record->cluster_id, record->dst_addr_mode);
-      log_d("Short address: 0x%04x", record->dst_address.addr_short);
-      log_d("ieee_address: %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", record->dst_address.addr_long[7], record->dst_address.addr_long[6], record->dst_address.addr_long[5], record->dst_address.addr_long[4], record->dst_address.addr_long[3], record->dst_address.addr_long[2], record->dst_address.addr_long[1], record->dst_address.addr_long[0]); 
 
       zb_device_params_t *device = (zb_device_params_t *)calloc(1, sizeof(zb_device_params_t));
       device->endpoint = record->dst_endp;
@@ -421,12 +419,13 @@ void ZigbeeCore::bindingTableCb(const esp_zb_zdo_binding_table_info_t *table_inf
       } else { //ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT
         memcpy(device->ieee_addr, record->dst_address.addr_long, sizeof(esp_zb_ieee_addr_t));
       }
-      log_d("Bound device: endpoint %d, short address 0x%04x to endpoint %d", record->dst_endp, record->dst_address.addr_short, record->src_endp);
 
       // Add to list of bound devices of proper endpoint
       for (std::list<ZigbeeEP *>::iterator it = Zigbee.ep_objects.begin(); it != Zigbee.ep_objects.end(); ++it) {
         if ((*it)->getEndpoint() == record->src_endp) {
           (*it)->addBoundDevice(device);
+          log_d("Device bound to EP %d -> device endpoint: %d, short addr: 0x%04x, ieee addr: %02X:%02X:%02X:%02X:%02X:%02X:%02X:%02X", record->src_endp, device->endpoint, device->short_addr, 
+          device->ieee_addr[7], device->ieee_addr[6], device->ieee_addr[5], device->ieee_addr[4], device->ieee_addr[3], device->ieee_addr[2], device->ieee_addr[1], device->ieee_addr[0]);
         }
       }
       record = record->next;

Diff for: libraries/Zigbee/src/ZigbeeEP.cpp

@@ -139,7 +139,9 @@ char *ZigbeeEP::readManufacturer(uint8_t endpoint, uint16_t short_addr, esp_zb_i
   // clear read manufacturer
   _read_manufacturer = nullptr;
 
+  esp_zb_lock_acquire(portMAX_DELAY);
   esp_zb_zcl_read_attr_cmd_req(&read_req);
+  esp_zb_lock_release();
 
   //Wait for response or timeout
   if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) {
@@ -173,22 +175,32 @@ char *ZigbeeEP::readModel(uint8_t endpoint, uint16_t short_addr, esp_zb_ieee_add
   // clear read model
   _read_model = nullptr;
 
+  esp_zb_lock_acquire(portMAX_DELAY);
   esp_zb_zcl_read_attr_cmd_req(&read_req);
+  esp_zb_lock_release();
 
   //Wait for response or timeout
-  //Semaphore take
   if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) {
     log_e("Error while reading model");
   }
   return _read_model;
 }
 
-void ZigbeeEP::printBoundDevices() {
-  log_i("Bound devices:");
-  for ([[maybe_unused]]
-       const auto &device : _bound_devices) {
-    log_i("Device on endpoint %d, short address: 0x%x", device->endpoint, device->short_addr);
-    print_ieee_addr(device->ieee_addr);
+void ZigbeeEP::printBoundDevices(Print *print) {
+  if (print == nullptr) {
+    log_i("Bound devices:");
+    for ([[maybe_unused]]
+    const auto &device : _bound_devices) {
+      log_i("Device on endpoint %d, short address: 0x%x, ieee address: %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", device->endpoint, device->short_addr, 
+      device->ieee_addr[7], device->ieee_addr[6], device->ieee_addr[5], device->ieee_addr[4], device->ieee_addr[3], device->ieee_addr[2], device->ieee_addr[1], device->ieee_addr[0]);
+      }
+  } else {
+    print->println("Bound devices:");
+    for ([[maybe_unused]]
+    const auto &device : _bound_devices) {
+      print->printf("Device on endpoint %d, short address: 0x%x, ieee address: %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x\r\n", device->endpoint, device->short_addr, 
+      device->ieee_addr[7], device->ieee_addr[6], device->ieee_addr[5], device->ieee_addr[4], device->ieee_addr[3], device->ieee_addr[2], device->ieee_addr[1], device->ieee_addr[0]);
+    }
   }
 }
 

Diff for: libraries/Zigbee/src/ZigbeeEP.h

@@ -9,8 +9,6 @@
 
 /* Useful defines */
 #define ZB_ARRAY_LENTH(arr) (sizeof(arr) / sizeof(arr[0]))
-#define print_ieee_addr(addr) \
-  log_i("IEEE Address: %02X:%02X:%02X:%02X:%02X:%02X:%02X:%02X", addr[0], addr[1], addr[2], addr[3], addr[4], addr[5], addr[6], addr[7])
 #define XYZ_TO_RGB(X, Y, Z, r, g, b)                                \
   {                                                                 \
     r = (float)(3.240479 * (X) - 1.537150 * (Y) - 0.498535 * (Z));  \
@@ -68,7 +66,7 @@ class ZigbeeEP {
     return _endpoint;
   }
 
-  void printBoundDevices();
+  void printBoundDevices(Print *print = nullptr);
   std::list<zb_device_params_t *> getBoundDevices() const {
     return _bound_devices;
   }
@@ -128,7 +126,6 @@ class ZigbeeEP {
     _bound_devices.push_back(device);
     _is_bound = true;
   }
-  
   friend class ZigbeeCore;
 };
 

Diff for: libraries/Zigbee/src/ep/ZigbeeColorDimmerSwitch.cpp

@@ -99,7 +99,9 @@ void ZigbeeColorDimmerSwitch::lightToggle() {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_DST_ADDR_ENDP_NOT_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_TOGGLE_ID;
     log_v("Sending 'light toggle' command");
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -113,7 +115,9 @@ void ZigbeeColorDimmerSwitch::lightToggle(uint16_t group_addr) {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_GROUP_ENDP_NOT_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_TOGGLE_ID;
     log_v("Sending 'light toggle' command to group address 0x%x", group_addr);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -128,7 +132,9 @@ void ZigbeeColorDimmerSwitch::lightToggle(uint8_t endpoint, uint16_t short_addr)
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_TOGGLE_ID;
     log_v("Sending 'light toggle' command to endpoint %d, address 0x%x", endpoint, short_addr);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -144,7 +150,9 @@ void ZigbeeColorDimmerSwitch::lightToggle(uint8_t endpoint, esp_zb_ieee_addr_t i
     memcpy(cmd_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t));
     log_v("Sending 'light toggle' command to endpoint %d, ieee address %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", 
     endpoint, ieee_addr[7], ieee_addr[6], ieee_addr[5], ieee_addr[4], ieee_addr[3], ieee_addr[2], ieee_addr[1], ieee_addr[0]);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -157,7 +165,9 @@ void ZigbeeColorDimmerSwitch::lightOn() {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_DST_ADDR_ENDP_NOT_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_ON_ID;
     log_v("Sending 'light on' command");
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -171,7 +181,9 @@ void ZigbeeColorDimmerSwitch::lightOn(uint16_t group_addr) {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_GROUP_ENDP_NOT_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_ON_ID;
     log_v("Sending 'light on' command to group address 0x%x", group_addr);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -186,7 +198,9 @@ void ZigbeeColorDimmerSwitch::lightOn(uint8_t endpoint, uint16_t short_addr) {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_ON_ID;
     log_v("Sending 'light on' command to endpoint %d, address 0x%x", endpoint, short_addr);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -202,7 +216,9 @@ void ZigbeeColorDimmerSwitch::lightOn(uint8_t endpoint, esp_zb_ieee_addr_t ieee_
     memcpy(cmd_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t));
     log_v("Sending 'light on' command to endpoint %d, ieee address %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", 
     endpoint, ieee_addr[7], ieee_addr[6], ieee_addr[5], ieee_addr[4], ieee_addr[3], ieee_addr[2], ieee_addr[1], ieee_addr[0]);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -215,7 +231,9 @@ void ZigbeeColorDimmerSwitch::lightOff() {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_DST_ADDR_ENDP_NOT_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_OFF_ID;
     log_v("Sending 'light off' command");
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -229,7 +247,9 @@ void ZigbeeColorDimmerSwitch::lightOff(uint16_t group_addr) {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_GROUP_ENDP_NOT_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_OFF_ID;
     log_v("Sending 'light off' command to group address 0x%x", group_addr);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -244,7 +264,9 @@ void ZigbeeColorDimmerSwitch::lightOff(uint8_t endpoint, uint16_t short_addr) {
     cmd_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT;
     cmd_req.on_off_cmd_id = ESP_ZB_ZCL_CMD_ON_OFF_OFF_ID;
     log_v("Sending 'light off' command to endpoint %d, address 0x%x", endpoint, short_addr);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }
@@ -260,7 +282,9 @@ void ZigbeeColorDimmerSwitch::lightOff(uint8_t endpoint, esp_zb_ieee_addr_t ieee
     memcpy(cmd_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t));
     log_v("Sending 'light off' command to endpoint %d, ieee address %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", 
     endpoint, ieee_addr[7], ieee_addr[6], ieee_addr[5], ieee_addr[4], ieee_addr[3], ieee_addr[2], ieee_addr[1], ieee_addr[0]);
+    esp_zb_lock_acquire(portMAX_DELAY);
     esp_zb_zcl_on_off_cmd_req(&cmd_req);
+    esp_zb_lock_release();
   } else {
     log_e("Light not bound");
   }