From 20628020bb8a9006a03361e3e87b8e3a8a14ba89 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 8 Nov 2023 08:38:44 -0500 Subject: [PATCH] Match `Wake On LAN Cluster` to spec (#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 --- .../all-clusters-app.matter | 2 +- examples/tv-app/linux/args.gni | 3 + .../clusters/wake-on-lan/WakeOnLanManager.cpp | 14 ++- examples/tv-app/tv-common/tv-app.matter | 2 +- .../tv-casting-common/tv-casting-app.matter | 3 +- .../app-templates/endpoint_config.h | 8 +- .../data-model/chip/wake-on-lan-cluster.xml | 10 +- .../data_model/controller-clusters.matter | 3 +- .../chip/devicecontroller/ChipClusters.java | 26 ++++++ .../devicecontroller/ClusterIDMapping.java | 1 + .../devicecontroller/ClusterReadMapping.java | 11 +++ .../cluster/clusters/WakeOnLanCluster.kt | 8 ++ .../CHIPAttributeTLVValueDecoder.cpp | 15 +++ .../python/chip/clusters/CHIPClusters.py | 6 ++ .../python/chip/clusters/Objects.py | 18 ++++ .../MTRAttributeSpecifiedCheck.mm | 3 + .../MTRAttributeTLVValueDecoder.mm | 11 +++ .../CHIP/zap-generated/MTRBaseClusters.h | 6 ++ .../CHIP/zap-generated/MTRBaseClusters.mm | 36 ++++++++ .../CHIP/zap-generated/MTRClusterConstants.h | 1 + .../CHIP/zap-generated/MTRClusters.h | 2 + .../CHIP/zap-generated/MTRClusters.mm | 5 + .../integration-tests/tv-app/test_app.py | 2 +- .../zap-generated/attributes/Accessors.cpp | 43 +++++++-- .../zap-generated/attributes/Accessors.h | 5 + .../zap-generated/cluster-objects.cpp | 2 + .../zap-generated/cluster-objects.h | 16 +++- .../app-common/zap-generated/ids/Attributes.h | 4 + .../zap-generated/cluster/Commands.h | 5 + .../cluster/logging/DataModelLogger.cpp | 5 + .../zap-generated/cluster/Commands.h | 91 +++++++++++++++++++ 31 files changed, 346 insertions(+), 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 index 191a87a0088dc2..b5046f38121963 100644 --- 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 index aa476d37b80c30..4861f23ac068dc 100644 --- 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 index c851e0674d6697..e6fb5e38e61969 100644 --- 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 #include +constexpr const char * kNullHexMACAddress = "000000000000"; + using namespace chip; using namespace chip::app::Clusters::WakeOnLan; @@ -33,7 +35,7 @@ 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 @@ -41,7 +43,7 @@ std::string getMacAddress() 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 index eb33dba7b9faf6..e420f9b9793e4c 100644 --- 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 index 0c658c249b386b..be984fb04b229e 100644 --- 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 index e30be351ebe20c..c7bc4b0509434d 100644 --- 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 index e9ad2ea7d5ec5b..c250eb2874413d 100644 --- 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 @@