fix review comments

project-chip · Nov 11, 2021 · 602d930 · 602d930
1 parent 2870738
commit 602d930
Show file tree

Hide file tree

Showing 12 changed files with 53 additions and 30 deletions.
diff --git a/.github/workflows/examples-esp32.yaml b/.github/workflows/examples-esp32.yaml
@@ -38,7 +38,7 @@ jobs:
         if: github.actor != 'restyled-io[bot]'
 
         container:
-            image: connectedhomeip/chip-build-esp32:0.5.25
+            image: connectedhomeip/chip-build-esp32:0.5.27
             volumes:
                 - "/tmp/bloat_reports:/tmp/bloat_reports"
 

diff --git a/.github/workflows/qemu.yaml b/.github/workflows/qemu.yaml
@@ -34,7 +34,7 @@ jobs:
         if: github.actor != 'restyled-io[bot]'
 
         container:
-            image: connectedhomeip/chip-build-esp32-qemu:0.5.25
+            image: connectedhomeip/chip-build-esp32-qemu:0.5.27
             volumes:
                 - "/tmp/log_output:/tmp/test_logs"
 

diff --git a/.github/workflows/release_artifacts.yaml b/.github/workflows/release_artifacts.yaml
@@ -29,7 +29,7 @@ jobs:
         runs-on: ubuntu-latest
 
         container:
-            image: connectedhomeip/chip-build-esp32:0.5.25
+            image: connectedhomeip/chip-build-esp32:0.5.27
 
         steps:
             - name: Checkout

diff --git a/config/esp32/components/chip/component.mk b/config/esp32/components/chip/component.mk
@@ -99,8 +99,8 @@ COMPONENT_ADD_INCLUDEDIRS +=   $(REL_OUTPUT_DIR)/src/include \
 
 # Tell the ESP-IDF build system that the CHIP component defines its own build
 # and clean targets.
-COMPONENT_OWNBUILDTARGET 	 = 1
-COMPONENT_OWNCLEANTARGET 	 = 1
+COMPONENT_OWNBUILDTARGET 	 := chip_build
+COMPONENT_OWNCLEANTARGET 	 := chip_clean
 
 is_debug ?= true
 
@@ -156,10 +156,10 @@ endif
 	cd $(COMPONENT_PATH); ninja $(subst 1,-v,$(filter 1,$(V))) -C $(OUTPUT_DIR) esp32
 
 
-build : install-chip
+chip_build : install-chip
 	echo "CHIP built and installed..."
 	cp -a ${OUTPUT_DIR}/lib/libCHIP.a ${OUTPUT_DIR}/libchip.a
 
-clean:
+chip_clean:
 	echo "RM $(OUTPUT_DIR)"
 	rm -rf $(OUTPUT_DIR)
diff --git a/examples/all-clusters-app/esp32/main/DeviceCallbacks.cpp b/examples/all-clusters-app/esp32/main/DeviceCallbacks.cpp
@@ -28,6 +28,8 @@
 #include "Globals.h"
 #include "LEDWidget.h"
 #include "WiFiWidget.h"
+#include "esp_check.h"
+#include "esp_err.h"
 #include "esp_heap_caps.h"
 #include "esp_log.h"
 #include "route_hook/esp_route_hook.h"
@@ -86,7 +88,7 @@ void DeviceCallbacks::DeviceEventCallback(const ChipDeviceEvent * event, intptr_
         }
         if (event->InterfaceIpAddressChanged.Type == InterfaceIpChangeType::kIpV6_Assigned)
         {
-            esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
+            ESP_ERROR_CHECK(esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF")));
         }
         break;
     }

diff --git a/examples/all-clusters-app/esp32/sdkconfig_c3devkit.defaults b/examples/all-clusters-app/esp32/sdkconfig_c3devkit.defaults
@@ -40,3 +40,6 @@ CONFIG_LWIP_IPV6_AUTOCONFIG=y
 CONFIG_PARTITION_TABLE_CUSTOM=y
 CONFIG_PARTITION_TABLE_FILENAME="partitions.csv"
 
+#enable lwIP route hooks
+CONFIG_LWIP_HOOK_IP6_ROUTE_DEFAULT=y
+CONFIG_LWIP_HOOK_ND6_GET_GW_DEFAULT=y
diff --git a/examples/all-clusters-app/esp32/sdkconfig_m5stack.defaults b/examples/all-clusters-app/esp32/sdkconfig_m5stack.defaults
@@ -50,3 +50,7 @@ CONFIG_ESP_MAIN_TASK_STACK_SIZE=5120
 
 #enable debug shell
 CONFIG_ENABLE_CHIP_SHELL=y
