Address comments from reviews (project-chip#34884)

* Updated the golabl data type's XMLs, removing the cluster entries.

* Zap generated after XML update.

* Fixed namespaces used of global structs.

* Restyled by clang-format

* Renamed LocationInfoStruct to AreaInfoStruct.

* Zap generated after XML update.

* Renamed LocationStruct to AreaStruct and its LocationID and LocationDesc fields.

* Zap generated after XML update.

* Updated SDK and example code to match the new naming.

* Updated the ProgressStruct's LocationID name to AreaID.

* Zap generated after XML update.

* Updated the SDK code following name changes.

* Updated the SelectLocationsStatus and SkipLocationStatus enum names and some of their enums.

* Zap generated after XML update.

* Updated the SelectLocationsStatus and SkipCurrentLocationStatus names and their enum names.

* Updated the names of the SupportedLocations, SelectedLocations and CurrentLocation attributes.

* Zap generated after XML update.

* Updated the changed names in the SDK.

* Updated the service area command names in XML.

* Zap generated after XML update.

* Updated the service area command names in the SDK.

* Updated the rvc-example zap file.

* Refactored LocationStructureWrapper to AreaStructureWrapper.

* Restyled by clang-format

* Regenerated zap files due to changes upsteram.

* Removed unused generated file.

* Updated the Service Area XML marking previously nullabel attributes as not-nullable.

* Zap generated after XML update.

* Updated the attribute encoding and some server logic following the romoval of the nullable quality for some attributes.

* Explicitly set the Service Area version.

* Zap generated after XML update.

* Updated the service area features in the XML to match the current spec.

* Zap generated after XML update.

* Updated the SupportedArea validation logic as if the MAPS feature is not supported, the Delegate may choose not to implement map related methods.

* Change the type of the MapID to uint32 to match the spec.

* Added the SkippedArea arg to the SkipArea command.

* Zap generated after XML update.

* Updated the Service Area server code to handle the new SkippedArea command arg.

* Updated the service area XML to match the current spec. This includes the addition of the LandmarkInfoStruct and updates of AreaInfoStruct, SelectAreasStatus.

* Zap generated after XML update.

* Updated SDK server code and rvc-example after changes to the XML.

* Restyled by whitespace

* added vector include.

* spacing changes form zap regen.

* Fixed minor mistake during merge.

* Restyled by clang-format

* Applied suggestions from review.

* Restyled by whitespace

* Updated the LondmarkInfoSturct PositionTag type.

* Zap generated after XML update.

* Fixed SDK following update to the position type.

* Restyled by clang-format

* Updated the AeraStructWrapper to not contain methods with a large number of params. Updated some relate Instance and Delegate methods.

* Restyled by whitespace

* Restyled by clang-format

* Missed a submudule update.

* Made the setters reture a ref to the sturct to allow chaining.

* simplified the = oporator and add the == operator.

* Restyled by clang-format

* minor change to get restyler going.

* Restyled by clang-format

* Fixed status text referance issue.

* Fixed issue casued by a change in the way that registrations are made.

* Fixed styling and minor bug in IsValidSupportedArea.

---------

Co-authored-by: Restyled.io <commits@restyled.io>

Author: 2 people

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;
         }
     }