Updating matter rvc ux #2343

HunsupJung · 2025-08-15T10:23:45Z

Type of Change

WWST Certification Request
- If this is your first time contributing code:
  - I have reviewed the README.md file
  - I have reviewed the CODE_OF_CONDUCT.md file
  - I have signed the CLA
- I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
Bug fix
New feature
Refactor

Checklist

I have performed a self-review of my code
I have commented my code in hard-to-understand areas
I have verified my changes by testing with a device or have communicated a plan for testing
I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

The unnecessary mode capability for Run Mode cluster has been deleted.

Summary of Completed Tests

This feature is being tested with following TC in SVT.
https://smartthings.atlassian.net/wiki/spaces/STHK/pages/4088660856/REQ-22349+Matter+RVC+UX+Upgrade+TC

- Integrate components - Remove mode capability for run mode cluster Signed-off-by: HunsupJung <[email protected]>

Signed-off-by: HunsupJung <[email protected]>

github-actions · 2025-08-15T10:24:12Z

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions · 2025-08-15T10:24:43Z

Invitation URL:
https://bestow-regional.api.smartthings.com/invite/1J2QnYXzqkl0

github-actions · 2025-08-15T10:26:01Z

Test Results

69 files 449 suites 0s ⏱️
2 336 tests 2 336 ✅ 0 💤 0 ❌
3 929 runs 3 929 ✅ 0 💤 0 ❌

Results for commit 0716447.

♻️ This comment has been updated with latest results.

github-actions · 2025-08-15T10:26:02Z

File	Coverage
All files	`73%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/Global/types/AreaTypeTag.lua	`66%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/Global/types/LandmarkTag.lua	`67%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/Global/types/RelativePositionTag.lua	`68%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/Global/types/LocationDescriptorStruct.lua	`63%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/ServiceArea/types/LandmarkInfoStruct.lua	`59%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/ServiceArea/types/SelectAreasStatus.lua	`69%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/ServiceArea/types/AreaInfoStruct.lua	`59%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/ServiceArea/types/AreaStruct.lua	`63%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/init.lua	`89%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/embedded_cluster_utils.lua	`72%`	❌

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 0716447

Signed-off-by: HunsupJung <[email protected]>

nickolas-deboom · 2025-08-18T19:36:18Z

drivers/SmartThings/matter-rvc/profiles/rvc-clean-mode-service-area.yml

@@ -5,6 +5,8 @@ components:
    capabilities:
      - id: robotCleanerOperatingState


Are these profiles currently in use by devices? If so, it might be better to create new profiles rather than removing capabilities to avoid breaking automations / rules for devices in the field.

I confirmed these profiles are used by devices, and that automations are being used on the mode operation for at least a handful, though the percentage is small.

Instead of adding new profiles, I believe we should modify the system to use the benefits of modular profiles.

@hcarter-775
Can we use the modular profiles by 9/9? If so, could you share the way to use the modular profiles?

nickolas-deboom · 2025-08-18T19:38:50Z

drivers/SmartThings/matter-rvc/src/init.lua

+local function component_to_endpoint(device, component_name)
+  -- Use the find_default_endpoint function to return the first endpoint that
+  -- supports a given cluster.
+  return find_default_endpoint(device, clusters.RvcOperationalState.ID)


What does the endpoint structure typically look like for a RVC? Are the clusters usually all implemented on the same endpoint?

I not sure, but Roborock implements Power Source, RVC Run Mode, RVC Clean Mode, RVC Operational State, and Service Area on the endpoint 1.

Signed-off-by: HunsupJung <[email protected]>

hcarter-775 · 2025-09-04T18:23:29Z

drivers/SmartThings/matter-rvc/src/init.lua

+  if not device:get_field(SERVICE_AREA_PROFILED) and #device:get_endpoints(clusters.ServiceArea.ID) > 0 then
+    match_profile(driver, device)
+    device:set_field(SERVICE_AREA_PROFILED, true)
+  end


Suggested change

if not device:get_field(SERVICE_AREA_PROFILED) and #device:get_endpoints(clusters.ServiceArea.ID) > 0 then

match_profile(driver, device)

device:set_field(SERVICE_AREA_PROFILED, true)

end

if not device:get_field(SERVICE_AREA_PROFILED) then

if #device:get_endpoints(clusters.ServiceArea.ID) > 0 then

match_profile(driver, device)

end

device:set_field(SERVICE_AREA_PROFILED, true, { persist = true })

end

