Add multi-button support for matter button devices. #849

greens · 2023-07-13T21:17:31Z

No description provided.

github-actions · 2023-07-13T21:18:02Z

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions · 2023-07-13T21:19:01Z

Channel deleted.

greens · 2023-07-13T21:19:29Z

So this works by either using multi-component for a device for which we have a profile with that number of buttons defined or it uses parent child for cases where we don't.

If people think we should add more than just 2,4,8 let me know.

Also thoughts on distinguishing battery vs. non-battery, or just assuming we have battery in all cases?

github-actions · 2023-07-13T21:19:40Z

Test Results

    52 files ±  0   339 suites +1 0s ⏱️ ±0s
1 608 tests +  6 1 608 ✔️ +  6 0 💤 ±0 0 ❌ ±0
2 787 runs +24 2 787 ✔️ +24 0 💤 ±0 0 ❌ ±0

Results for commit 7960872. ± Comparison against base commit 330ba0a.

This pull request removes 3 and adds 9 tests. Note that renamed tests count towards both.

Handle on command
Handle on position
Receiving a max press attribute of greater than 6 should not emit event

Handle a held event in a multi-press sequence
Handle long press sequence for a long hold on long-release-capable button
Handle long press sequence for a long hold on multi button
Handle single press sequence for a long hold on long-release-capable button
Handle single press sequence for a long hold on multi button
Handle single press sequence for a multi press on multi button
Handle single press sequence for emulated hold on short-release-only button
Handle single press sequence for short release-supported button
Receiving a max press attribute of greater than 6 should only emit up to pushed_6x

♻️ This comment has been updated with latest results.

github-actions · 2023-07-13T21:19:42Z

