Match Wake On LAN Cluster to spec (project-chip#30214)

* Add Link local address to wake on lan cluster * zap regen * Fix up wake-on-lan implementation to match spec * zap regen * minor change to kick CI * minor change - fix the revision attribute * restyle * Fix openiot test * Make link local address provisional * Return an error instead of static_assert * Restyle * Remove openthread from linux builds of the tv app * Added prefix for CLUSTER_REVISION_ID --------- Co-authored-by: Andrei Litvin <[email protected]>
tima-q · Nov 8, 2023 · 368b9a0 · 368b9a0
1 parent 155917e
commit 368b9a0
Show file tree

Hide file tree

Showing 31 changed files with 346 additions and 21 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
@@ -4768,7 +4768,7 @@ server cluster RadonConcentrationMeasurement = 1071 {
 
 /** This cluster provides an interface for managing low power mode on a device that supports the Wake On LAN protocol. */
 server cluster WakeOnLan = 1283 {
-  readonly attribute char_string<32> MACAddress = 0;
+  readonly attribute char_string<12> MACAddress = 0;
   readonly attribute command_id generatedCommandList[] = 65528;
   readonly attribute command_id acceptedCommandList[] = 65529;
   readonly attribute event_id eventList[] = 65530;

diff --git a/examples/tv-app/linux/args.gni b/examples/tv-app/linux/args.gni
@@ -31,3 +31,6 @@ chip_enable_additional_data_advertising = true
 chip_enable_rotating_device_id = true
 
 matter_enable_tracing_support = true
+
+# Thread devices do not support WakeOnLan because their mac address is >48bit
+chip_enable_openthread = false
diff --git a/examples/tv-app/tv-common/clusters/wake-on-lan/WakeOnLanManager.cpp b/examples/tv-app/tv-common/clusters/wake-on-lan/WakeOnLanManager.cpp
@@ -23,6 +23,8 @@
 #include <platform/ConfigurationManager.h>
 #include <string>
 
+constexpr const char * kNullHexMACAddress = "000000000000";
+
 using namespace chip;
 using namespace chip::app::Clusters::WakeOnLan;
 
@@ -33,15 +35,15 @@ std::string getMacAddress()
     if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR)
     {
         ChipLogProgress(Zcl, "WakeOnLanManager::getMacAddress no primary MAC configured by DeviceLayer");
-        return "0000000000";
+        return kNullHexMACAddress;
     }
 
     char macStr[chip::DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength * 2 + 1] = { 0 }; // added null char
     if (BytesToHex(&macBuffer[0], sizeof(macBuffer), &macStr[0], sizeof(macBuffer) * 2u, chip::Encoding::HexFlags::kUppercase) !=
         CHIP_NO_ERROR)
     {
         ChipLogProgress(Zcl, "WakeOnLanManager::getMacAddress hex conversion failed");
-        return "0000000000";
+        return kNullHexMACAddress;
     }
 
     return std::string(macStr);
@@ -51,5 +53,13 @@ CHIP_ERROR WakeOnLanManager::HandleGetMacAddress(chip::app::AttributeValueEncode
 {
     ChipLogProgress(Zcl, "WakeOnLanManager::HandleGetMacAddress");
 
+    // Spec REQUIRES 48-bit mac addresses. This means at least Thread devices will
+    // fail here.
+    if (chip::DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength != 6)
+    {
+        ChipLogError(Zcl, "WakeOnLanManager: primary MAC address is not 48-bit");
+        return CHIP_ERROR_BUFFER_TOO_SMALL;
+    }
+
     return aEncoder.Encode(CharSpan::fromCharString(getMacAddress().c_str()));
 }
diff --git a/examples/tv-app/tv-common/tv-app.matter b/examples/tv-app/tv-common/tv-app.matter
@@ -1828,7 +1828,7 @@ server cluster RelativeHumidityMeasurement = 1029 {
 
 /** This cluster provides an interface for managing low power mode on a device that supports the Wake On LAN protocol. */
 server cluster WakeOnLan = 1283 {
-  readonly attribute char_string<32> MACAddress = 0;
+  readonly attribute char_string<12> MACAddress = 0;
   readonly attribute command_id generatedCommandList[] = 65528;
   readonly attribute command_id acceptedCommandList[] = 65529;
   readonly attribute event_id eventList[] = 65530;

diff --git a/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter b/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter
@@ -1347,7 +1347,8 @@ server cluster FixedLabel = 64 {
 
 /** This cluster provides an interface for managing low power mode on a device that supports the Wake On LAN protocol. */
 client cluster WakeOnLan = 1283 {
-  readonly attribute optional char_string<32> MACAddress = 0;
+  readonly attribute optional char_string<12> MACAddress = 0;
+  readonly attribute optional octet_string<16> linkLocalAddress = 1;
   readonly attribute command_id generatedCommandList[] = 65528;
   readonly attribute command_id acceptedCommandList[] = 65529;
   readonly attribute event_id eventList[] = 65530;

diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h
@@ -1182,7 +1182,7 @@
             { ZAP_SIMPLE_DEFAULT(3), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },   /* ClusterRevision */                                 \
                                                                                                                                    \
             /* Endpoint: 1, Cluster: Wake on LAN (server) */                                                                       \
-            { ZAP_EMPTY_DEFAULT(), 0x00000000, 33, ZAP_TYPE(CHAR_STRING), 0 }, /* MACAddress */                                    \
+            { ZAP_EMPTY_DEFAULT(), 0x00000000, 13, ZAP_TYPE(CHAR_STRING), 0 }, /* MACAddress */                                    \
             { ZAP_SIMPLE_DEFAULT(0), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), 0 },   /* FeatureMap */                                    \
             { ZAP_SIMPLE_DEFAULT(1), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },     /* ClusterRevision */                               \
                                                                                                                                    \
@@ -2721,7 +2721,7 @@
       .clusterId = 0x00000503, \
       .attributes = ZAP_ATTRIBUTE_INDEX(536), \
       .attributeCount = 3, \
-      .clusterSize = 39, \
+      .clusterSize = 19, \
       .mask = ZAP_CLUSTER_MASK(SERVER), \
       .functions = NULL, \
       .acceptedCommandList = nullptr, \
@@ -3024,7 +3024,7 @@
 // This is an array of EmberAfEndpointType structures.
 #define GENERATED_ENDPOINT_TYPES                                                                                                   \
     {                                                                                                                              \
-        { ZAP_CLUSTER_INDEX(0), 27, 345 }, { ZAP_CLUSTER_INDEX(27), 44, 3717 }, { ZAP_CLUSTER_INDEX(71), 7, 127 },                 \
+        { ZAP_CLUSTER_INDEX(0), 27, 345 }, { ZAP_CLUSTER_INDEX(27), 44, 3697 }, { ZAP_CLUSTER_INDEX(71), 7, 127 },                 \
             { ZAP_CLUSTER_INDEX(78), 2, 4 },                                                                                       \
     }
 
@@ -3037,7 +3037,7 @@ static_assert(ATTRIBUTE_LARGEST <= CHIP_CONFIG_MAX_ATTRIBUTE_STORE_ELEMENT_SIZE,
 #define ATTRIBUTE_SINGLETONS_SIZE (37)
 
 // Total size of attribute storage
-#define ATTRIBUTE_MAX_SIZE (4193)
+#define ATTRIBUTE_MAX_SIZE (4173)
 
 // Number of fixed endpoints
 #define FIXED_ENDPOINT_COUNT (4)

diff --git a/src/app/zap-templates/zcl/data-model/chip/wake-on-lan-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/wake-on-lan-cluster.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <!--
-Copyright (c) 2021 Project CHIP Authors
+Copyright (c) 2021-2023 Project CHIP Authors
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -24,6 +24,10 @@ limitations under the License.
     <client init="false" tick="false">true</client>
     <server init="false" tick="false">true</server>
     <description>This cluster provides an interface for managing low power mode on a device that supports the Wake On LAN protocol.</description>
-    <attribute side="server" code="0x0000" define="WAKE_ON_LAN_MAC_ADDRESS" type="char_string" length="32" writable="false" optional="true">MACAddress</attribute>
+
+    <global side="either" code="0xFFFD" value="1"/>
+
+    <attribute side="server" code="0x0000" define="WAKE_ON_LAN_MAC_ADDRESS" type="char_string" length="12" writable="false" optional="true">MACAddress</attribute>
+    <attribute apiMaturity="provisional" side="server" code="0x0001" define="LINK_LOCAL_ADDRESS" type="octet_string" length="16" writable="false" optional="true">LinkLocalAddress</attribute>
   </cluster>
-</configurator>
+</configurator>
diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter
@@ -5859,7 +5859,8 @@ client cluster RadonConcentrationMeasurement = 1071 {
 
 /** This cluster provides an interface for managing low power mode on a device that supports the Wake On LAN protocol. */
 client cluster WakeOnLan = 1283 {
-  readonly attribute optional char_string<32> MACAddress = 0;
+  readonly attribute optional char_string<12> MACAddress = 0;
+  readonly attribute optional octet_string<16> linkLocalAddress = 1;
   readonly attribute command_id generatedCommandList[] = 65528;
   readonly attribute command_id acceptedCommandList[] = 65529;
   readonly attribute event_id eventList[] = 65530;

diff --git a/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java b/src/controller/java/generated/java/chip/devicecontroller/ChipClusters.java
@@ -43201,6 +43201,7 @@ public static class WakeOnLanCluster extends BaseChipCluster {
     public static final long CLUSTER_ID = 1283L;
 
     private static final long M_A_C_ADDRESS_ATTRIBUTE_ID = 0L;
+    private static final long LINK_LOCAL_ADDRESS_ATTRIBUTE_ID = 1L;
     private static final long GENERATED_COMMAND_LIST_ATTRIBUTE_ID = 65528L;
     private static final long ACCEPTED_COMMAND_LIST_ATTRIBUTE_ID = 65529L;
     private static final long EVENT_LIST_ATTRIBUTE_ID = 65530L;
@@ -43259,6 +43260,31 @@ public void onSuccess(byte[] tlv) {
         }, M_A_C_ADDRESS_ATTRIBUTE_ID, minInterval, maxInterval);
     }
 
+    public void readLinkLocalAddressAttribute(
+        OctetStringAttributeCallback callback) {
+      ChipAttributePath path = ChipAttributePath.newInstance(endpointId, clusterId, LINK_LOCAL_ADDRESS_ATTRIBUTE_ID);
+
+      readAttribute(new ReportCallbackImpl(callback, path) {
+          @Override
+          public void onSuccess(byte[] tlv) {
+            byte[] value = ChipTLVValueDecoder.decodeAttributeValue(path, tlv);
+            callback.onSuccess(value);
+          }
+        }, LINK_LOCAL_ADDRESS_ATTRIBUTE_ID, true);
+    }
+
+    public void subscribeLinkLocalAddressAttribute(
+        OctetStringAttributeCallback callback, int minInterval, int maxInterval) {
+      ChipAttributePath path = ChipAttributePath.newInstance(endpointId, clusterId, LINK_LOCAL_ADDRESS_ATTRIBUTE_ID);
+
+      subscribeAttribute(new ReportCallbackImpl(callback, path) {
+          @Override
+          public void onSuccess(byte[] tlv) {
+            byte[] value = ChipTLVValueDecoder.decodeAttributeValue(path, tlv);
+          }
+        }, LINK_LOCAL_ADDRESS_ATTRIBUTE_ID, minInterval, maxInterval);
+    }
+
     public void readGeneratedCommandListAttribute(
         GeneratedCommandListAttributeCallback callback) {
       ChipAttributePath path = ChipAttributePath.newInstance(endpointId, clusterId, GENERATED_COMMAND_LIST_ATTRIBUTE_ID);

diff --git a/src/controller/java/generated/java/chip/devicecontroller/ClusterIDMapping.java b/src/controller/java/generated/java/chip/devicecontroller/ClusterIDMapping.java
@@ -11968,6 +11968,7 @@ public long getID() {
 
         public enum Attribute {
             MACAddress(0L),
+            LinkLocalAddress(1L),
             GeneratedCommandList(65528L),
             AcceptedCommandList(65529L),
             EventList(65530L),

diff --git a/src/controller/java/generated/java/chip/devicecontroller/ClusterReadMapping.java b/src/controller/java/generated/java/chip/devicecontroller/ClusterReadMapping.java
@@ -14002,6 +14002,17 @@ private static Map<String, InteractionInfo> readWakeOnLanInteractionInfo() {
           readWakeOnLanMACAddressCommandParams
         );
         result.put("readMACAddressAttribute", readWakeOnLanMACAddressAttributeInteractionInfo);
+     Map<String, CommandParameterInfo> readWakeOnLanLinkLocalAddressCommandParams = new LinkedHashMap<String, CommandParameterInfo>();
+        InteractionInfo readWakeOnLanLinkLocalAddressAttributeInteractionInfo = new InteractionInfo(
+          (cluster, callback, commandArguments) -> {
+            ((ChipClusters.WakeOnLanCluster) cluster).readLinkLocalAddressAttribute(
+              (ChipClusters.OctetStringAttributeCallback) callback
+            );
+          },
+          () -> new ClusterInfoMapping.DelegatedOctetStringAttributeCallback(),
+          readWakeOnLanLinkLocalAddressCommandParams
+        );
+        result.put("readLinkLocalAddressAttribute", readWakeOnLanLinkLocalAddressAttributeInteractionInfo);
      Map<String, CommandParameterInfo> readWakeOnLanGeneratedCommandListCommandParams = new LinkedHashMap<String, CommandParameterInfo>();
         InteractionInfo readWakeOnLanGeneratedCommandListAttributeInteractionInfo = new InteractionInfo(
           (cluster, callback, commandArguments) -> {

diff --git a/...ntroller/java/generated/java/matter/devicecontroller/cluster/clusters/WakeOnLanCluster.kt b/...ntroller/java/generated/java/matter/devicecontroller/cluster/clusters/WakeOnLanCluster.kt
@@ -37,6 +37,14 @@ class WakeOnLanCluster(private val controller: MatterController, private val end
     // Implementation needs to be added here
   }
 
+  suspend fun readLinkLocalAddressAttribute(): OctetString {
+    // Implementation needs to be added here
+  }
+
+  suspend fun subscribeLinkLocalAddressAttribute(minInterval: Int, maxInterval: Int): OctetString {
+    // Implementation needs to be added here
+  }
+
   suspend fun readGeneratedCommandListAttribute(): GeneratedCommandListAttribute {
     // Implementation needs to be added here
   }

diff --git a/src/controller/java/zap-generated/CHIPAttributeTLVValueDecoder.cpp b/src/controller/java/zap-generated/CHIPAttributeTLVValueDecoder.cpp
diff --git a/src/controller/python/chip/clusters/CHIPClusters.py b/src/controller/python/chip/clusters/CHIPClusters.py
diff --git a/src/controller/python/chip/clusters/Objects.py b/src/controller/python/chip/clusters/Objects.py
diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRAttributeSpecifiedCheck.mm b/src/darwin/Framework/CHIP/zap-generated/MTRAttributeSpecifiedCheck.mm
diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRAttributeTLVValueDecoder.mm b/src/darwin/Framework/CHIP/zap-generated/MTRAttributeTLVValueDecoder.mm
diff --git a/src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h b/src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h