Address various DEM testing issues (#35067)

* Add checks for correct DEM feature map for test * Add checks for a nominalPower having a value and get slotIndex check correct * Allow a little tolerance checking the duration returned in the event as CI tests can run slower * Restyled by clang-format * Restyled by autopep8 * Add check DEM feature map step + add event registration step if necessary and renumber test cases * Fix issue #4415 Fails randomly on CI when checking pause duration * Test an illegal slot index == number of slots in a forecast * Add a test in PFR when ModifyForecastRequest does not specify NominalPower * Set ClusterRevision to 4 * First versionof DEM 1-1 * Regenerate files * Get DEM 1.1 test passing * Update test step definitions * Get featureSet option correct * Restyled by autopep8 * First cut of a Q quality test * Regenerate files following a re-bootstrap * Fix typo in comment * Cancel subscriptions at the end of the test * Add a cancel subscription method * Ensure MatterReportingAttributeChangeCallback is called after forecast is changed * Restyled by autopep8 * Restyled by isort * Fix ruff detected errors * Restyled by autopep8 * Fix the cancelling of subscriptions * Used .name instead of needing a feature string mapping. * Removed TC_DEM_1_1 now that the test plan has removed this test case. * Corrected Featuremap -> FeatureMap in test steps * Aligned verification step 2 on TC_DEM_2.3 to actual code and syncd test plan. * Aligned verification step 2 on TC_DEM_2.4 to actual code and syncd test plan. * Added in missing text in step 18 in TC_DEM_2.4 * TC_DEM_2.5 minor update to step 2 and aligning to updated test plan. * TC_DEM_2.5 - slight change to call to name args in validate_feature_map() call * TC_DEM_2.6,2.7,2.8 step 2 alignment with tweaked test plan * TC_DEM_2.6/2.7/2.8 - slight change to call to name args in validate_feature_map() call * Corrected log message in TC_DEM_2.9 step 4b * Updated TC_DEM_2.10 (Q Quality) Basic structure is there - steps need tidying up and fails due to powerAdjustmentCapability not being updated as expected. * Fixed bug where PowerAdjustReason wasn't calling MatterReportingAttributeChangeCallback when reason was changing. * Restyled by clang-format * Restyled by autopep8 * Restyled by isort * Autogen'd the TestSteps from latest test plan * Changed featureSet to 0x7b to enable PA, PFR, FA (plus other) features --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: James Harrow <james.harrow@gmail.com> Co-authored-by: Hasty Granbery <hasty@granbery.org>
project-chip · Aug 29, 2024 · bab66f5 · bab66f5
1 parent ac5dfee
commit bab66f5
Show file tree

Hide file tree

Showing 16 changed files with 1,177 additions and 821 deletions.
diff --git a/...y-management-common/device-energy-management/include/DeviceEnergyManagementDelegateImpl.h b/...y-management-common/device-energy-management/include/DeviceEnergyManagementDelegateImpl.h
@@ -189,6 +189,7 @@ class DeviceEnergyManagementDelegate : public DeviceEnergyManagement::Delegate
     CHIP_ERROR SetAbsMinPower(int64_t);
     CHIP_ERROR SetAbsMaxPower(int64_t);
     CHIP_ERROR SetPowerAdjustmentCapability(const DataModel::Nullable<Structs::PowerAdjustCapabilityStruct::Type> &);
+    CHIP_ERROR SetPowerAdjustmentCapabilityPowerAdjustReason(const PowerAdjustReasonEnum);
 
     // The DeviceEnergyManagementDelegate owns the master copy of the ForecastStruct object which is accessed via GetForecast and
     // SetForecast. The slots field of forecast is owned and managed by the object that implements the DEMManufacturerDelegate

diff --git a/...rgy-management-common/device-energy-management/src/DeviceEnergyManagementDelegateImpl.cpp b/...rgy-management-common/device-energy-management/src/DeviceEnergyManagementDelegateImpl.cpp
@@ -120,11 +120,11 @@ Status DeviceEnergyManagementDelegate::PowerAdjustRequest(const int64_t powerMw,
     switch (cause)
     {
     case AdjustmentCauseEnum::kLocalOptimization:
-        mPowerAdjustCapabilityStruct.Value().cause = PowerAdjustReasonEnum::kLocalOptimizationAdjustment;
+        SetPowerAdjustmentCapabilityPowerAdjustReason(PowerAdjustReasonEnum::kLocalOptimizationAdjustment);
         break;
 
     case AdjustmentCauseEnum::kGridOptimization:
-        mPowerAdjustCapabilityStruct.Value().cause = PowerAdjustReasonEnum::kGridOptimizationAdjustment;
+        SetPowerAdjustmentCapabilityPowerAdjustReason(PowerAdjustReasonEnum::kGridOptimizationAdjustment);
         break;
 
     default:
@@ -175,7 +175,7 @@ void DeviceEnergyManagementDelegate::HandlePowerAdjustRequestFailure()
 
     mPowerAdjustmentInProgress = false;
 
-    mPowerAdjustCapabilityStruct.Value().cause = PowerAdjustReasonEnum::kNoAdjustment;
+    SetPowerAdjustmentCapabilityPowerAdjustReason(PowerAdjustReasonEnum::kNoAdjustment);
 
     // TODO
     // Should we inform the mpDEMManufacturerDelegate that PowerAdjustRequest has failed?
@@ -210,7 +210,7 @@ void DeviceEnergyManagementDelegate::HandlePowerAdjustTimerExpiry()
 
     SetESAState(ESAStateEnum::kOnline);
 
-    mPowerAdjustCapabilityStruct.Value().cause = PowerAdjustReasonEnum::kNoAdjustment;
+    SetPowerAdjustmentCapabilityPowerAdjustReason(PowerAdjustReasonEnum::kNoAdjustment);
 
     // Generate a PowerAdjustEnd event
     GeneratePowerAdjustEndEvent(CauseEnum::kNormalCompletion);
@@ -264,8 +264,7 @@ CHIP_ERROR DeviceEnergyManagementDelegate::CancelPowerAdjustRequestAndGenerateEv
     SetESAState(ESAStateEnum::kOnline);
 
     mPowerAdjustmentInProgress = false;
-
-    mPowerAdjustCapabilityStruct.Value().cause = PowerAdjustReasonEnum::kNoAdjustment;
+    SetPowerAdjustmentCapabilityPowerAdjustReason(PowerAdjustReasonEnum::kNoAdjustment);
 
     CHIP_ERROR err = GeneratePowerAdjustEndEvent(cause);
 
@@ -922,6 +921,18 @@ DeviceEnergyManagementDelegate::SetPowerAdjustmentCapability(
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR
+DeviceEnergyManagementDelegate::SetPowerAdjustmentCapabilityPowerAdjustReason(PowerAdjustReasonEnum powerAdjustReason)
+{
+    assertChipStackLockedByCurrentThread();
+
+    mPowerAdjustCapabilityStruct.Value().cause = powerAdjustReason;
+
+    MatterReportingAttributeChangeCallback(mEndpointId, DeviceEnergyManagement::Id, PowerAdjustmentCapability::Id);
+
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR DeviceEnergyManagementDelegate::SetForecast(const DataModel::Nullable<Structs::ForecastStruct::Type> & forecast)
 {
     assertChipStackLockedByCurrentThread();
@@ -962,9 +973,9 @@ CHIP_ERROR DeviceEnergyManagementDelegate::SetOptOutState(OptOutStateEnum newVal
     if (mPowerAdjustmentInProgress)
     {
         if ((newValue == OptOutStateEnum::kLocalOptOut &&
-             mPowerAdjustCapabilityStruct.Value().cause == PowerAdjustReasonEnum::kLocalOptimizationAdjustment) ||
+             GetPowerAdjustmentCapability().Value().cause == PowerAdjustReasonEnum::kLocalOptimizationAdjustment) ||
             (newValue == OptOutStateEnum::kGridOptOut &&
-             mPowerAdjustCapabilityStruct.Value().cause == PowerAdjustReasonEnum::kGridOptimizationAdjustment) ||
+             GetPowerAdjustmentCapability().Value().cause == PowerAdjustReasonEnum::kGridOptimizationAdjustment) ||
             newValue == OptOutStateEnum::kOptOut)
         {
             err = CancelPowerAdjustRequestAndGenerateEvent(DeviceEnergyManagement::CauseEnum::kUserOptOut);

diff --git a/examples/energy-management-app/energy-management-common/energy-management-app.matter b/examples/energy-management-app/energy-management-common/energy-management-app.matter
@@ -2704,7 +2704,7 @@ endpoint 1 {
     callback attribute eventList;
     callback attribute attributeList;
     callback attribute featureMap;
-    ram      attribute clusterRevision default = 3;
+    ram      attribute clusterRevision default = 4;
 
     handle command PowerAdjustRequest;
     handle command CancelPowerAdjustRequest;

diff --git a/examples/energy-management-app/energy-management-common/energy-management-app.zap b/examples/energy-management-app/energy-management-common/energy-management-app.zap
@@ -4210,7 +4210,7 @@
               "storageOption": "RAM",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "3",
+              "defaultValue": "4",
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,

diff --git a/src/app/clusters/device-energy-management-server/device-energy-management-server.cpp b/src/app/clusters/device-energy-management-server/device-energy-management-server.cpp
@@ -676,7 +676,7 @@ void Instance::HandleModifyForecastRequest(HandlerContext & ctx, const Commands:
         const Structs::SlotAdjustmentStruct::Type & slotAdjustment = iterator.GetValue();
 
         // Check for an invalid slotIndex
-        if (slotAdjustment.slotIndex > forecast.Value().slots.size())
+        if (slotAdjustment.slotIndex >= forecast.Value().slots.size())
         {
             ChipLogError(Zcl, "DEM: Bad slot index %d", slotAdjustment.slotIndex);
             ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Failure);
@@ -696,8 +696,8 @@ void Instance::HandleModifyForecastRequest(HandlerContext & ctx, const Commands:
         // NominalPower is only relevant if PFR is supported
         if (HasFeature(Feature::kPowerForecastReporting))
         {
-            if (!slot.minPowerAdjustment.HasValue() || !slot.maxPowerAdjustment.HasValue() ||
-                slotAdjustment.nominalPower.Value() < slot.minPowerAdjustment.Value() ||
+            if (!slotAdjustment.nominalPower.HasValue() || !slot.minPowerAdjustment.HasValue() ||
+                !slot.maxPowerAdjustment.HasValue() || slotAdjustment.nominalPower.Value() < slot.minPowerAdjustment.Value() ||
                 slotAdjustment.nominalPower.Value() > slot.maxPowerAdjustment.Value())
             {
                 ChipLogError(Zcl, "DEM: Bad nominalPower");

diff --git a/src/python_testing/TC_DEMTestBase.py b/src/python_testing/TC_DEMTestBase.py
@@ -38,6 +38,26 @@ async def check_dem_attribute(self, attribute, expected_value, endpoint: int = N
         asserts.assert_equal(value, expected_value,
                              f"Unexpected '{attribute}' value - expected {expected_value}, was {value}")
 
+    async def validate_feature_map(self, must_have_features, must_not_have_features):
+        feature_map = await self.read_dem_attribute_expect_success(attribute="FeatureMap")
+        for must_have_feature in must_have_features:
+            asserts.assert_true(feature_map & must_have_feature,
+                                f"{must_have_feature.name} must be set but is not. feature_map 0x{feature_map:x}")
+
+        for must_not_have_feature in must_not_have_features:
+            asserts.assert_false(feature_map & must_not_have_feature,
+                                 f"{must_not_have_feature.name} is not allowed to be set. feature_map 0x{feature_map:x}")
+
+    async def validate_pfr_or_sfr_in_feature_map(self):
+        feature_map = await self.read_dem_attribute_expect_success(attribute="FeatureMap")
+
+        illegal_combination = Clusters.DeviceEnergyManagement.Bitmaps.Feature.kPowerForecastReporting | Clusters.DeviceEnergyManagement.Bitmaps.Feature.kStateForecastReporting
+        asserts.assert_not_equal(feature_map & illegal_combination, illegal_combination,
+                                 f"Cannot have kPowerForecastReporting and kStateForecastReporting both set. feature_map 0x{feature_map:x}")
+
+        asserts.assert_not_equal(feature_map & illegal_combination, 0,
+                                 f"Must have one of kPowerForecastReporting and kStateForecastReporting set. feature_map 0x{feature_map:x}")
+
     async def send_power_adjustment_command(self, power: int, duration: int,
                                             cause: Clusters.Objects.DeviceEnergyManagement.Enums.CauseEnum,
                                             endpoint: int = None, timedRequestTimeoutMs: int = 3000,