File	Coverage
All files	`96%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-button/src/init.lua	`96%`	✅

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 7960872

cjswedes

This looks really good! The event handling for presses seems very solid wrt the various cluster feature support.

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

tpmanley · 2023-07-18T16:34:46Z

Also thoughts on distinguishing battery vs. non-battery, or just assuming we have battery in all cases?

I know there will be some button devices that don't have battery so it'd be best to support both

posborne · 2023-08-17T16:44:57Z

I did some testing with the Tuo Smart Button and this driver and the device is far more functional and single press, held seemed to work correctly. Double-press does seem to register but the driver is also emitting single-press events for the multi-press case. Here's logs from a double-press from the driver at 6ff3af1.

2023-08-17T16:38:02.862852537+00:00 TRACE Matter Button  Received event with handler matter
2023-08-17T16:38:02.863991791+00:00 INFO Matter Button  <MatterDevice: eeedda58-b442-4017-a34d-50143daa61ff [64F9F0730C6AEC69-16B2FECCAA5AF5A4] (TUO Smart Button)> received InteractionResponse: <InteractionResponse || type: REPORT_DATA, response_blocks: [<InteractionResponseInfoBlock || status: SUCCESS, <InteractionInfoBlock || endpoint: 0x01, cluster: Switch, attribute: CurrentPosition, data: Uint8: \x01>>, <InteractionResponseInfoBlock || status: SUCCESS, event number: 0x20134, <InteractionInfoBlock || endpoint: 0x01, cluster: Switch, event: InitialPress, data: Structure: {new_position: \x01}>>]>
2023-08-17T16:38:02.866614606+00:00 DEBUG Matter Button  TUO Smart Button device thread event handled
2023-08-17T16:38:02.871790908+00:00 TRACE Matter Button  Found MatterMessageDispatcher handler in matter-button
2023-08-17T16:38:02.875774961+00:00 INFO Matter Button  Executing EventReportHandler: cluster: Switch, event: InitialPress
2023-08-17T16:38:02.881793871+00:00 DEBUG Matter Button  TUO Smart Button device thread event handled
2023-08-17T16:38:03.888726398+00:00 TRACE Matter Button  Received event with handler matter
2023-08-17T16:38:03.895450591+00:00 INFO Matter Button  <MatterDevice: eeedda58-b442-4017-a34d-50143daa61ff [64F9F0730C6AEC69-16B2FECCAA5AF5A4] (TUO Smart Button)> received InteractionResponse: <InteractionResponse || type: REPORT_DATA, response_blocks: [<InteractionResponseInfoBlock || status: SUCCESS, <InteractionInfoBlock || endpoint: 0x01, cluster: Switch, attribute: CurrentPosition, data: Uint8: \x00>>, <InteractionResponseInfoBlock || status: SUCCESS, event number: 0x20135, <InteractionInfoBlock || endpoint: 0x01, cluster: Switch, event: ShortRelease, data: Structure: {previous_position: \x01}>>, <InteractionResponseInfoBlock || status: SUCCESS, event number: 0x20136, <InteractionInfoBlock || endpoint: 0x01, cluster: Switch, event: InitialPress, data: Structure: {new_position: \x01}>>, <InteractionResponseInfoBlock || status: SUCCESS, event number: 0x20138, <InteractionInfoBlock || endpoint: 0x01, cluster: Switch, event: ShortRelease, data: Structure: {previous_position: \x01}>>, <InteractionResponseInfoBlock || status: SUCCESS, event number: 0x20139, <InteractionInfoBlock || endpoint: 0x01, cluster: Switch, event: MultiPressComplete, data: Structure: {total_number_of_presses_counted: \x02, new_position: \x01}>>]>
2023-08-17T16:38:03.902309443+00:00 DEBUG Matter Button  TUO Smart Button device thread event handled
2023-08-17T16:38:03.957850204+00:00 TRACE Matter Button  Found MatterMessageDispatcher handler in matter-button
2023-08-17T16:38:03.958906797+00:00 INFO Matter Button  Executing EventReportHandler: cluster: Switch, event: ShortRelease
2023-08-17T16:38:03.972340186+00:00 INFO Matter Button  <MatterDevice: eeedda58-b442-4017-a34d-50143daa61ff [64F9F0730C6AEC69-16B2FECCAA5AF5A4] (TUO Smart Button)> emitting event: {"state_change":true,"attribute_id":"button","capability_id":"button","component_id":"main","state":{"value":"pushed"}}
2023-08-17T16:38:03.993540362+00:00 DEBUG Matter Button  TUO Smart Button device thread event handled
2023-08-17T16:38:03.994613620+00:00 TRACE Matter Button  Found MatterMessageDispatcher handler in matter-button
2023-08-17T16:38:03.996307500+00:00 INFO Matter Button  Executing EventReportHandler: cluster: Switch, event: InitialPress
2023-08-17T16:38:03.997971383+00:00 DEBUG Matter Button  TUO Smart Button device thread event handled
2023-08-17T16:38:04.008164+00:00 TRACE Matter Button  Found MatterMessageDispatcher handler in matter-button
2023-08-17T16:38:04.009217593+00:00 INFO Matter Button  Executing EventReportHandler: cluster: Switch, event: ShortRelease
2023-08-17T16:38:04.022343671+00:00 INFO Matter Button  <MatterDevice: eeedda58-b442-4017-a34d-50143daa61ff [64F9F0730C6AEC69-16B2FECCAA5AF5A4] (TUO Smart Button)> emitting event: {"state_change":true,"attribute_id":"button","capability_id":"button","component_id":"main","state":{"value":"pushed"}}
2023-08-17T16:38:04.050057725+00:00 DEBUG Matter Button  TUO Smart Button device thread event handled
2023-08-17T16:38:04.082857422+00:00 TRACE Matter Button  Found MatterMessageDispatcher handler in matter-button
2023-08-17T16:38:04.083906015+00:00 INFO Matter Button  Executing EventReportHandler: cluster: Switch, event: MultiPressComplete
2023-08-17T16:38:04.118725236+00:00 INFO Matter Button  <MatterDevice: eeedda58-b442-4017-a34d-50143daa61ff [64F9F0730C6AEC69-16B2FECCAA5AF5A4] (TUO Smart Button)> emitting event: {"state_change":true,"attribute_id":"button","capability_id":"button","component_id":"main","state":{"value":"double"}}
2023-08-17T16:38:04.136373330+00:00 DEBUG Matter Button  TUO Smart Button device thread event handled

greens · 2023-08-17T18:37:18Z

@posborne was that from a new install or a driver switch?

posborne · 2023-08-17T22:42:25Z

@posborne was that from a new install or a driver switch?

I driver switched to my self-published off this branch but I can retest with a fresh join.

greens · 2023-08-17T23:05:48Z

@posborne I expect better results off of a fresh join. A lot of the interrogation work and setting of important state happens in added, which doesn't fire on driver switches I don't think?

posborne

I strongly suspect we'll be revisiting this code a few times in the near future but this is significantly improved over the current base in tree and changes overall LGTM.

re-order type handling so that multi-press buttons have priority keep newly-created components and devices around in a map to avoid race conditions updating device storage avoid race condition when switching profile change multi buttons to support held events

tpmanley · 2023-09-06T03:43:07Z

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

+        set_field_for_endpoint(device, MULTI_BUTTON, ep, true, true)
+        supportedButtonValues_event = nil -- deferred until max press handler
+      elseif contains(MSL, ep) then
+        device.log.debug("configuring for long press device")


It looks like this case is missing some logic

I said that because my 4-button device supports press/hold and isn't working, but on further review it's not clear to me if the issue is here or somewhere else

Does it also support Multipress or just Press & Hold?
Did you try to re-add the device?

It does not support multipress. I removed/re-added a few times and the behavior was same each time.

tpmanley

I think there is some more testing we need to do with multi-button devices, but I think this is a good improvement over what we have now and would suggest we merge this sooner than later to get these fixes out to everyone.

greens requested review from tpmanley, dljsjr, cjswedes and ctowns July 13, 2023 21:17

cjswedes reviewed Jul 18, 2023

View reviewed changes

posborne approved these changes Sep 5, 2023

View reviewed changes

greens force-pushed the feature/matter-multi-button branch from 1c98aae to 7960872 Compare September 5, 2023 21:53

tpmanley reviewed Sep 6, 2023

View reviewed changes

tpmanley approved these changes Sep 12, 2023

View reviewed changes

greens merged commit d92b792 into main Sep 12, 2023
11 checks passed

greens deleted the feature/matter-multi-button branch September 12, 2023 19:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi-button support for matter button devices. #849

Add multi-button support for matter button devices. #849

greens commented Jul 13, 2023

github-actions bot commented Jul 13, 2023 •

edited

Loading

github-actions bot commented Jul 13, 2023 •

edited

Loading

greens commented Jul 13, 2023

github-actions bot commented Jul 13, 2023 •

edited

Loading

github-actions bot commented Jul 13, 2023 •

edited

Loading

cjswedes left a comment

tpmanley commented Jul 18, 2023

posborne commented Aug 17, 2023

greens commented Aug 17, 2023

posborne commented Aug 17, 2023

greens commented Aug 17, 2023

posborne left a comment

tpmanley Sep 6, 2023

tpmanley Sep 6, 2023

ldoering Sep 6, 2023

tpmanley Sep 6, 2023

tpmanley left a comment

Add multi-button support for matter button devices. #849

Add multi-button support for matter button devices. #849

Conversation

greens commented Jul 13, 2023

github-actions bot commented Jul 13, 2023 • edited Loading

github-actions bot commented Jul 13, 2023 • edited Loading

greens commented Jul 13, 2023

github-actions bot commented Jul 13, 2023 • edited Loading

Test Results

github-actions bot commented Jul 13, 2023 • edited Loading

cjswedes left a comment

Choose a reason for hiding this comment

tpmanley commented Jul 18, 2023

posborne commented Aug 17, 2023

greens commented Aug 17, 2023

posborne commented Aug 17, 2023

greens commented Aug 17, 2023

posborne left a comment

Choose a reason for hiding this comment

tpmanley Sep 6, 2023

Choose a reason for hiding this comment

tpmanley Sep 6, 2023

Choose a reason for hiding this comment

ldoering Sep 6, 2023

Choose a reason for hiding this comment

tpmanley Sep 6, 2023

Choose a reason for hiding this comment

tpmanley left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 13, 2023 •

edited

Loading

github-actions bot commented Jul 13, 2023 •

edited

Loading

github-actions bot commented Jul 13, 2023 •

edited

Loading

github-actions bot commented Jul 13, 2023 •

edited

Loading