we only want this logic to run once, and we want to set the device as profiled whether or not service area is part of the device

Okay, I got it.

hcarter-775 · 2025-09-04T18:24:32Z

drivers/SmartThings/matter-rvc/src/init.lua

+
+local function do_configure(driver, device)
+  match_profile(driver, device)
+  device:set_field(SERVICE_AREA_PROFILED, true)


Suggested change

device:set_field(SERVICE_AREA_PROFILED, true)

device:set_field(SERVICE_AREA_PROFILED, true, { persist = true })

this field should not be reset on a driver restart

hcarter-775 · 2025-09-04T18:29:23Z

drivers/SmartThings/matter-rvc/src/init.lua

 local RUN_MODE_SUPPORTED_MODES = "__run_mode_supported_modes"
+local CURRENT_RUN_MODE = "__current_run_mode"


why do we need to set CURRENT_RUN_MODE instead of using the get_latest_state api as we did before? And would you mind adding a short comment by this field to explain this?

mode capability related to run mode is going to be deleted. So CURRENT_RUN_MODE value is needed to store the latest CurrentMode value of RVC Run Mode Cluster

hcarter-775 · 2025-09-04T18:32:54Z

drivers/SmartThings/matter-rvc/src/init.lua

@@ -32,10 +32,11 @@ if version.api < 13 then
  clusters.Global = require "Global"
 end

-local COMPONENT_TO_ENDPOINT_MAP = "__component_to_endpoint_map"


If we're removing this field, can you please set it as null in the init function? we can leave this local variable deleted and just write something like:

-- comp/ep map functionality removed 9/5/25. device:set_field("__component_to_endpoint_map", nil)

Okay, I got it.

hcarter-775 · 2025-09-04T18:35:45Z

drivers/SmartThings/matter-rvc/src/init.lua

+  local eps = device:get_endpoints(cluster)
+  table.sort(eps)
+  for _, v in ipairs(eps) do
+    if v ~= 0 then --0 is the matter RootNode endpoint


I don't understand why we're ignoring the root endpoint here?

When the manufacturer create a ZAP file through the ZAP tool, number 0 will be used as Root node and the device type that is wanted to add will be 1. Of course, the manufacturer can set the endpoint arbitrarily.
This code is also included in all other Matter drivers, so if we remove it, it would be better to remove it all at once in the future.

hcarter-775 · 2025-09-04T18:39:53Z

drivers/SmartThings/matter-rvc/src/init.lua

-    capabilities.mode.ID,
-    capabilities.mode.mode.NAME
-  )
+  local current_run_mode = device:get_field(CURRENT_RUN_MODE)


not sure where to comment about this, but line 534 of the current PR is:
endpoint_id = device:component_to_endpoint("runMode")
we need to remove this reference. I wonder also how this was tested where this was missed? Is this logic required at all anymore?

Thank you for finding the part. As you mentioned, we need to change "runMode" to cmd.component.

The reason why it was not a problem during the test is that component_to_endpoint finds an endpoint with RvcOperationalState cluster regardless of what component is put in.

hcarter-775 · 2025-09-04T18:42:51Z

drivers/SmartThings/matter-rvc/src/init.lua

-    ["cleanMode"] = clean_mode_eps[1]
-  }
-  device:set_field(COMPONENT_TO_ENDPOINT_MAP, component_to_endpoint_map, {persist = true})
+local function component_to_endpoint(device, component_name)


I suggest we remove this function in all places in the driver and directly use find_default_endpoint with the appropriate cluster that the command handler should be mapping to.

Okay, I got it.

hcarter-775

these are some initial comments/questions I have

hcarter-775 · 2025-09-04T18:46:12Z

drivers/SmartThings/matter-rvc/src/init.lua

-  else
-    return device.MATTER_DEFAULT_ENDPOINT
+local function find_default_endpoint(device, cluster)
+  local res = device.MATTER_DEFAULT_ENDPOINT


let's remove the res variable setting and just return default.MATTER_DEFAULT_ENDPOINT explicitly at the end?

Okay, I got it.

hcarter-775 · 2025-09-04T18:48:03Z

drivers/SmartThings/matter-rvc/profiles/rvc-clean-mode-service-area.yml

-  - component: cleanMode
-    capability: mode
+  - component: main
+    capability: serviceArea


why was the location of this capability moved in this definition?

There is no reason. I wanted to emphasize the relation between mode and robotCleanerOperatingState.

drivers/SmartThings/matter-rvc/src/init.lua

