Reduce RAM usage from CHIPDeviceEvent

- CHIPDeviceEvent has had a historical `InternetConnectivityChange` event that embeds a 46 byte string (instead of a 16 byte IPAddress) and is *only used for logging* in examples.= - This event is in a union and is the largest of all the sub-structs, therefore forcing max-sized events to that event. Issue project-chip#9261 This PR: - Changes the `address` from a string to an IPAddress - Renames to `ipAddress` to break external clients using this struct so that they notice on the next roll to master (usage change is trivial). - Updates all examples to no longer log the address, since it was only logged for IPv4 and not IPv6, and IPv6 is the standard, so if it was good enough for IPv6 not to log, it's good enough for IPv4. - Make IPAddress trivially copyable by removing a non-trivial `operator=` that was actually implementing the trivial copy. Upstream OpenWeave code that was the basis for the IPAddress.h file removed it already in late 2021, > 1.5 years after import (see openweave/openweave-core@fbbb01c#diff-33121ed998c085b8dc0026f30f24aaadb8b8b36042e38b449cec15c32c8ba86e) - Update all platforms that populated the address to use the actual address, not the string Testing done: - All cert tests pass - All unit tests pass
tcarmelveilleux · May 2, 2022 · ad70dac · ad70dac
1 parent 0b22dad
commit ad70dac
Show file tree

Hide file tree

