Address comments from reviews (#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 <[email protected]>
project-chip · Aug 9, 2024 · a190f44 · a190f44
1 parent 3d890c5
commit a190f44
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 27 deletions.
diff --git a/examples/rvc-app/rvc-common/include/rvc-service-area-delegate.h b/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

diff --git a/examples/rvc-app/rvc-common/src/rvc-service-area-delegate.cpp b/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;

diff --git a/src/app/clusters/service-area-server/service-area-delegate.h b/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);

diff --git a/src/app/clusters/service-area-server/service-area-server.cpp b/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;
         }
     }