+
+#enable lwIP route hooks
+CONFIG_LWIP_HOOK_IP6_ROUTE_DEFAULT=y
+CONFIG_LWIP_HOOK_ND6_GET_GW_DEFAULT=y
diff --git a/examples/bridge-app/esp32/main/DeviceCallbacks.cpp b/examples/bridge-app/esp32/main/DeviceCallbacks.cpp
@@ -17,6 +17,8 @@
  */
 
 #include "DeviceCallbacks.h"
+#include "esp_check.h"
+#include "esp_err.h"
 #include "esp_heap_caps.h"
 #include "esp_log.h"
 #include "route_hook/esp_route_hook.h"
@@ -82,7 +84,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
     {
         ESP_LOGI(TAG, "IPv6 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
-        esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
+        ESP_ERROR_CHECK(esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF")));
     }
     else if (event->InternetConnectivityChange.IPv6 == kConnectivity_Lost)
     {

diff --git a/examples/lock-app/esp32/main/DeviceCallbacks.cpp b/examples/lock-app/esp32/main/DeviceCallbacks.cpp
@@ -27,6 +27,8 @@
 #include "AppConfig.h"
 #include "BoltLockManager.h"
 
+#include "esp_check.h"
+#include "esp_err.h"
 #include "esp_heap_caps.h"
 #include "esp_log.h"
 #include "route_hook/esp_route_hook.h"
@@ -66,7 +68,7 @@ void DeviceCallbacks::DeviceEventCallback(const ChipDeviceEvent * event, intptr_
         }
         if (event->InterfaceIpAddressChanged.Type == InterfaceIpChangeType::kIpV6_Assigned)
         {
-            esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
+            ESP_ERROR_CHECK(esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF")));
         }
         break;
     }

diff --git a/examples/platform/esp32/route_hook/esp_route_hook.c b/examples/platform/esp32/route_hook/esp_route_hook.c
@@ -28,12 +28,12 @@
 
 #define TAG "ROUTE_HOOK"
 
-typedef struct esp_route_hook
+typedef struct esp_route_hook_t
 {
     struct netif * netif;
     struct raw_pcb * pcb;
-    struct esp_route_hook * next;
-} esp_route_hook;
+    struct esp_route_hook_t * next;
+} esp_route_hook_t;
 
 PACK_STRUCT_BEGIN
 struct rio_header_t
@@ -48,7 +48,7 @@ PACK_STRUCT_END
 
 typedef struct rio_header_t rio_header_t;
 
-static esp_route_hook * s_hooks;
+static esp_route_hook_t * s_hooks;
 
 static bool is_self_address(struct netif * netif, const ip6_addr_t * addr)
 {
@@ -80,11 +80,18 @@ static void ra_recv_handler(struct netif * netif, const uint8_t * icmp_payload,
         if (opt_type == ND6_OPTION_TYPE_ROUTE_INFO && opt_len >= sizeof(rio_header_t) && !is_self_address(netif, src_addr) &&
             payload_len >= opt_len)
         {
-            rio_header_t * rio_header = (rio_header_t *) icmp_payload;
-            uint8_t prefix_len_bytes  = (rio_header->prefix_length + 7) / 8;
-            int8_t preference         = -2 * ((rio_header->preference >> 4) & 1) + (((rio_header->preference) >> 3) & 1);
-            const uint8_t * rio_data  = &icmp_payload[sizeof(rio_header_t)];
-            uint8_t rio_data_len      = opt_len - sizeof(rio_header_t);
+            rio_header_t rio_header;
+            memcpy(&rio_header, icmp_payload, sizeof(rio_header));
+
+            // skip if prefix is longer than IPv6 address.
+            if (rio_header.prefix_length > 128)
+            {
+                break;
+            }
+            uint8_t prefix_len_bytes = (rio_header.prefix_length + 7) / 8;
+            int8_t preference        = -2 * ((rio_header.preference >> 4) & 1) + (((rio_header.preference) >> 3) & 1);
+            const uint8_t * rio_data = &icmp_payload[sizeof(rio_header_t)];
+            uint8_t rio_data_len     = opt_len - sizeof(rio_header_t);
 
             ESP_LOGI(TAG, "Received RIO");
             for (; rio_data_len >= prefix_len_bytes; rio_data_len -= prefix_len_bytes, rio_data += prefix_len_bytes)
@@ -96,10 +103,10 @@ static void ra_recv_handler(struct netif * netif, const uint8_t * icmp_payload,
                 memcpy(&prefix.addr, rio_data, prefix_len_bytes);
                 route.netif            = netif;
                 route.gateway          = *src_addr;
-                route.prefix_length    = rio_header->prefix_length;
+                route.prefix_length    = rio_header.prefix_length;
                 route.prefix           = prefix;
                 route.preference       = preference;
-                route.lifetime_seconds = lwip_ntohl(rio_header->route_lifetime);
+                route.lifetime_seconds = lwip_ntohl(rio_header.route_lifetime);
                 ESP_LOGI(TAG, "prefix %s lifetime %u\n", ip6addr_ntoa(&prefix), route.lifetime_seconds);
                 if (esp_route_table_add_route_entry(&route) == NULL)
                 {
@@ -120,7 +127,7 @@ static uint8_t icmp6_raw_recv_handler(void * arg, struct raw_pcb * pcb, struct p
     struct icmp6_hdr * icmp6_header;
     ip6_addr_t src;
     ip6_addr_t dest;
-    esp_route_hook * hook = (esp_route_hook *) arg;
+    esp_route_hook_t * hook = (esp_route_hook_t *) arg;
 
     memcpy(src.addr, ip6_header->src.addr, sizeof(src.addr));
     memcpy(dest.addr, ip6_header->dest.addr, sizeof(dest.addr));
@@ -157,15 +164,15 @@ static uint8_t icmp6_raw_recv_handler(void * arg, struct raw_pcb * pcb, struct p
 esp_err_t esp_route_hook_init(esp_netif_t * netif)
 {
     struct netif * lwip_netif;
-    ip_addr_t router_group = IPADDR6_INIT_HOST(0xFF020000, 0, 0, 0x02);
-    esp_route_hook * hook  = NULL;
-    esp_err_t ret          = ESP_OK;
+    ip_addr_t router_group  = IPADDR6_INIT_HOST(0xFF020000, 0, 0, 0x02);
+    esp_route_hook_t * hook = NULL;
+    esp_err_t ret           = ESP_OK;
 
     ESP_RETURN_ON_FALSE(netif != NULL, ESP_ERR_INVALID_ARG, TAG, "Invalid network interface");
     lwip_netif = netif_get_by_index(esp_netif_get_netif_impl_index(netif));
     ESP_RETURN_ON_FALSE(lwip_netif != NULL, ESP_ERR_INVALID_ARG, TAG, "Invalid network interface");
 
-    for (esp_route_hook * iter = s_hooks; iter != NULL; iter++)
+    for (esp_route_hook_t * iter = s_hooks; iter != NULL; iter++)
     {
         if (iter->netif == lwip_netif)
         {
@@ -174,7 +181,7 @@ esp_err_t esp_route_hook_init(esp_netif_t * netif)
         }
     }
 
-    hook = (esp_route_hook *) malloc(sizeof(esp_route_hook));
+    hook = (esp_route_hook_t *) malloc(sizeof(esp_route_hook_t));
     ESP_RETURN_ON_FALSE(hook != NULL, ESP_ERR_NO_MEM, TAG, "Cannot allocate hook");
 
     ESP_GOTO_ON_FALSE(mld6_joingroup_netif(lwip_netif, ip_2_ip6(&router_group)) == ESP_OK, ESP_FAIL, exit, TAG,

diff --git a/examples/temperature-measurement-app/esp32/main/DeviceCallbacks.cpp b/examples/temperature-measurement-app/esp32/main/DeviceCallbacks.cpp
@@ -24,6 +24,8 @@
  **/
 #include "DeviceCallbacks.h"
 
+#include "esp_check.h"
+#include "esp_err.h"
 #include "esp_heap_caps.h"
 #include "esp_log.h"
 #include "route_hook/esp_route_hook.h"
@@ -62,7 +64,7 @@ void DeviceCallbacks::DeviceEventCallback(const ChipDeviceEvent * event, intptr_
         }
         if (event->InterfaceIpAddressChanged.Type == InterfaceIpChangeType::kIpV6_Assigned)
         {
-            esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
+            ESP_ERROR_CHECK(esp_route_hook_init(esp_netif_get_handle_from_ifkey("WIFI_STA_DEF")));
         }
         break;
     }

diff --git a/scripts/requirements.esp32.txt b/scripts/requirements.esp32.txt
@@ -1,10 +1,11 @@
 setuptools>=21
-click>=5.0
-pyserial>=3.0
+click>=7.0
+pyserial>=3.3
 future>=0.15.2
 cryptography>=2.1.4
 pyparsing>=2.0.3,<2.4.0
 pyelftools>=0.22
+idf-component-manager>=0.2.99-beta
 gdbgui==0.13.2.0
 pygdbmi<=0.9.0.2
 reedsolo>=1.5.3,<=1.5.4