Signed-off-by: HunsupJung <[email protected]>

hcarter-775 · 2025-09-05T16:14:57Z

drivers/SmartThings/matter-rvc/src/init.lua

-local function find_default_endpoint(device, cluster)
-  local res = device.MATTER_DEFAULT_ENDPOINT
-  local eps = device:get_endpoints(cluster)
+local function find_default_endpoint(device, component_name)


sorry, I don't think I was completely clear before. Instead of using the component_name as an argument here, I think we should make the cluster be the argument, and call this function for whatever cluster is needed in each handler. I'm thinking something like:

local function find_default_endpoint(device, cluster_id) local eps = device:get_endpoints(cluster_id) ... leave the rest the same ... end

and then change what is passed in at the callsite.

hcarter-775 · 2025-09-05T16:16:12Z

drivers/SmartThings/matter-rvc/src/init.lua

-  if not device:get_field(SERVICE_AREA_PROFILED) and #device:get_endpoints(clusters.ServiceArea.ID) > 0 then
-    match_profile(driver, device)
-    device:set_field(SERVICE_AREA_PROFILED, true)
+  device:set_component_to_endpoint_fn(find_default_endpoint)


we no longer require this line if we go the route of my previous suggestion

HunsupJung added 2 commits August 15, 2025 17:20

Upgrade matter-rvc driver

ceae111

- Integrate components - Remove mode capability for run mode cluster Signed-off-by: HunsupJung <[email protected]>

Update Test Unit

355860d

Signed-off-by: HunsupJung <[email protected]>

HunsupJung requested review from z-michel, hcarter-775 and nickolas-deboom August 15, 2025 10:23

HunsupJung changed the title ~~Feature/updating matter rvc ux~~ Updating matter rvc ux Aug 15, 2025

Fix failed part on luacheck

df9744e

Signed-off-by: HunsupJung <[email protected]>

HunsupJung force-pushed the feature/updating-matter-rvc-ux branch from d1c20d1 to df9744e Compare August 15, 2025 10:31

nickolas-deboom reviewed Aug 18, 2025

View reviewed changes

HunsupJung requested review from tpmanley and nickolas-deboom August 24, 2025 02:56

HunsupJung force-pushed the feature/updating-matter-rvc-ux branch from a5a6c42 to 97099e4 Compare September 2, 2025 09:40

Re-match profile to support Service Area

6cf2dfc

Signed-off-by: HunsupJung <[email protected]>

HunsupJung force-pushed the feature/updating-matter-rvc-ux branch from 97099e4 to 6cf2dfc Compare September 2, 2025 09:46

Merge branch 'main' into feature/updating-matter-rvc-ux

9b377d5

hcarter-775 reviewed Sep 4, 2025

View reviewed changes

nickolas-deboom reviewed Sep 4, 2025

View reviewed changes

drivers/SmartThings/matter-rvc/src/init.lua Outdated Show resolved Hide resolved

Update the PR by applying the reviews

0716447

Signed-off-by: HunsupJung <[email protected]>

hcarter-775 reviewed Sep 5, 2025

View reviewed changes

		@@ -5,6 +5,8 @@ components:
		capabilities:
		- id: robotCleanerOperatingState

	device:set_field(SERVICE_AREA_PROFILED, true)
	device:set_field(SERVICE_AREA_PROFILED, true, { persist = true })

		local RUN_MODE_SUPPORTED_MODES = "__run_mode_supported_modes"
		local CURRENT_RUN_MODE = "__current_run_mode"

Updating matter rvc ux #2343

Are you sure you want to change the base?

Updating matter rvc ux #2343

Uh oh!

Conversation

HunsupJung commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of Change

Checklist

Description of Change

Summary of Completed Tests

Uh oh!

github-actions bot commented Aug 15, 2025

Uh oh!

github-actions bot commented Aug 15, 2025

Uh oh!

github-actions bot commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions bot commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hcarter-775 Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hcarter-775 Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HunsupJung Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hcarter-775 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hcarter-775 Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

HunsupJung commented Aug 15, 2025 •

edited

Loading

github-actions bot commented Aug 15, 2025 •

edited

Loading

github-actions bot commented Aug 15, 2025 •

edited

Loading

hcarter-775 Sep 4, 2025 •

edited

Loading

hcarter-775 Sep 4, 2025 •

edited

Loading

HunsupJung Sep 5, 2025 •

edited

Loading

hcarter-775 Sep 5, 2025 •

edited

Loading