Address comments from reviews

examples/rvc-app/rvc-common/include/rvc-service-area-delegate.h

@@ -44,12 +44,12 @@ class RvcServiceAreaDelegate : public Delegate
     CHIP_ERROR Init() override;
 
     // command support
-    bool IsSetSelectedAreasAllowed(MutableCharSpan statusText) override;
+    bool IsSetSelectedAreasAllowed(MutableCharSpan & statusText) override;
 
     bool IsValidSelectAreasSet(const ServiceArea::Commands::SelectAreas::DecodableType & req,
-                               ServiceArea::SelectAreasStatus & areaStatus, MutableCharSpan statusText) override;
+                               ServiceArea::SelectAreasStatus & areaStatus, MutableCharSpan & statusText) override;
 
-    bool HandleSkipCurrentArea(uint32_t skippedArea, MutableCharSpan skipStatusText) override;
+    bool HandleSkipCurrentArea(uint32_t skippedArea, MutableCharSpan & skipStatusText) override;
 
     //*************************************************************************
     // Supported Areas accessors

examples/rvc-app/rvc-common/src/rvc-service-area-delegate.cpp

@@ -77,20 +77,20 @@ CHIP_ERROR RvcServiceAreaDelegate::Init()
 //*************************************************************************
 // command support
 
-bool RvcServiceAreaDelegate::IsSetSelectedAreasAllowed(MutableCharSpan statusText)
+bool RvcServiceAreaDelegate::IsSetSelectedAreasAllowed(MutableCharSpan & statusText)
 {
     // TODO IMPLEMENT
     return true;
 };
 
 bool RvcServiceAreaDelegate::IsValidSelectAreasSet(const Commands::SelectAreas::DecodableType & req, SelectAreasStatus & areaStatus,
-                                                   MutableCharSpan statusText)
+                                                   MutableCharSpan & statusText)
 {
     // TODO IMPLEMENT
     return true;
 };
 
-bool RvcServiceAreaDelegate::HandleSkipCurrentArea(uint32_t skippedArea, MutableCharSpan skipStatusText)
+bool RvcServiceAreaDelegate::HandleSkipCurrentArea(uint32_t skippedArea, MutableCharSpan & skipStatusText)
 {
     // TODO IMPLEMENT
     return true;

src/app/clusters/service-area-server/service-area-delegate.h

@@ -72,7 +72,7 @@ class Delegate
      * @note The statusText field SHOULD indicate why the request is not allowed, given the current mode
      *       of the device, which may involve other clusters.
      */
-    virtual bool IsSetSelectedAreasAllowed(MutableCharSpan statusText) = 0;
+    virtual bool IsSetSelectedAreasAllowed(MutableCharSpan & statusText) = 0;
 
     /**
      * Given a set of locations to be set to the SelectedAreas attribute, this method should check that
@@ -92,7 +92,7 @@ class Delegate
      * device must stop.
      */
     virtual bool IsValidSelectAreasSet(const Commands::SelectAreas::DecodableType & req, SelectAreasStatus & locationStatus,
-                                       MutableCharSpan statusText) = 0;
+                                       MutableCharSpan & statusText) = 0;
 
     /**
      * @brief The server instance ensures that the SelectedAreas and CurrentArea attributes are not null before
@@ -121,7 +121,7 @@ class Delegate
      * InvalidInMode, the StatusText field SHOULD indicate why the request is not allowed, given the current mode of the device,
      * which may involve other clusters.
      */
-    virtual bool HandleSkipCurrentArea(uint32_t skippedArea, MutableCharSpan skipStatusText)
+    virtual bool HandleSkipCurrentArea(uint32_t skippedArea, MutableCharSpan & skipStatusText)
     {
         // device support of this command is optional
         CopyCharSpanToMutableCharSpan("Skip Current Area command not supported by device"_span, skipStatusText);

src/app/clusters/service-area-server/service-area-server.cpp

@@ -399,6 +399,8 @@ void Instance::HandleSkipCurrentAreaCmd(HandlerContext & ctx, const Commands::Sk
         exitResponse(SkipAreaStatus::kInvalidInMode, skipStatusText);
         return;
     }
+
+    exitResponse(SkipAreaStatus::kSuccess, ""_span);
 }
 
 //*************************************************************************
@@ -469,35 +471,28 @@ bool Instance::IsValidSupportedArea(const AreaStructureWrapper & aArea)
     }
 
     // The mapID field SHALL be null if SupportedMaps is not supported or SupportedMaps is an empty list.
-    bool shouldMapsBeNull = false;
-    if (mFeature.Has(Feature::kMaps))
+    if (mFeature.Has(Feature::kMaps) && (mDelegate->GetNumberOfSupportedMaps() > 0))
     {
-        if (mDelegate->GetNumberOfSupportedMaps() == 0)
+        if (aArea.mapID.IsNull())
         {
-            shouldMapsBeNull = true;
+            ChipLogDetail(Zcl, "IsValidSupportedArea %u - map Id should not be null when there are supported maps", aArea.areaID);
+            return false;
         }
-    }
-    else
-    {
-        shouldMapsBeNull = true;
-    }
 
-    if (shouldMapsBeNull)
-    {
-        if (!aArea.mapID.IsNull())
+        // If the SupportedMaps attribute is not null, mapID SHALL be the ID of an entry from the SupportedMaps attribute.
+        if (!IsSupportedMap(aArea.mapID.Value()))
         {
-            ChipLogDetail(Zcl, "IsValidSupportedArea %u - map Id %u is not in empty supported map list", aArea.areaID,
-                          aArea.mapID.Value());
+            ChipLogError(Zcl, "IsValidSupportedArea %u - map Id %u is not in supported map list", aArea.areaID,
+                         aArea.mapID.Value());
             return false;
         }
     }
     else
     {
-        // If the SupportedMaps attribute is not null, mapID SHALL be the ID of an entry from the SupportedMaps attribute.
-        if (!IsSupportedMap(aArea.mapID.Value()))
+        if (!aArea.mapID.IsNull())
         {
-            ChipLogError(Zcl, "IsValidSupportedArea %u - map Id %u is not in supported map list", aArea.areaID,
-                         aArea.mapID.Value());
+            ChipLogDetail(Zcl, "IsValidSupportedArea %u - map Id %u is not in empty supported map list", aArea.areaID,
+                          aArea.mapID.Value());
             return false;
         }
     }