Remove DuplicatedAreas error (#35126)

* Removed the DuplicatedAreas error from the XML

* Generated code after XML update.

* The service are server ignores any duplicate values before calling the delegate. Example was updated accodingly.

* Updated test SEAR-1.3 following changes to the duplicated areas error.

* Restyled by clang-format

* Made select areas const.

---------

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

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

@@ -79,8 +79,8 @@ class RvcServiceAreaDelegate : public Delegate
  // command support
  bool IsSetSelectedAreasAllowed(MutableCharSpan & statusText) override;
 
- bool IsValidSelectAreasSet(const ServiceArea::Commands::SelectAreas::DecodableType & req,
- ServiceArea::SelectAreasStatus & areaStatus, MutableCharSpan & statusText) override;
+ bool IsValidSelectAreasSet(const Span<const uint32_t> & selectedAreas, ServiceArea::SelectAreasStatus & areaStatus,
+ MutableCharSpan & statusText) override;
 
  bool HandleSkipArea(uint32_t skippedArea, MutableCharSpan & skipStatusText) override;
 

examples/rvc-app/rvc-common/rvc-app.matter

@@ -1438,9 +1438,8 @@ provisional cluster ServiceArea = 336 {
  enum SelectAreasStatus : enum8 {
  kSuccess = 0;
  kUnsupportedArea = 1;
- kDuplicatedAreas = 2;
- kInvalidInMode = 3;
- kInvalidSet = 4;
+ kInvalidInMode = 2;
+ kInvalidSet = 3;
  }
 
  enum SkipAreaStatus : enum8 {

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

@@ -113,23 +113,12 @@ bool RvcServiceAreaDelegate::IsSetSelectedAreasAllowed(MutableCharSpan & statusT
  return (mIsSetSelectedAreasAllowedDeviceInstance->*mIsSetSelectedAreasAllowedCallback)(statusText);
 };
 
-bool RvcServiceAreaDelegate::IsValidSelectAreasSet(const Commands::SelectAreas::DecodableType & req, SelectAreasStatus & areaStatus,
+bool RvcServiceAreaDelegate::IsValidSelectAreasSet(const Span<const uint32_t> & selectedAreas, SelectAreasStatus & areaStatus,
  MutableCharSpan & statusText)
 {
- // if req is empty list return true.
+ if (selectedAreas.empty())
  {
- size_t reqSize;
- if (req.newAreas.ComputeSize(&reqSize) != CHIP_NO_ERROR)
- {
- areaStatus = SelectAreasStatus::kInvalidSet; // todo Not sure this is the correct error to use here
- CopyCharSpanToMutableCharSpan("error computing number of selected areas"_span, statusText);
- return false;
- }
-
- if (reqSize == 0)
- {
- return true;
- }
+ return true;
  }
 
  // If there is 1 or 0 supported maps, any combination of areas is valid.
@@ -139,42 +128,34 @@ bool RvcServiceAreaDelegate::IsValidSelectAreasSet(const Commands::SelectAreas::
  }
 
  // Check that all the requested areas are in the same map.
- auto newAreasIter = req.newAreas.begin();
- newAreasIter.Next();
-
- AreaStructureWrapper tempArea;
- uint32_t ignoredIndex;
- if (!GetSupportedAreaById(newAreasIter.GetValue(), ignoredIndex, tempArea))
  {
- areaStatus = SelectAreasStatus::kUnsupportedArea;
- CopyCharSpanToMutableCharSpan("unable to find selected area in supported areas"_span, statusText);
- return false;
- }
-
- auto mapId = tempArea.mapID.Value(); // It is safe to call `.Value()` as we confirmed that there are at least 2 maps.
-
- while (newAreasIter.Next())
- {
- if (!GetSupportedAreaById(newAreasIter.GetValue(), ignoredIndex, tempArea))
+ AreaStructureWrapper tempArea;
+ uint32_t ignoredIndex;
+ if (!GetSupportedAreaById(selectedAreas[0], ignoredIndex, tempArea))
  {
  areaStatus = SelectAreasStatus::kUnsupportedArea;
  CopyCharSpanToMutableCharSpan("unable to find selected area in supported areas"_span, statusText);
  return false;
  }
 
- if (tempArea.mapID.Value() != mapId)
+ auto mapId = tempArea.mapID.Value(); // It is safe to call `.Value()` as we confirmed that there are at least 2 maps.
+
+ for (const auto & areaId : selectedAreas.SubSpan(1))
  {
- areaStatus = SelectAreasStatus::kInvalidSet;
- CopyCharSpanToMutableCharSpan("all selected areas must be in the same map"_span, statusText);
- return false;
- }
- }
+ if (!GetSupportedAreaById(areaId, ignoredIndex, tempArea))
+ {
+ areaStatus = SelectAreasStatus::kUnsupportedArea;
+ CopyCharSpanToMutableCharSpan("unable to find selected area in supported areas"_span, statusText);
+ return false;
+ }
 
- if (CHIP_NO_ERROR != newAreasIter.GetStatus())
- {
- areaStatus = SelectAreasStatus::kInvalidSet;
- CopyCharSpanToMutableCharSpan("error processing new areas."_span, statusText);
- return false;
+ if (tempArea.mapID.Value() != mapId)
+ {
+ areaStatus = SelectAreasStatus::kInvalidSet;
+ CopyCharSpanToMutableCharSpan("all selected areas must be in the same map"_span, statusText);
+ return false;
+ }
+ }
  }
 
  return true;

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

@@ -80,18 +80,18 @@ class Delegate
  * If the set of locations is invalid, the locationStatus should be set to InvalidSet and
  * the statusText SHALL include a vendor-defined error description.
  *
- * The caller of this method will ensure that there are no duplicates is the list
+ * The caller of this method will ensure that there are no duplicates in the list
  * and that all the locations in the set are valid supported locations.
  *
- * @param[in] req List of new selected locations.
+ * @param[in] selectedAreas List of new selected locations.
  * @param[out] locationStatus Success if all checks pass, error code if failure.
  * @param[out] statusText text describing failure (see description above), size kMaxSizeStatusText.
  * @return true if success.
  *
  * @note If the SelectAreas command is allowed when the device is operating and the selected locations change to none, the
  * device must stop.
  */
- virtual bool IsValidSelectAreasSet(const Commands::SelectAreas::DecodableType & req, SelectAreasStatus & locationStatus,
+ virtual bool IsValidSelectAreasSet(const Span<const uint32_t> & selectedAreas, SelectAreasStatus & locationStatus,
  MutableCharSpan & statusText) = 0;
 
  /**

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

@@ -240,64 +240,72 @@ void Instance::HandleSelectAreasCmd(HandlerContext & ctx, const Commands::Select
  }
  }
 
+ uint32_t selectedAreasBuffer[kMaxNumSelectedAreas];
+ auto selectedAreasSpan = Span<uint32_t>(selectedAreasBuffer, kMaxNumSelectedAreas);
+ uint32_t numberOfSelectedAreas = 0;
+
+ // Closure for checking if an area ID exists in the selectedAreasSpan
+ auto areaAlreadyExists = [&numberOfSelectedAreas, &selectedAreasSpan](uint32_t areaId) {
+ for (uint32_t i = 0; i < numberOfSelectedAreas; i++)
+ {
+ if (areaId == selectedAreasSpan[i])
+ {
+ return true;
+ }
+ }
+ return false;
+ };
+
  // if number of selected locations in parameter matches number in attribute - the locations *might* be the same
  bool matchesCurrentSelectedAreas = (numberOfAreas == mDelegate->GetNumberOfSelectedAreas());
 
+ // do as much parameter validation as we can
  if (numberOfAreas != 0)
  {
- // do as much parameter validation as we can
+ uint32_t ignoredIndex = 0;
+ uint32_t oldSelectedArea;
+ auto iAreaIter = req.newAreas.begin();
+ while (iAreaIter.Next())
  {
- uint32_t ignoredIndex = 0;
- uint32_t oldSelectedArea;
- uint32_t i = 0;
- auto iAreaIter = req.newAreas.begin();
- while (iAreaIter.Next())
- {
- uint32_t aSelectedArea = iAreaIter.GetValue();
+ uint32_t selectedArea = iAreaIter.GetValue();
 
- // each item in this list SHALL match the AreaID field of an entry on the SupportedAreas attribute's list
- // If the Status field is set to UnsupportedArea, the StatusText field SHALL be an empty string.
- if (!IsSupportedArea(aSelectedArea))
- {
- exitResponse(SelectAreasStatus::kUnsupportedArea, ""_span);
- return;
- }
+ // If aSelectedArea is already in selectedAreasSpan skip
+ if (areaAlreadyExists(selectedArea))
+ {
+ continue;
+ }
 
- // Checking for duplicate locations.
- uint32_t j = 0;
- auto jAreaIter = req.newAreas.begin();
- while (j < i)
- {
- jAreaIter.Next(); // Since j < i and i is valid, we can safely call Next() without checking the return value.
- if (jAreaIter.GetValue() == aSelectedArea)
- {
- exitResponse(SelectAreasStatus::kDuplicatedAreas, ""_span);
- return;
- }
- j += 1;
- }
+ // each item in this list SHALL match the AreaID field of an entry on the SupportedAreas attribute's list
+ // If the Status field is set to UnsupportedArea, the StatusText field SHALL be an empty string.
+ if (!IsSupportedArea(selectedArea))
+ {
+ exitResponse(SelectAreasStatus::kUnsupportedArea, ""_span);
+ return;
+ }
 
- // check to see if parameter list and attribute still match
- if (matchesCurrentSelectedAreas)
+ // check to see if parameter list and attribute still match
+ if (matchesCurrentSelectedAreas)
+ {
+ if (!mDelegate->GetSelectedAreaByIndex(ignoredIndex, oldSelectedArea) || (selectedArea != oldSelectedArea))
  {
- if (!mDelegate->GetSelectedAreaByIndex(ignoredIndex, oldSelectedArea) || (aSelectedArea != oldSelectedArea))
- {
- matchesCurrentSelectedAreas = false;
- }
+ matchesCurrentSelectedAreas = false;
  }
-
- i += 1;
  }
 
- // after iterating with Next through DecodableType - check for failure
- if (CHIP_NO_ERROR != iAreaIter.GetStatus())
- {
- ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand);
- return;
- }
+ selectedAreasSpan[numberOfSelectedAreas] = selectedArea;
+ numberOfSelectedAreas += 1;
+ }
+
+ // after iterating with Next through DecodableType - check for failure
+ if (CHIP_NO_ERROR != iAreaIter.GetStatus())
+ {
+ ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::InvalidCommand);
+ return;
  }
  }
 
+ selectedAreasSpan.reduce_size(numberOfSelectedAreas);
+
  // If the newAreas field is the same as the value of the SelectedAreas attribute
  // the SelectAreasResponse command SHALL have the Status field set to Success and
  // the StatusText field MAY be supplied with a human-readable string or include an empty string.
@@ -327,7 +335,7 @@ void Instance::HandleSelectAreasCmd(HandlerContext & ctx, const Commands::Select
  // ask the device to handle SelectAreas Command
  // (note - locationStatusText to be filled out by delegated function for kInvalidInMode and InvalidSet)
  auto locationStatus = SelectAreasStatus::kSuccess;
- if (!mDelegate->IsValidSelectAreasSet(req, locationStatus, delegateStatusText))
+ if (!mDelegate->IsValidSelectAreasSet(selectedAreasSpan, locationStatus, delegateStatusText))
  {
  exitResponse(locationStatus, delegateStatusText);
  return;
@@ -342,11 +350,10 @@ void Instance::HandleSelectAreasCmd(HandlerContext & ctx, const Commands::Select
 
  if (numberOfAreas != 0)
  {
- auto locationIter = req.newAreas.begin();
  uint32_t ignored;
- while (locationIter.Next())
+ for (uint32_t areaId : selectedAreasSpan)
  {
- mDelegate->AddSelectedArea(locationIter.GetValue(), ignored);
+ mDelegate->AddSelectedArea(areaId, ignored);
  }
  }
 

src/app/zap-templates/zcl/data-model/chip/service-area-cluster.xml

@@ -63,9 +63,8 @@ limitations under the License.
  <cluster code="0x0150"/>
  <item value="0x00" name="Success"/>
  <item value="0x01" name="UnsupportedArea"/>
- <item value="0x02" name="DuplicatedAreas"/>
- <item value="0x03" name="InvalidInMode"/>
- <item value="0x04" name="InvalidSet"/>
+ <item value="0x02" name="InvalidInMode"/>
+ <item value="0x03" name="InvalidSet"/>
  </enum>
 
  <enum name="SkipAreaStatus" type="enum8">

src/controller/data_model/controller-clusters.matter

@@ -6463,9 +6463,8 @@ provisional cluster ServiceArea = 336 {
  enum SelectAreasStatus : enum8 {
  kSuccess = 0;
  kUnsupportedArea = 1;
- kDuplicatedAreas = 2;
- kInvalidInMode = 3;
- kInvalidSet = 4;
+ kInvalidInMode = 2;
+ kInvalidSet = 3;
  }
 
  enum SkipAreaStatus : enum8 {

src/python_testing/TC_SEAR_1_3.py

@@ -109,15 +109,17 @@ async def test_TC_SEAR_1_3(self):
 
  duplicated_areas = [valid_area_id, valid_area_id]
 
- # FIXME need to check if this is the correct name of this status code
- await self.send_cmd_select_areas_expect_response(step=3, new_areas=duplicated_areas, expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kDuplicatedAreas)
+ await self.send_cmd_select_areas_expect_response(step=3, new_areas=duplicated_areas, expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
 
- await self.send_cmd_select_areas_expect_response(step=4, new_areas=[], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
+ selected_areas = await self.read_selected_areas(step=4)
+ asserts.assert_true(selected_areas == [valid_area_id], "SelectedAreas should be empty")
 
- selected_areas = await self.read_selected_areas(step=5)
+ await self.send_cmd_select_areas_expect_response(step=5, new_areas=[], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
+
+ selected_areas = await self.read_selected_areas(step=6)
  asserts.assert_true(len(selected_areas) == 0, "SelectedAreas should be empty")
 
- await self.send_cmd_select_areas_expect_response(step=6, new_areas=[invalid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kUnsupportedArea)
+ await self.send_cmd_select_areas_expect_response(step=8, new_areas=[invalid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kUnsupportedArea)
 
  if self.check_pics("SEAR.S.M.INVALID_STATE_FOR_SELECT_AREAS") and self.check_pics("SEAR.S.M.HAS_MANUAL_SELAREA_STATE_CONTROL"):
  test_step = "Manually intervene to put the device in a state that prevents it from executing the SelectAreas command"
@@ -127,7 +129,7 @@ async def test_TC_SEAR_1_3(self):
  else:
  self.wait_for_user_input(prompt_msg=f"{test_step}, and press Enter when done.\n")
 
- await self.send_cmd_select_areas_expect_response(step=8, new_areas=[valid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kInvalidInMode)
+ await self.send_cmd_select_areas_expect_response(step=10, new_areas=[valid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kInvalidInMode)
 
  if self.check_pics("SEAR.S.M.VALID_STATE_FOR_SELECT_AREAS") and self.check_pics("SEAR.S.M.HAS_MANUAL_SELAREA_STATE_CONTROL"):
  test_step = f"Manually intervene to put the device in a state that allows it to execute the SelectAreas({supported_area_ids}) command"
@@ -137,27 +139,27 @@ async def test_TC_SEAR_1_3(self):
  else:
  self.wait_for_user_input(prompt_msg=f"{test_step}, and press Enter when done.\n")
 
- await self.send_cmd_select_areas_expect_response(step=10, new_areas=supported_area_ids, expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
+ await self.send_cmd_select_areas_expect_response(step=11, new_areas=supported_area_ids, expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
 
- selected_areas = await self.read_selected_areas(step=11)
+ selected_areas = await self.read_selected_areas(step=12)
  asserts.assert_true(len(selected_areas) == len(supported_area_ids),
  f"SelectedAreas({selected_areas}) should match SupportedAreas({supported_area_ids})")
 
- await self.send_cmd_select_areas_expect_response(step=12, new_areas=supported_area_ids, expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
+ await self.send_cmd_select_areas_expect_response(step=13, new_areas=supported_area_ids, expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
 
  if self.check_pics("SEAR.S.M.VALID_STATE_FOR_SELECT_AREAS") and self.check_pics("SEAR.S.M.HAS_MANUAL_SELAREA_STATE_CONTROL") and self.check_pics("SEAR.S.M.SELECT_AREAS_WHILE_NON_IDLE"):
  test_step = f"Manually intervene to put the device in a state that allows it to execute the SelectAreas({valid_area_id}) command, and put the device in a non-idle state"
- self.print_step("13", test_step)
+ self.print_step("14", test_step)
  if self.is_ci:
  self.write_to_app_pipe('{"Name": "Reset"}')
  await self.send_single_cmd(cmd=Clusters.Objects.RvcRunMode.Commands.ChangeToMode(newMode=1), endpoint=self.endpoint)
  else:
  self.wait_for_user_input(prompt_msg=f"{test_step}, and press Enter when done.\n")
 
  if self.check_pics("SEAR.S.F00"):
- await self.send_cmd_select_areas_expect_response(step=14, new_areas=[valid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
+ await self.send_cmd_select_areas_expect_response(step=15, new_areas=[valid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kSuccess)
  else:
- await self.send_cmd_select_areas_expect_response(step=14, new_areas=[valid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kInvalidInMode)
+ await self.send_cmd_select_areas_expect_response(step=15, new_areas=[valid_area_id], expected_response=Clusters.ServiceArea.Enums.SelectAreasStatus.kInvalidInMode)
 
 
 if __name__ == "__main__":