Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add device configuration for supporting Routine #1458

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

HunsupJung
Copy link
Collaborator

In the case of mode capability, presentation is implemented in the list display type.
Therefore, device configuration was added to support automation in the device type that supports the mode capability.

Copy link

Duplicate profile check: Passed - no duplicate profiles detected.

Copy link

github-actions bot commented Jun 19, 2024

Channel deleted.

Copy link

github-actions bot commented Jun 19, 2024

Test Results

   60 files  ±0    375 suites  ±0   0s ⏱️ ±0s
1 820 tests ±0  1 820 ✅ ±0  0 💤 ±0  0 ❌ ±0 
3 168 runs  ±0  3 168 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit c98cd6f. ± Comparison against base commit c18a6de.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 19, 2024

File Coverage
All files 24%
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-rvc/src/init.lua 24%

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against c98cd6f

@ctowns
Copy link
Contributor

ctowns commented Jul 1, 2024

Is metadata the only way we can support supportedArguments for now? Longterm, could we modify the plugin to utilize the supportedArguments value for R3 so we do not need custom metadata? I think this is okay but in general I think we try to avoid using metadata with custom presentations.

@HunsupJung
Copy link
Collaborator Author

Yes, the metadata is the only way to support supportedArguments for now. I will ask Plugin team to apply supportedArguments to mode capability. (Actually, It also needs to apply dynamic list to mode capability). But I'm not sure how long does it take that Plugin team applies my request.

I think this is okay but in general I think we try to avoid using metadata with custom presentations.

I agree with you

@ctowns ctowns requested a review from hcarter-775 July 15, 2024 20:32
@@ -162,6 +162,8 @@ local function rvc_run_mode_supported_mode_attr_handler(driver, device, ib, resp
)

local component = device.profile.components["runMode"]
local labels = get_field_labels_of_supported_modes(device, RVC_RUN_MODE_SUPPORTED_MODES)
device:emit_component_event(component, capabilities.mode.supportedArguments(labels))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between supportedArguments and supportedModes?

Also, should this be labels_of_supported_modes instead of just labels?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose is to use SupportedArguments for the detail view of presentation and use SupportedModes for the Automation. the SupportedArguments are used to dynamically change the mode value that can be selected according to the operation status of the device. And SupportedModes is a full mode list that the device fixedly supports.

Copy link
Contributor

@ctowns ctowns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HunsupJung can you help me understand what effect this change will have and how I can test it? If I use the Matter VDA to test, what difference should I expect to see with these changes?

Also, do you need to add supportedArguments for the appliance drivers also?

@HunsupJung
Copy link
Collaborator Author

@ctowns
The list stored in supportedArguments must be available for selection in the Detail view and the list stored in supportedModes must be available for selection in Routine (Automation).

Additionally, The presentation team is preparing patches that the setting that only can be set in device configuration can be set in embedded configuration. so This patch will be used temporarily and replaced with it when new patch are added.

Signed-off-by: Hunsup Jung <hunsup.jung@samsung.com>
@HunsupJung HunsupJung force-pushed the feature/matter-device-configure branch from 254ae73 to c98cd6f Compare July 30, 2024 06:26
@HunsupJung HunsupJung merged commit 4e5aef5 into main Aug 5, 2024
13 checks passed
@HunsupJung HunsupJung deleted the feature/matter-device-configure branch August 5, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants