Start addressing comments from Andrei

Author: PeterC1965

examples/energy-management-app/energy-management-common/include/ChargingTargetsMemMgr.h

@@ -16,6 +16,18 @@
  *    limitations under the License.
  */
 
+#pragma once
+
+#include <app/clusters/energy-evse-server/energy-evse-server.h>
+#include <lib/core/CHIPError.h>
+
+#include <app-common/zap-generated/cluster-objects.h>
+
+namespace chip {
+namespace app {
+namespace Clusters {
+namespace EnergyEvse {
+
 /*
  * The full Target data structure defined as:
  *
@@ -67,28 +79,16 @@
  *
  */
 
-#pragma once
-
-#include <app/clusters/energy-evse-server/energy-evse-server.h>
-#include <lib/core/CHIPError.h>
-
-#include <app-common/zap-generated/cluster-objects.h>
-
-namespace chip {
-namespace app {
-namespace Clusters {
-namespace EnergyEvse {
-
 class ChargingTargetsMemMgr
 {
 public:
     ChargingTargetsMemMgr();
     ~ChargingTargetsMemMgr();
 
     void Reset(uint16_t chargingTargetSchedulesIdx);
-    void AddChargingTarget(EnergyEvse::Structs::ChargingTargetStruct::Type & chargingTarget);
-    void AllocAndCopy();
-    void AllocAndCopy(const DataModel::List<const Structs::ChargingTargetStruct::Type> & chargingTargets);
+    void AddChargingTarget(const EnergyEvse::Structs::ChargingTargetStruct::Type & chargingTarget);
+    CHIP_ERROR AllocAndCopy();
+    CHIP_ERROR AllocAndCopy(const DataModel::List<const Structs::ChargingTargetStruct::Type> & chargingTargets);
     CHIP_ERROR AllocAndCopy(const DataModel::DecodableList<Structs::ChargingTargetStruct::DecodableType> & chargingTargets);
 
     EnergyEvse::Structs::ChargingTargetStruct::Type * GetChargingTargets() const;

examples/energy-management-app/energy-management-common/include/EnergyEvseDelegateImpl.h

@@ -39,7 +39,7 @@ CHIP_ERROR GetEpochTS(uint32_t & chipEpoch);
  *
  * @param   reference to hold the day of week as a bitmap
  *
- * Sunday = 0x01, Monday = 0x01 ... Saturday = 0x40 (1<<6)
+ * Sunday = 0x00, Monday = 0x01 ... Saturday = 0x40 (1<<6)
  */
 CHIP_ERROR GetDayOfWeekNow(uint8_t & dayOfWeekMap);
 
@@ -57,6 +57,12 @@ CHIP_ERROR GetDayOfWeekNow(uint8_t & dayOfWeekMap);
  */
 uint8_t GetDayOfWeekUnixEpoch(time_t unixEpoch);
 
+/**
+ * @brief   Helper function to get current timestamp and work out the current number of minutes
+ *          past midnight based on localtime
+ *
+ * @param   reference to hold the number of minutes past midnight
+ */
 CHIP_ERROR GetMinutesPastMidnight(uint16_t & minutesPastMidnight);
 
 namespace chip {
@@ -309,11 +315,11 @@ class EnergyEvseDelegate : public EnergyEvse::Delegate
 
 private:
     /* Constants */
-    static constexpr int kDefaultMinChargeCurrent                     = 6000;  /* 6A */
-    static constexpr int kDefaultUserMaximumChargeCurrent             = 80000; /* 80A */
-    static constexpr int kDefaultRandomizationDelayWindow             = 600;   /* 600s */
+    static constexpr int kDefaultMinChargeCurrent_mA                     = 6000;  /* 6A */
+    static constexpr int kDefaultUserMaximumChargeCurrent_mA             = 80000; /* 80A */
+    static constexpr int kDefaultRandomizationDelayWindow_sec             = 600;   /* 600s */
     static constexpr int kMaxVehicleIDBufSize                         = 32;
-    static constexpr int kPeriodicCheckIntervalRealTimeClockNotSynced = 30;
+    static constexpr int kPeriodicCheckIntervalRealTimeClockNotSynced_sec = 30;
 
     /* private variables for controlling the hardware - these are not attributes */
     int64_t mMaxHardwareCurrentLimit                = 0; /* Hardware current limit in mA */
@@ -366,11 +372,11 @@ class EnergyEvseDelegate : public EnergyEvse::Delegate
     DataModel::Nullable<uint32_t> mChargingEnabledUntil;    // TODO Default to 0 to indicate disabled
     DataModel::Nullable<uint32_t> mDischargingEnabledUntil; // TODO Default to 0 to indicate disabled
     int64_t mCircuitCapacity           = 0;
-    int64_t mMinimumChargeCurrent      = kDefaultMinChargeCurrent;
+    int64_t mMinimumChargeCurrent      = kDefaultMinChargeCurrent_mA;
     int64_t mMaximumChargeCurrent      = 0;
     int64_t mMaximumDischargeCurrent   = 0;
-    int64_t mUserMaximumChargeCurrent  = kDefaultUserMaximumChargeCurrent; // TODO update spec
-    uint32_t mRandomizationDelayWindow = kDefaultRandomizationDelayWindow;
+    int64_t mUserMaximumChargeCurrent  = kDefaultUserMaximumChargeCurrent_mA; // TODO update spec
+    uint32_t mRandomizationDelayWindow = kDefaultRandomizationDelayWindow_sec;
     /* PREF attributes */
     DataModel::Nullable<uint32_t> mNextChargeStartTime;
     DataModel::Nullable<uint32_t> mNextChargeTargetTime;

examples/energy-management-app/energy-management-common/src/ChargingTargetsMemMgr.cpp

@@ -57,7 +57,7 @@ void ChargingTargetsMemMgr::Reset(uint16_t chargingTargetSchedulesIdx)
     }
 }
 
-void ChargingTargetsMemMgr::AddChargingTarget(EnergyEvse::Structs::ChargingTargetStruct::Type & chargingTarget)
+void ChargingTargetsMemMgr::AddChargingTarget(const EnergyEvse::Structs::ChargingTargetStruct::Type & chargingTarget)
 {
     if (mNumDailyChargingTargets < kEvseTargetsMaxTargetsPerDay)
     {
@@ -75,14 +75,19 @@ uint16_t ChargingTargetsMemMgr::GetNumDailyChargingTargets() const
     return mNumDailyChargingTargets;
 }
 
-void ChargingTargetsMemMgr::AllocAndCopy()
+CHIP_ERROR ChargingTargetsMemMgr::AllocAndCopy()
 {
     if (mNumDailyChargingTargets > 0)
     {
         // Allocate the memory first and then use placement new to initialise the memory of each element in the array
         mpListOfDays[mChargingTargetSchedulesIdx] = static_cast<EnergyEvse::Structs::ChargingTargetStruct::Type *>(
             chip::Platform::MemoryAlloc(sizeof(EnergyEvse::Structs::ChargingTargetStruct::Type) * mNumDailyChargingTargets));
 
+        if (mpListOfDays[mChargingTargetSchedulesIdx] == nullptr)
+        {
+            return CHIP_ERROR_NO_MEMORY;
+        }
+
         for (uint16_t idx = 0; idx < mNumDailyChargingTargets; idx++)
         {
             // This will cause the ChargingTargetStruct constructor to be called and this element in the array
@@ -92,9 +97,12 @@ void ChargingTargetsMemMgr::AllocAndCopy()
             mpListOfDays[mChargingTargetSchedulesIdx][idx] = mDailyChargingTargets[idx];
         }
     }
+
+    return CHIP_NO_ERROR;
 }
 
-void ChargingTargetsMemMgr::AllocAndCopy(const DataModel::List<const Structs::ChargingTargetStruct::Type> & chargingTargets)
+CHIP_ERROR
+ChargingTargetsMemMgr::AllocAndCopy(const DataModel::List<const Structs::ChargingTargetStruct::Type> & chargingTargets)
 {
     mNumDailyChargingTargets = static_cast<uint16_t>(chargingTargets.size());
 
@@ -104,6 +112,11 @@ void ChargingTargetsMemMgr::AllocAndCopy(const DataModel::List<const Structs::Ch
         mpListOfDays[mChargingTargetSchedulesIdx] = static_cast<EnergyEvse::Structs::ChargingTargetStruct::Type *>(
             chip::Platform::MemoryAlloc(sizeof(EnergyEvse::Structs::ChargingTargetStruct::Type) * chargingTargets.size()));
 
+        if (mpListOfDays[mChargingTargetSchedulesIdx] == nullptr)
+        {
+            return CHIP_ERROR_NO_MEMORY;
+        }
+
         uint16_t idx = 0;
         for (auto & chargingTarget : chargingTargets)
         {
@@ -116,6 +129,8 @@ void ChargingTargetsMemMgr::AllocAndCopy(const DataModel::List<const Structs::Ch
             idx++;
         }
     }
+
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR

examples/energy-management-app/energy-management-common/src/EnergyEvseDelegateImpl.cpp

@@ -200,7 +200,7 @@ Status EnergyEvseDelegate::ScheduleCheckOnEnabledTimeout()
     else if (err == CHIP_ERROR_REAL_TIME_NOT_SYNCED)
     {
         /* Real time isn't sync'd -lets check again in 30 seconds - otherwise keep the charger enabled */
-        DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds32(kPeriodicCheckIntervalRealTimeClockNotSynced),
+        DeviceLayer::SystemLayer().StartTimer(System::Clock::Seconds32(kPeriodicCheckIntervalRealTimeClockNotSynced_sec),
                                               EvseCheckTimerExpiry, this);
     }
     return Status::Success;

examples/energy-management-app/energy-management-common/src/EnergyEvseTargetsStore.cpp

@@ -176,7 +176,12 @@ CHIP_ERROR EvseTargetsDelegate::LoadTargets()
 
         // Allocate an array for the chargingTargets loaded for this schedule and copy the chargingTargets into that array.
         // The allocated array will be pointed to in the List below.
-        mChargingTargets.AllocAndCopy();
+        err = mChargingTargets.AllocAndCopy();
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(AppServer, "SetTargets: Failed to allocate memory during LoadTargets %s", chip::ErrorStr(err));
+            return err;
+        }
 
         // Construct the List<ChargingTargetStruct>. mChargingTargetSchedulesArray will be pointed to in the
         // List<ChargingTargetScheduleStruct> mChargingTargetSchedulesList below
@@ -304,7 +309,12 @@ CHIP_ERROR EvseTargetsDelegate::SetTargets(
                 updatedBitmask = BitMask<TargetDayOfWeekBitmap>(bitmaskB);
 
                 // Copy the existing chargingTargets
-                updatedChargingTargets.AllocAndCopy(currentChargingTargetSchedule.chargingTargets);
+                CHIP_ERROR err = updatedChargingTargets.AllocAndCopy(currentChargingTargetSchedule.chargingTargets);
+                if (err != CHIP_NO_ERROR)
+                {
+                    ChipLogError(AppServer, "SetTargets: Failed to copy the new chargingTargets %s", chip::ErrorStr(err));
+                    return err;
+                }
             }
 
             // Update the new schedule with the dayOfWeekForSequence and list of chargingTargets