Modify helper endpoint config 4byte default (#18478)

Change defaultValue in EmberAfDefaultOrMinMaxAttributeValue's from uint16_t to uint32_t In EmberAfDefaultOrMinMaxAttributeValue's union defaultValue was uint16_t. In this union there field that are pointers, and most targets use 32bit addressing so the size of the union ends up being 4 bytes. By changing defaultValue to uint32_t we save some rodata region since this data can fit within the EmberAfDefaultOrMinMaxAttributeValue struct. Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · May 19, 2022 · 6b82aee · 6b82aee
1 parent fa926d8
commit 6b82aee
Show file tree

Hide file tree

Showing 30 changed files with 1,549 additions and 6,165 deletions.
diff --git a/examples/bridge-app/esp32/main/main.cpp b/examples/bridge-app/esp32/main/main.cpp
@@ -359,7 +359,7 @@ void HandleDeviceStatusChanged(Device * dev, Device::Changed_t itemChangedMask)
         uint8_t buffer[kFixedLabelAttributeArraySize];
         EmberAfAttributeMetadata am = { .attributeId  = ZCL_LABEL_LIST_ATTRIBUTE_ID,
                                         .size         = kFixedLabelAttributeArraySize,
-                                        .defaultValue = static_cast<uint16_t>(0) };
+                                        .defaultValue = static_cast<uint32_t>(0) };
 
         EncodeFixedLabel("room", dev->GetLocation(), buffer, sizeof(buffer), &am);
 

diff --git a/examples/bridge-app/linux/main.cpp b/examples/bridge-app/linux/main.cpp
@@ -350,7 +350,7 @@ void HandleDeviceStatusChanged(Device * dev, Device::Changed_t itemChangedMask)
         uint8_t buffer[kFixedLabelAttributeArraySize];
         EmberAfAttributeMetadata am = { .attributeId  = ZCL_LABEL_LIST_ATTRIBUTE_ID,
                                         .size         = kFixedLabelAttributeArraySize,
-                                        .defaultValue = static_cast<uint16_t>(0) };
+                                        .defaultValue = static_cast<uint32_t>(0) };
 
         EncodeFixedLabel("room", dev->GetLocation(), buffer, sizeof(buffer), &am);
 

diff --git a/src/app/tests/TestWriteInteraction.cpp b/src/app/tests/TestWriteInteraction.cpp
@@ -328,7 +328,7 @@ void TestWriteInteraction::TestWriteHandler(nlTestSuite * apSuite, void * apCont
 const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath)
 {
     // Note: This test does not make use of the real attribute metadata.
-    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint16_t(0)) };
+    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint32_t(0)) };
     return &stub;
 }
 

diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp
@@ -644,7 +644,7 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
 const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath)
 {
     // Note: This test does not make use of the real attribute metadata.
-    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint16_t(0)) };
+    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint32_t(0)) };
     return &stub;
 }
 

diff --git a/src/app/tests/integration/chip_im_responder.cpp b/src/app/tests/integration/chip_im_responder.cpp
@@ -125,7 +125,7 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
 const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath)
 {
     // Note: This test does not make use of the real attribute metadata.
-    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint16_t(0)) };
+    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint32_t(0)) };
     return &stub;
 }
 