Showing 22 changed files with 49 additions and 47 deletions.
diff --git a/examples/all-clusters-app/ameba/main/DeviceCallbacks.cpp b/examples/all-clusters-app/ameba/main/DeviceCallbacks.cpp
@@ -99,7 +99,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ChipLogProgress(DeviceLayer, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ChipLogProgress(DeviceLayer, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
     }
     else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/examples/all-clusters-app/esp32/main/DeviceCallbacks.cpp b/examples/all-clusters-app/esp32/main/DeviceCallbacks.cpp
@@ -212,7 +212,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
     static bool isOTAInitialized = false;
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ESP_LOGI(TAG, "IPv4 Server ready...");
         wifiLED.Set(true);
         chip::app::DnssdServer::Instance().StartServer();
 

diff --git a/examples/bridge-app/esp32/main/DeviceCallbacks.cpp b/examples/bridge-app/esp32/main/DeviceCallbacks.cpp
@@ -73,7 +73,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ESP_LOGI(TAG, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
     }
     else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/examples/chef/esp32/main/main.cpp b/examples/chef/esp32/main/main.cpp
@@ -78,7 +78,7 @@ void DeviceEventCallback(const ChipDeviceEvent * event, intptr_t arg)
     case DeviceEventType::kInternetConnectivityChange:
         if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
         {
-            ChipLogProgress(Shell, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+            ChipLogProgress(Shell, "IPv4 Server ready...");
             chip::app::DnssdServer::Instance().StartServer();
         }
         else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/examples/lighting-app/ameba/main/DeviceCallbacks.cpp b/examples/lighting-app/ameba/main/DeviceCallbacks.cpp
@@ -78,7 +78,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        printf("Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        printf("IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
     }
     else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/examples/lighting-app/bouffalolab/bl602/src/DeviceCallbacks.cpp b/examples/lighting-app/bouffalolab/bl602/src/DeviceCallbacks.cpp
@@ -129,7 +129,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        log_info("Server ready at: %s:%d\r\n", event->InternetConnectivityChange.address, CHIP_PORT);
+        log_info("IPv4 Server ready...\r\n");
         // TODO
         // wifiLED.Set(true);
         chip::app::DnssdServer::Instance().StartServer();

diff --git a/examples/lighting-app/esp32/main/DeviceCallbacks.cpp b/examples/lighting-app/esp32/main/DeviceCallbacks.cpp
@@ -161,7 +161,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
     static bool isOTAInitialized = false;
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ESP_LOGI(TAG, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
 
         if (!isOTAInitialized)

diff --git a/examples/lock-app/esp32/main/DeviceCallbacks.cpp b/examples/lock-app/esp32/main/DeviceCallbacks.cpp
@@ -100,7 +100,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ESP_LOGI(TAG, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
     }
     else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/examples/ota-provider-app/esp32/main/DeviceCallbacks.cpp b/examples/ota-provider-app/esp32/main/DeviceCallbacks.cpp
@@ -82,7 +82,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ESP_LOGI(TAG, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
     }
     else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/examples/ota-requestor-app/ameba/main/DeviceCallbacks.cpp b/examples/ota-requestor-app/ameba/main/DeviceCallbacks.cpp
@@ -99,7 +99,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ChipLogProgress(DeviceLayer, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ChipLogProgress(DeviceLayer, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
     }
     else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/examples/ota-requestor-app/esp32/main/DeviceCallbacks.cpp b/examples/ota-requestor-app/esp32/main/DeviceCallbacks.cpp
@@ -90,7 +90,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
     static bool isOTAInitialized = false;
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ESP_LOGI(TAG, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
         if (!isOTAInitialized)
         {

diff --git a/examples/temperature-measurement-app/esp32/main/DeviceCallbacks.cpp b/examples/temperature-measurement-app/esp32/main/DeviceCallbacks.cpp
@@ -88,7 +88,7 @@ void DeviceCallbacks::OnInternetConnectivityChange(const ChipDeviceEvent * event
 {
     if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established)
     {
-        ESP_LOGI(TAG, "Server ready at: %s:%d", event->InternetConnectivityChange.address, CHIP_PORT);
+        ESP_LOGI(TAG, "IPv4 Server ready...");
         chip::app::DnssdServer::Instance().StartServer();
     }
     else if (event->InternetConnectivityChange.IPv4 == kConnectivity_Lost)

diff --git a/src/include/platform/CHIPDeviceEvent.h b/src/include/platform/CHIPDeviceEvent.h
@@ -25,6 +25,7 @@
 #pragma once
 #include <stdint.h>
 
+#include <inet/IPAddress.h>
 #include <lib/core/DataModelTypes.h>
 
 namespace chip {
@@ -373,7 +374,14 @@ struct ChipDeviceEvent final
         {
             ConnectivityChange IPv4;
             ConnectivityChange IPv6;
-            char address[INET6_ADDRSTRLEN];
+            // WARNING: There used to be `char address[INET6_ADDRSTRLEN]` here and it is
+            //          deprecated/removed since it was too large and only used for logging.
+            //          Consider not relying on ipAddress field either since the platform
+            //          layer *does not actually validate* that the actual internet is reachable
+            //          before issuing this event *and* there may be multiple addresses
+            //          (especially IPv6) so it's recommended to use `ChipDevicePlatformEvent`
+            //          instead and do something that is better for your platform.
+            chip::Inet::IPAddress ipAddress;
         } InternetConnectivityChange;
         struct
         {

diff --git a/src/inet/IPAddress.cpp b/src/inet/IPAddress.cpp
@@ -57,19 +57,6 @@ bool IPAddress::operator!=(const IPAddress & other) const
     return Addr[0] != other.Addr[0] || Addr[1] != other.Addr[1] || Addr[2] != other.Addr[2] || Addr[3] != other.Addr[3];
 }
 
-IPAddress & IPAddress::operator=(const IPAddress & other)
-{
-    if (this != &other)
-    {
-        Addr[0] = other.Addr[0];
-        Addr[1] = other.Addr[1];
-        Addr[2] = other.Addr[2];
-        Addr[3] = other.Addr[3];
-    }
-
-    return *this;
-}
-
 #if CHIP_SYSTEM_CONFIG_USE_LWIP && !CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_ENDPOINT
 
 IPAddress::IPAddress(const ip6_addr_t & ipv6Addr)

diff --git a/src/inet/IPAddress.h b/src/inet/IPAddress.h
@@ -120,6 +120,7 @@ union SockAddr
  * @details
  *  The CHIP Inet Layer uses objects of this class to represent Internet
  *  protocol addresses (independent of protocol version).
+ *
  */
 class DLL_EXPORT IPAddress
 {
@@ -324,15 +325,6 @@ class DLL_EXPORT IPAddress
      */
     bool operator!=(const IPAddress & other) const;
 
-    /**
-     * @brief   Conventional assignment operator.
-     *
-     * @param[in]   other   The address to copy.
-     *
-     * @return  A reference to this object.
-     */
-    IPAddress & operator=(const IPAddress & other);
-
     /**
      * @brief   Emit the IP address in conventional text presentation format.
      *
@@ -667,5 +659,7 @@ class DLL_EXPORT IPAddress
     static IPAddress Any;
 };
 
+static_assert(std::is_trivial<IPAddress>::value, "IPAddress is not trivial");
+
 } // namespace Inet
 } // namespace chip
diff --git a/src/platform/Ameba/ConnectivityManagerImpl.cpp b/src/platform/Ameba/ConnectivityManagerImpl.cpp
@@ -702,7 +702,8 @@ void ConnectivityManagerImpl::UpdateInternetConnectivityState(void)
         event.Type                            = DeviceEventType::kInternetConnectivityChange;
         event.InternetConnectivityChange.IPv4 = GetConnectivityChange(hadIPv4Conn, haveIPv4Conn);
         event.InternetConnectivityChange.IPv6 = GetConnectivityChange(hadIPv6Conn, haveIPv6Conn);
-        addr.ToString(event.InternetConnectivityChange.address);
+        event.InternetConnectivityChange.ipAddress = addr;
+
         PlatformMgr().PostEventOrDie(&event);
 
         if (haveIPv4Conn != hadIPv4Conn)

diff --git a/src/platform/EFR32/ConnectivityManagerImpl_WIFI.cpp b/src/platform/EFR32/ConnectivityManagerImpl_WIFI.cpp
@@ -453,7 +453,8 @@ void ConnectivityManagerImpl::UpdateInternetConnectivityState(void)
         event.Type                            = DeviceEventType::kInternetConnectivityChange;
         event.InternetConnectivityChange.IPv4 = GetConnectivityChange(hadIPv4Conn, haveIPv4Conn);
         event.InternetConnectivityChange.IPv6 = GetConnectivityChange(hadIPv6Conn, haveIPv6Conn);
-        addr.ToString(event.InternetConnectivityChange.address, sizeof(event.InternetConnectivityChange.address));
+        event.InternetConnectivityChange.ipAddress = addr;
+
         (void) PlatformMgr().PostEvent(&event);
 
         if (haveIPv4Conn != hadIPv4Conn)

diff --git a/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp b/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp
@@ -1014,7 +1014,8 @@ void ConnectivityManagerImpl::UpdateInternetConnectivityState(void)
         event.Type                            = DeviceEventType::kInternetConnectivityChange;
         event.InternetConnectivityChange.IPv4 = GetConnectivityChange(hadIPv4Conn, haveIPv4Conn);
         event.InternetConnectivityChange.IPv6 = GetConnectivityChange(hadIPv6Conn, haveIPv6Conn);
-        addr.ToString(event.InternetConnectivityChange.address);
+        event.InternetConnectivityChange.ipAddress = addr;
+
         PlatformMgr().PostEventOrDie(&event);
 
         if (haveIPv4Conn != hadIPv4Conn)

diff --git a/src/platform/Linux/ConnectivityManagerImpl.cpp b/src/platform/Linux/ConnectivityManagerImpl.cpp
@@ -1184,10 +1184,12 @@ void ConnectivityManagerImpl::PostNetworkConnect()
                 event.Type                            = DeviceEventType::kInternetConnectivityChange;
                 event.InternetConnectivityChange.IPv4 = kConnectivity_Established;
                 event.InternetConnectivityChange.IPv6 = kConnectivity_NoChange;
-                addr.ToString(event.InternetConnectivityChange.address);
+                event.InternetConnectivityChange.ipAddress = addr;
 
-                ChipLogDetail(DeviceLayer, "Got IP address on interface: %s IP: %s", ifName,
-                              event.InternetConnectivityChange.address);
+                char ipStrBuf[chip::Inet::IPAddress::kMaxStringLength] = { 0 };
+                addr.ToString(ipStrBuf);
+
+                ChipLogDetail(DeviceLayer, "Got IP address on interface: %s IP: %s", ifName, ipStrBuf);
 
                 PlatformMgr().PostEventOrDie(&event);
             }

diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp
@@ -149,15 +149,15 @@ void PlatformManagerImpl::WiFIIPChangeListener()
                             continue;
                         }
 
+                        char ipStrBuf[chip::Inet::IPAddress::kMaxStringLength] = { 0 };
+                        inet_ntop(AF_INET, RTA_DATA(routeInfo), ipStrBuf, sizeof(ipStrBuf));
+                        ChipLogDetail(DeviceLayer, "Got IP address on interface: %s IP: %s", name, ipStrBuf);
+
                         ChipDeviceEvent event;
                         event.Type                            = DeviceEventType::kInternetConnectivityChange;
                         event.InternetConnectivityChange.IPv4 = kConnectivity_Established;
                         event.InternetConnectivityChange.IPv6 = kConnectivity_NoChange;
-                        inet_ntop(AF_INET, RTA_DATA(routeInfo), event.InternetConnectivityChange.address,
-                                  sizeof(event.InternetConnectivityChange.address));
-
-                        ChipLogDetail(DeviceLayer, "Got IP address on interface: %s IP: %s", name,
-                                      event.InternetConnectivityChange.address);
+                        VerifyOrDie(chip::Inet::IPAddress::FromString(ipStrBuf, event.InternetConnectivityChange.ipAddress));
 
                         CHIP_ERROR status = PlatformMgr().PostEvent(&event);
                         if (status != CHIP_NO_ERROR)

diff --git a/src/platform/P6/ConnectivityManagerImpl.cpp b/src/platform/P6/ConnectivityManagerImpl.cpp
@@ -660,7 +660,7 @@ void ConnectivityManagerImpl::UpdateInternetConnectivityState(void)
         event.Type                            = DeviceEventType::kInternetConnectivityChange;
         event.InternetConnectivityChange.IPv4 = GetConnectivityChange(hadIPv4Conn, haveIPv4Conn);
         event.InternetConnectivityChange.IPv6 = GetConnectivityChange(hadIPv6Conn, haveIPv6Conn);
-        addr.ToString(event.InternetConnectivityChange.address);
+        event.InternetConnectivityChange.ipAddress = addr;
         PlatformMgr().PostEventOrDie(&event);
 
         if (haveIPv4Conn != hadIPv4Conn)

diff --git a/src/platform/mbed/NetworkCommissioningWiFiDriver.cpp b/src/platform/mbed/NetworkCommissioningWiFiDriver.cpp
@@ -442,6 +442,7 @@ void WiFiDriverImpl::OnNetworkConnected()
             event.Type                            = DeviceEventType::kInternetConnectivityChange;
             event.InternetConnectivityChange.IPv4 = kConnectivity_Lost;
             event.InternetConnectivityChange.IPv6 = kConnectivity_NoChange;
+            event.InternetConnectivityChange.ipAddress = mIp4Address;
             ConnectivityMgrImpl().PostEvent(&event, true);
             ChipLogError(DeviceLayer, "Unexpected loss of Ip4 address");
         }
@@ -454,6 +455,7 @@ void WiFiDriverImpl::OnNetworkConnected()
             event.Type                            = DeviceEventType::kInternetConnectivityChange;
             event.InternetConnectivityChange.IPv4 = kConnectivity_NoChange;
             event.InternetConnectivityChange.IPv6 = kConnectivity_Lost;
+            event.InternetConnectivityChange.ipAddress = mIp6Address;
             ConnectivityMgrImpl().PostEvent(&event, true);
             ChipLogError(DeviceLayer, "Unexpected loss of Ip6 address");
         }
@@ -470,6 +472,7 @@ void WiFiDriverImpl::OnNetworkConnected()
                 event.Type                            = DeviceEventType::kInternetConnectivityChange;
                 event.InternetConnectivityChange.IPv4 = kConnectivity_Established;
                 event.InternetConnectivityChange.IPv6 = kConnectivity_NoChange;
+                event.InternetConnectivityChange.ipAddress = mIp4Address;
                 ConnectivityMgrImpl().PostEvent(&event, true);
                 ChipLogProgress(DeviceLayer, "New Ip4 address set: %s", address.get_ip_address());
             }
@@ -485,6 +488,7 @@ void WiFiDriverImpl::OnNetworkConnected()
                     event.Type                            = DeviceEventType::kInternetConnectivityChange;
                     event.InternetConnectivityChange.IPv4 = kConnectivity_NoChange;
                     event.InternetConnectivityChange.IPv6 = kConnectivity_Lost;
+                    event.InternetConnectivityChange.ipAddress = mIp6Address;
                     ConnectivityMgrImpl().PostEvent(&event, true);
                     ChipLogError(DeviceLayer, "Unexpected loss of Ip6 address");
                 }
@@ -498,6 +502,7 @@ void WiFiDriverImpl::OnNetworkConnected()
                     event.Type                            = DeviceEventType::kInternetConnectivityChange;
                     event.InternetConnectivityChange.IPv4 = kConnectivity_NoChange;
                     event.InternetConnectivityChange.IPv6 = kConnectivity_Established;
+                    event.InternetConnectivityChange.ipAddress = mIp6Address;
                     ConnectivityMgrImpl().PostEvent(&event, true);
                     ChipLogProgress(DeviceLayer, "New Ip6 address set %s", address.get_ip_address());
                 }
@@ -512,6 +517,7 @@ void WiFiDriverImpl::OnNetworkConnected()
                 event.Type                            = DeviceEventType::kInternetConnectivityChange;
                 event.InternetConnectivityChange.IPv4 = kConnectivity_NoChange;
                 event.InternetConnectivityChange.IPv6 = kConnectivity_Established;
+                event.InternetConnectivityChange.ipAddress = mIp6Address;
                 ConnectivityMgrImpl().PostEvent(&event, true);
                 ChipLogProgress(DeviceLayer, "New Ip6 address set %s", address.get_ip_address());
             }
@@ -537,6 +543,7 @@ void WiFiDriverImpl::OnNetworkDisconnected()
         event.Type                            = DeviceEventType::kInternetConnectivityChange;
         event.InternetConnectivityChange.IPv4 = kConnectivity_Lost;
         event.InternetConnectivityChange.IPv6 = kConnectivity_NoChange;
+        event.InternetConnectivityChange.ipAddress = mIp4Address;
         ConnectivityMgrImpl().PostEvent(&event, true);
         ChipLogError(DeviceLayer, "Loss of Ip4 address");
     }
@@ -549,6 +556,7 @@ void WiFiDriverImpl::OnNetworkDisconnected()
         event.Type                            = DeviceEventType::kInternetConnectivityChange;
         event.InternetConnectivityChange.IPv4 = kConnectivity_NoChange;
         event.InternetConnectivityChange.IPv6 = kConnectivity_Lost;
+        event.InternetConnectivityChange.ipAddress = mIp6Address;
         ConnectivityMgrImpl().PostEvent(&event, true);
         ChipLogError(DeviceLayer, "Loss of Ip6 address");
     }