Cleanup up comments and useless static cast

project-chip · Aug 30, 2024 · 4c2702a · 4c2702a
1 parent 64dbdd2
commit 4c2702a
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 6 deletions.
diff --git a/src/app/clusters/scenes-server/SceneHandlerImpl.cpp b/src/app/clusters/scenes-server/SceneHandlerImpl.cpp
@@ -121,8 +121,16 @@ void CapAttributeValue(typename app::NumericAttributeTraits<Type>::WorkingType &
 
     if (metadata->IsBoolean())
     {
-        // Caping the value to 1 in case values greater than 1 are set
-        value = value ? 1 : 0;
+        if (metadata->IsNullable() && (value != 1 && value != 0))
+        {
+            // If the attribute is nullable, the value can be set to NULL
+            app::NumericAttributeTraits<WorkingType>::SetNull(value);
+        }
+        else
+        {
+            // Caping the value to 1 in case values greater than 1 are set
+            value = value ? 1 : 0;
+        }
         return;
     }
 
@@ -134,6 +142,10 @@ void CapAttributeValue(typename app::NumericAttributeTraits<Type>::WorkingType &
         maxValue                                        = ConvertDefaultValueToWorkingValue<Type>(minMaxValue->maxValue);
     }
 
+    // If the attribute is nullable, the min and max values calculated for types will not be valid, however this does not
+    // change the behavior here as the value will already be NULL if it is out of range. E.g. a nullable INT8U has a minValue of
+    // -127. The code above determin minValue = -128, so an input value of -128 would not enter the condition block below, but would
+    // be considered NULL nonetheless.
     if (metadata->IsNullable() && (minValue > value || maxValue < value))
     {
         // If the attribute is nullable, the value can be set to NULL

diff --git a/src/app/tests/TestSceneTable.cpp b/src/app/tests/TestSceneTable.cpp
@@ -176,7 +176,7 @@ static EmberAfAttributeMetadata mockMetadataBool = {
     .attributeId   = 0,
     .size          = 1,
     .attributeType = ZCL_BOOLEAN_ATTRIBUTE_TYPE,
-    .mask          = ATTRIBUTE_MASK_WRITABLE | ATTRIBUTE_MASK_NULLABLE,
+    .mask          = ATTRIBUTE_MASK_WRITABLE,
 };
 
 static EmberAfAttributeMetadata mockMetadataUint8 = {
@@ -1065,7 +1065,8 @@ TEST_F(TestSceneTable, TestHandlerFunctions)
         MockCCPairs[5].attributeID = MockAttributeId(kEnhancedColorMode);
         MockCCPairs[5].valueSigned8.SetValue(static_cast<int8_t>(-2)); // will cap to -1
         MockCCPairs[6].attributeID = MockAttributeId(kColorLoopActiveId);
-        MockCCPairs[6].valueSigned16.SetValue(static_cast<int16_t>(0x7FFE)); // will cap to 0x7FFD in int16
+        MockCCPairs[6].valueSigned16.SetValue(
+            static_cast<int16_t>(0x7FFE)); // will cap to 0x7FFD in int16 due to declared maximum in the attribute's mock metadata
         MockCCPairs[7].attributeID = MockAttributeId(kColorLoopDirectionId);
         MockCCPairs[7].valueSigned32.SetValue(-1); // will cap to -1 in int24
         MockCCPairs[8].attributeID = MockAttributeId(kColorLoopTimeId);
@@ -1279,9 +1280,9 @@ TEST_F(TestSceneTable, TestHandlerFunctions)
 
         // Verify that the output value is capped to uint40 max value
         uint64_t uint40Max = static_cast<uint64_t>(0x000000FFFFFFFFFF);
-        EXPECT_EQ(uint40Max, static_cast<uint64_t>(extensionFieldValueCapOut.attributeValueList[0].valueUnsigned64.Value()));
+        EXPECT_EQ(uint40Max, extensionFieldValueCapOut.attributeValueList[0].valueUnsigned64.Value());
 
-        // Verify that the output value is not capped
+        // Verify that the output value is capped to int40 max value
         int64_t int40Max = static_cast<int64_t>(0x0000007FFFFFFFFF);
         EXPECT_EQ(int40Max, extensionFieldValueCapOut.attributeValueList[1].valueSigned64.Value());