diff --git a/src/app/util/attribute-metadata.h b/src/app/util/attribute-metadata.h
@@ -100,19 +100,20 @@ typedef struct
 union EmberAfDefaultOrMinMaxAttributeValue
 {
     constexpr EmberAfDefaultOrMinMaxAttributeValue(const uint8_t * ptr) : ptrToDefaultValue(ptr) {}
-    constexpr EmberAfDefaultOrMinMaxAttributeValue(uint16_t val) : defaultValue(val) {}
+    constexpr EmberAfDefaultOrMinMaxAttributeValue(uint32_t val) : defaultValue(val) {}
     constexpr EmberAfDefaultOrMinMaxAttributeValue(const EmberAfAttributeMinMaxValue * ptr) : ptrToMinMaxValue(ptr) {}
 
     /**
-     * Points to data if size is more than 2 bytes.
-     * If size is more than 2 bytes, and this value is NULL,
+     * Points to data if the attribute type is a string or the size of the data is more than 4 bytes.
+     * If the attribute type is a string or the data size is more than 4 bytes, and this value is NULL,
      * then the default value is all zeroes.
      */
     const uint8_t * ptrToDefaultValue;
     /**
-     * Actual default value if the attribute size is 2 bytes or less.
+     * Actual default value if the attribute is non string and size
+     * is 4 bytes or less.
      */
-    uint16_t defaultValue;
+    uint32_t defaultValue;
     /**
      * Points to the min max attribute value structure, if min/max is
      * supported for this attribute.

diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp
@@ -81,7 +81,7 @@ uint8_t singletonAttributeData[ACTUAL_SINGLETONS_SIZE];
 
 uint16_t emberEndpointCount = 0;
 
-// If we have attributes that are more than 2 bytes, then
+// If we have attributes that are more than 4 bytes, then
 // we need this data block for the defaults
 #if (defined(GENERATED_DEFAULTS) && GENERATED_DEFAULTS_COUNT)
 constexpr const uint8_t generatedDefaults[] = GENERATED_DEFAULTS;
@@ -1245,11 +1245,20 @@ void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional
 
                     if (ptr == nullptr)
                     {
+                        size_t defaultValueSizeForBigEndianNudger = 0;
+                        // Bypasses compiler warning about unused variable for little endian platforms.
+                        (void) defaultValueSizeForBigEndianNudger;
                         if ((am->mask & ATTRIBUTE_MASK_MIN_MAX) != 0U)
                         {
+                            // This is intentionally 2 and not 4 bytes since defaultValue in min/max
+                            // attributes is still uint16_t.
                             if (emberAfAttributeSize(am) <= 2)
                             {
+                                static_assert(sizeof(am->defaultValue.ptrToMinMaxValue->defaultValue.defaultValue) == 2,
+                                              "if statement relies on size of max/min defaultValue being 2");
                                 ptr = (uint8_t *) &(am->defaultValue.ptrToMinMaxValue->defaultValue.defaultValue);
+                                defaultValueSizeForBigEndianNudger =
+                                    sizeof(am->defaultValue.ptrToMinMaxValue->defaultValue.defaultValue);
                             }
                             else
                             {
@@ -1258,9 +1267,10 @@ void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional
                         }
                         else
                         {
-                            if (emberAfAttributeSize(am) <= 2)
+                            if ((emberAfAttributeSize(am) <= 4) && !emberAfIsStringAttributeType(am->attributeType))
                             {
-                                ptr = (uint8_t *) &(am->defaultValue.defaultValue);
+                                ptr                                = (uint8_t *) &(am->defaultValue.defaultValue);
+                                defaultValueSizeForBigEndianNudger = sizeof(am->defaultValue.defaultValue);
                             }
                             else
                             {
@@ -1271,13 +1281,15 @@ void emAfLoadAttributeDefaults(EndpointId endpoint, bool ignoreStorage, Optional
                         // it should be treated as if it is pointing to an array of all zeroes.
 
 #if (BIGENDIAN_CPU)
-                        // The default value for one- and two-byte attributes is stored in an
-                        // uint16_t.  On big-endian platforms, a pointer to the default value of
-                        // a one-byte attribute will point to the wrong byte.  So, for those
-                        // cases, nudge the pointer forward so it points to the correct byte.
-                        if (emberAfAttributeSize(am) == 1 && ptr != NULL)
+                        // The default values for attributes that are less than or equal to
+                        // defaultValueSizeForBigEndianNudger in bytes are stored in an
+                        // uint32_t.  On big-endian platforms, a pointer to the default value
+                        // of size less than defaultValueSizeForBigEndianNudger will point to the wrong
+                        // byte.  So, for those cases, nudge the pointer forward so it points
+                        // to the correct byte.
+                        if (emberAfAttributeSize(am) < defaultValueSizeForBigEndianNudger && ptr != NULL)
                         {
-                            *ptr++;
+                            ptr += (defaultValueSizeForBigEndianNudger - emberAfAttributeSize(am));
                         }
 #endif // BIGENDIAN
                     }

diff --git a/src/app/util/attribute-table.cpp b/src/app/util/attribute-table.cpp
@@ -316,6 +316,8 @@ EmberAfStatus emAfWriteAttribute(EndpointId endpoint, ClusterId cluster, Attribu
         const uint8_t * maxBytes;
         if (dataLen <= 2)
         {
+            static_assert(sizeof(minv.defaultValue) == 2, "if statement relies on size of minv.defaultValue being 2");
+            static_assert(sizeof(maxv.defaultValue) == 2, "if statement relies on size of maxv.defaultValue being 2");
             minBytes = reinterpret_cast<const uint8_t *>(&(minv.defaultValue));
             maxBytes = reinterpret_cast<const uint8_t *>(&(maxv.defaultValue));
 // On big endian cpu with length 1 only the second byte counts

diff --git a/src/app/zap-templates/templates/app/endpoint_config.zapt b/src/app/zap-templates/templates/app/endpoint_config.zapt
@@ -5,7 +5,7 @@
 
 #include <lib/core/CHIPConfig.h>
 
-{{#endpoint_config allowUnknownStorageOption="false"}}
+{{#endpoint_config allowUnknownStorageOption="false" spaceForDefaultValue=4}}
 
 // Default values for the attributes longer than a pointer,
 // in a form of a binary blob
@@ -22,8 +22,8 @@
 #define ZAP_TYPE(type) ZCL_ ## type ## _ATTRIBUTE_TYPE
 #define ZAP_LONG_DEFAULTS_INDEX(index) { &generatedDefaults[index] }
 #define ZAP_MIN_MAX_DEFAULTS_INDEX(index) { &minMaxDefaults[index] }
-#define ZAP_EMPTY_DEFAULT() {(uint16_t) 0}
-#define ZAP_SIMPLE_DEFAULT(x) {(uint16_t) x}
+#define ZAP_EMPTY_DEFAULT() {(uint32_t) 0}
+#define ZAP_SIMPLE_DEFAULT(x) {(uint32_t) x}
 
 // This is an array of EmberAfAttributeMinMaxValue structures.
 #define GENERATED_MIN_MAX_DEFAULT_COUNT {{endpoint_attribute_min_max_count}}

diff --git a/src/controller/tests/data_model/TestWrite.cpp b/src/controller/tests/data_model/TestWrite.cpp
@@ -58,7 +58,7 @@ namespace app {
 const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath)
 {
     // Note: This test does not make use of the real attribute metadata.
-    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint16_t(0)) };
+    static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint32_t(0)) };
     return &stub;
 }
 

diff --git a/src/darwin/Framework/CHIP/CHIPIMDispatch.mm b/src/darwin/Framework/CHIP/CHIPIMDispatch.mm
@@ -156,7 +156,7 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath,
     const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath)
     {
         // Note: This test does not make use of the real attribute metadata.
-        static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint16_t(0)) };
+        static EmberAfAttributeMetadata stub = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(uint32_t(0)) };
         return &stub;
     }
 

diff --git a/third_party/zap/repo b/third_party/zap/repo
+1 −1		Jenkinsfile
+1 −1		apack.json
+3 −3		cypress/integration/clusters/cluster-filter.spec.js
+0 −57		cypress/integration/overview/overview.spec.js
+10 −2		docs/make-schema-diagram
+5 −1		docs/releasenotes.md
+2,259 −2,184		docs/zap-schema.svg
+36,955 −414		package-lock.json
+1 −1		package.json
+6 −2		src-electron/db/db-mapping.js
+5 −2		src-electron/db/query-command.js
+8 −4		src-electron/db/query-config.js
+324 −0		src-electron/db/query-device-type.js
+11 −4		src-electron/db/query-endpoint-type.js
+11 −11		src-electron/db/query-endpoint.js
+0 −276		src-electron/db/query-zcl.js
+11 −9		src-electron/generator/helper-endpointconfig.js
+61 −11		src-electron/generator/helper-zcl.js
+13 −12		src-electron/importexport/import-isc.js
+13 −12		src-electron/rest/static-zcl.js
+15 −13		src-electron/zcl/zcl-loader-silabs.js
+9 −8		src-electron/zcl/zcl-loader.js
+78 −0		src/components/InitialContent.vue
+6 −6		src/components/ZclClusterManager.vue
+2 −2		src/components/ZclClusterView.vue
+18 −28		src/components/ZclCommandManager.vue
+61 −69		src/components/ZclCustomZclView.vue
+32 −30		src/components/ZclEndpointCard.vue
+62 −67		src/components/ZclGeneralOptionsBar.vue
+114 −9		src/layouts/ZclConfiguratorLayout.vue
+20 −6		src/layouts/ZclLayout.vue
+4 −0		src/store/zap/mutations.js
+1 −0		src/store/zap/state.js
+2 −2		test/custom-cluster.test.js
+7 −0		test/gen-matter.test.js
+18 −0		test/gen-template/matter/device-types.zapt
+5 −0		test/gen-template/matter/gen-test.json
+2 −1		test/query.test.js
+1 −1		test/test-util.js
+13 −12		test/validation.test.js
+3 −2		test/zcl-loader-consecutive.test.js
+16 −15		test/zcl-loader.test.js
+10 −9		test/zcl-properties-loader.test.js