TC-EEVSE-2.3: Fix check when command returns success, but expects error (project-chip#41687)

cecille · restyled-commits · jamesharrow · web-flow · commit 5efe2e7d6097 · 2025-10-31T00:48:05.000Z
* TC-CNET-4.12: Potential fix for connection timeout [WIP] * Another try with srp * some more fixes * fix instance name * Fix EEVSE check for command errors * Fix app * Restyled by autopep8 * Revert "fix instance name" This reverts commit 2d5a46f. * Revert "some more fixes" This reverts commit 04a32b5. * Revert "Another try with srp" This reverts commit ecd3d2c. * Revert "TC-CNET-4.12: Potential fix for connection timeout [WIP]" This reverts commit 67b1b24. * Made the check at a lower part of the SDK and changed to use an existing constant. * Added stronger validation for all send_single_cmd() calls to ensure the expected_status is Success in the success path. * Added comments as proposed in PR review * Added suggestion from PR to improve assert message when it fails. * Restyled by autopep8 --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: James Harrow <james.harrow@geotogether.com>
diff --git a/examples/energy-management-app/energy-management-common/energy-evse/src/ChargingTargetsMemMgr.cpp b/examples/energy-management-app/energy-management-common/energy-evse/src/ChargingTargetsMemMgr.cpp
@@ -93,6 +93,7 @@ CHIP_ERROR ChargingTargetsMemMgr::AllocAndCopy()
     // ChargingTargetsMemMgr.h before this method can be called.
 
     VerifyOrDie(mpListOfDays[mChargingTargetSchedulesIdx] == nullptr);
+    VerifyOrReturnError(mNumDailyChargingTargets <= kEvseTargetsMaxTargetsPerDay, CHIP_ERROR_NO_MEMORY);
 
     if (mNumDailyChargingTargets > 0)
     {
@@ -124,6 +125,7 @@ ChargingTargetsMemMgr::AllocAndCopy(const DataModel::List<const Structs::Chargin
     VerifyOrDie(mpListOfDays[mChargingTargetSchedulesIdx] == nullptr);
 
     mNumDailyChargingTargets = static_cast<uint16_t>(chargingTargets.size());
+    VerifyOrReturnError(mNumDailyChargingTargets <= kEvseTargetsMaxTargetsPerDay, CHIP_ERROR_NO_MEMORY);
 
     if (mNumDailyChargingTargets > 0)
     {
@@ -161,6 +163,7 @@ ChargingTargetsMemMgr::AllocAndCopy(const DataModel::DecodableList<Structs::Char
     ReturnErrorOnFailure(chargingTargets.ComputeSize(&numDailyChargingTargets));
 
     mNumDailyChargingTargets = static_cast<uint16_t>(numDailyChargingTargets);
+    VerifyOrReturnError(mNumDailyChargingTargets <= kEvseTargetsMaxTargetsPerDay, CHIP_ERROR_NO_MEMORY);
 
     if (mNumDailyChargingTargets > 0)
     {
diff --git a/examples/energy-management-app/energy-management-common/energy-evse/src/EnergyEvseDelegateImpl.cpp b/examples/energy-management-app/energy-management-common/energy-evse/src/EnergyEvseDelegateImpl.cpp
@@ -368,6 +368,10 @@ Status EnergyEvseDelegate::SetTargets(
     VerifyOrReturnError(targets != nullptr, Status::Failure);
 
     CHIP_ERROR err = targets->SetTargets(chargingTargetSchedules);
+    if (err == CHIP_ERROR_NO_MEMORY)
+    {
+        return Status::ResourceExhausted;
+    }
     VerifyOrReturnError(err == CHIP_NO_ERROR, StatusIB(err).mStatus);
 
     /* The Application needs to be told that the Targets have been updated
diff --git a/src/app/clusters/energy-evse-server/energy-evse-server.cpp b/src/app/clusters/energy-evse-server/energy-evse-server.cpp
@@ -449,6 +449,12 @@ Status Instance::ValidateTargets(
             innerIdx++;
         }
 
+        if (innerIdx > kEvseTargetsMaxTargetsPerDay)
+        {
+            ChipLogError(AppServer, "Too many targets in a single ChargingTargetScheduleStruct (%d)", innerIdx);
+            return Status::ResourceExhausted;
+        }
+
         if (iterInner.GetStatus() != CHIP_NO_ERROR)
         {
             return Status::InvalidCommand;
diff --git a/src/python_testing/TC_EEVSE_Utils.py b/src/python_testing/TC_EEVSE_Utils.py
@@ -86,7 +86,12 @@ async def send_enable_charge_command(self, endpoint: int = None, charge_until: i
                     endpoint=endpoint,
                     timedRequestTimeoutMs=timedRequestTimeoutMs)
 
+            # If the command was expected to fail but it succeeded, check it wasn't meant to fail
+            asserts.assert_equal(expected_status, Status.Success,
+                                 f"Unexpected Success returned when expected {expected_status}")
+
         except InteractionModelError as e:
+            # The command failed, which might be what we expected, check if it is the expected error
             asserts.assert_equal(e.status, expected_status,
                                  "Unexpected error returned")
 
@@ -106,7 +111,12 @@ async def send_enable_discharge_command(self, endpoint: int = None, discharge_un
                     endpoint=endpoint,
                     timedRequestTimeoutMs=timedRequestTimeoutMs)
 
+            # If the command was expected to fail but it succeeded, check it wasn't meant to fail
+            asserts.assert_equal(expected_status, Status.Success,
+                                 f"Unexpected Success returned when expected {expected_status}")
+
         except InteractionModelError as e:
+            # The command failed, which might be what we expected, check if it is the expected error
             asserts.assert_equal(e.status, expected_status,
                                  "Unexpected error returned")
 
@@ -116,7 +126,12 @@ async def send_disable_command(self, endpoint: int = None, timedRequestTimeoutMs
                                        endpoint=endpoint,
                                        timedRequestTimeoutMs=timedRequestTimeoutMs)
 
+            # If the command was expected to fail but it succeeded, check it wasn't meant to fail
+            asserts.assert_equal(expected_status, Status.Success,
+                                 f"Unexpected Success returned when expected {expected_status}")
+
         except InteractionModelError as e:
+            # The command failed, which might be what we expected, check if it is the expected error
             asserts.assert_equal(e.status, expected_status,
                                  "Unexpected error returned")
 
@@ -127,7 +142,12 @@ async def send_start_diagnostics_command(self, endpoint: int = None, timedReques
                                        endpoint=endpoint,
                                        timedRequestTimeoutMs=timedRequestTimeoutMs)
 
+            # If the command was expected to fail but it succeeded, check it wasn't meant to fail
+            asserts.assert_equal(expected_status, Status.Success,
+                                 f"Unexpected Success returned when expected {expected_status}")
+
         except InteractionModelError as e:
+            # The command failed, which might be what we expected, check if it is the expected error
             asserts.assert_equal(e.status, expected_status,
                                  "Unexpected error returned")
 
@@ -138,7 +158,12 @@ async def send_clear_targets_command(self, endpoint: int = None, timedRequestTim
                                        endpoint=endpoint,
                                        timedRequestTimeoutMs=timedRequestTimeoutMs)
 
+            # If the command was expected to fail but it succeeded, check it wasn't meant to fail
+            asserts.assert_equal(expected_status, Status.Success,
+                                 f"Unexpected Success returned when expected {expected_status}")
+
         except InteractionModelError as e:
+            # The command failed, which might be what we expected, check if it is the expected error
             asserts.assert_equal(e.status, expected_status,
                                  "Unexpected error returned")
 
@@ -149,7 +174,12 @@ async def send_get_targets_command(self, endpoint: int = None, timedRequestTimeo
                                                           endpoint=endpoint,
                                                           timedRequestTimeoutMs=timedRequestTimeoutMs)
 
+            # If the command was expected to fail but it succeeded, check it wasn't meant to fail
+            asserts.assert_equal(expected_status, Status.Success,
+                                 f"Unexpected Success returned when expected {expected_status}")
+
         except InteractionModelError as e:
+            # The command failed, which might be what we expected, check if it is the expected error
             asserts.assert_equal(e.status, expected_status,
                                  "Unexpected error returned")
 
@@ -165,7 +195,12 @@ async def send_set_targets_command(self, endpoint: int = None,
                                        endpoint=endpoint,
                                        timedRequestTimeoutMs=timedRequestTimeoutMs)
 
+            # If the command was expected to fail but it succeeded, check it wasn't meant to fail
+            asserts.assert_equal(expected_status, Status.Success,
+                                 f"Unexpected Success returned when expected {expected_status}")
+
         except InteractionModelError as e:
+            # The command failed, which might be what we expected, check if it is the expected error
             asserts.assert_equal(e.status, expected_status,
                                  "Unexpected error returned")
 
@@ -298,7 +333,7 @@ def compute_expected_target_time_as_epoch_s(self, minutes_past_midnight):
 
         logger.info(
             f"minutesPastMidnight = {minutes_past_midnight} => "
-            f"{int(minutes_past_midnight/60)}:{int(minutes_past_midnight%60)}"
+            f"{int(minutes_past_midnight/60)}:{int(minutes_past_midnight % 60)}"
             f" Expected target_time = {target_time}")
 
         # Matter Epoch is 1st Jan 2000

Original file line number	Diff line number	Diff line change
`@@ -93,6 +93,7 @@ CHIP_ERROR ChargingTargetsMemMgr::AllocAndCopy()`
`93`	`93`	`// ChargingTargetsMemMgr.h before this method can be called.`
`94`	`94`
`95`	`95`	`VerifyOrDie(mpListOfDays[mChargingTargetSchedulesIdx] == nullptr);`
	`96`	`+ VerifyOrReturnError(mNumDailyChargingTargets <= kEvseTargetsMaxTargetsPerDay, CHIP_ERROR_NO_MEMORY);`
`96`	`97`
`97`	`98`	`if (mNumDailyChargingTargets > 0)`
`98`	`99`	`{`
`@@ -124,6 +125,7 @@ ChargingTargetsMemMgr::AllocAndCopy(const DataModel::List<const Structs::Chargin`
`124`	`125`	`VerifyOrDie(mpListOfDays[mChargingTargetSchedulesIdx] == nullptr);`
`125`	`126`
`126`	`127`	`mNumDailyChargingTargets = static_cast<uint16_t>(chargingTargets.size());`
	`128`	`+ VerifyOrReturnError(mNumDailyChargingTargets <= kEvseTargetsMaxTargetsPerDay, CHIP_ERROR_NO_MEMORY);`
`127`	`129`
`128`	`130`	`if (mNumDailyChargingTargets > 0)`
`129`	`131`	`{`
`@@ -161,6 +163,7 @@ ChargingTargetsMemMgr::AllocAndCopy(const DataModel::DecodableList<Structs::Char`
`161`	`163`	`ReturnErrorOnFailure(chargingTargets.ComputeSize(&numDailyChargingTargets));`
`162`	`164`
`163`	`165`	`mNumDailyChargingTargets = static_cast<uint16_t>(numDailyChargingTargets);`
	`166`	`+ VerifyOrReturnError(mNumDailyChargingTargets <= kEvseTargetsMaxTargetsPerDay, CHIP_ERROR_NO_MEMORY);`
`164`	`167`
`165`	`168`	`if (mNumDailyChargingTargets > 0)`
`166`	`169`	`{`
Original file line number	Diff line number	Diff line change
`@@ -449,6 +449,12 @@ Status Instance::ValidateTargets(`
`449`	`449`	`innerIdx++;`
`450`	`450`	`}`
`451`	`451`
	`452`	`+ if (innerIdx > kEvseTargetsMaxTargetsPerDay)`
	`453`	`+ {`
	`454`	`+ ChipLogError(AppServer, "Too many targets in a single ChargingTargetScheduleStruct (%d)", innerIdx);`
	`455`	`+ return Status::ResourceExhausted;`
	`456`	`+ }`
	`457`	`+`
`452`	`458`	`if (iterInner.GetStatus() != CHIP_NO_ERROR)`
`453`	`459`	`{`
`454`	`460`	`return Status::InvalidCommand;`