Request to add a Zigbee button for SLED switch #1021

seungkwan-choi · 2023-10-19T01:19:08Z

This is a new project for Samsung LED.
It's a ZigBee Button and the model no. is SAMSUNG-ITM-Z-005.
Please review it.

Add SAMSUNG-ITM-Z-005

SLED profiles for SAMSUNG-ITM-Z-005

Add SLED for SAMSUNG-ITM-Z-005

add SLED for SAMSUNG-ITM-Z-005

CLAassistant · 2023-10-19T01:19:17Z

All committers have signed the CLA.

github-actions · 2023-10-20T15:44:33Z

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions · 2023-10-20T15:45:46Z

Channel deleted.

github-actions · 2023-10-20T15:46:26Z

Test Results

    54 files   346 suites 0s ⏱️
1 621 tests 1 621 ✔️ 0 💤 0 ❌
2 848 runs 2 848 ✔️ 0 💤 0 ❌

Results for commit 8c6fc70.

♻️ This comment has been updated with latest results.

github-actions · 2023-10-20T15:46:28Z

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 8c6fc70

greens · 2023-10-20T18:34:14Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua

@@ -0,0 +1,93 @@
+-- Copyright 2022 SmartThings


I will modify it.

greens · 2023-10-20T18:35:03Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua

+local fields = {
+  IGNORE_MOVETOCOLORTEMP = "ignore_next_movetocolortemperature"
+}


I will delete it.

greens · 2023-10-20T18:36:43Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua

+  local comp = device.profile.components[button_name]
+  if comp ~= nil then
+    device:emit_component_event(comp, event)
+    device:emit_event(event)


the main component does not have the button capability so this will fail, same below

Do you mean to delete this code?
Or do you want me to add button from main in profile?
-id: main
capabilities:
- id: button
version: 1

Up to you how to implement it. Either of those solutions would work.

In our drivers, we have had the main component of a multi-button device emit an event when any of the buttons are pressed, but some users have found this confusing.

We will also be confused if we have a main component.
Then please guide me on how to modify the code without main component.
If not, I will add main component.

just remove the device:emit_event(event) line

That attempts to emit the event from the main component, which does not have the capability used by the event, and so it would log a warning.

I will check it after modifying it. Thanks.

I modified the code, but the held button is not visible, so I am rechecking it.

greens

please includes some tests

I'd at least like a test of the "added" and some of the pressed and pushed events just to verify that you're setting supportedButtonValues correctly.

lelandblue · 2023-10-24T17:33:26Z

Hello @seungkwan-choi Please see our review, we look forward to your additional changes and thank you.

greens · 2023-10-25T16:10:44Z

Checking drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua 3 warnings

    drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua:23:7: (W211) unused variable fields
    drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua:57:3: (W113) accessing undefined variable do_refresh
    drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua:66:54: (W113) accessing undefined variable do_refresh

add SLED folder

modify code

greens · 2023-10-30T18:24:47Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/supported_values.lua

+      { mfr = "Samsung Electronics", model = "SAMSUNG-ITM-Z-005" },
    },
    SUPPORTED_BUTTON_VALUES = { "pushed" },


This is why you're not seeing held events. A supportedButtonValues event needs to be generated for each button component that has the button capability that lists the supported values. Because you've added your device to this list here, the zigbee-multi-button driver's init method only emits a supportedButtonValues event with "pushed" instead of "pushed", "held"

The code has been modified. Please review.

greens · 2023-10-30T18:28:31Z

Please also address the luacheck errors I pointed out.

add push_held_3

greens · 2023-10-31T17:19:21Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua

+end
+
+local do_configure = function(self, device)
+  do_refresh(self, device)


as pointed out by the luacheck automation, do_refresh is not defined

lua code has been changed.

greens

I'd like to see some tests of this code, please.

Those tests would have made it even more obvious that you were referencing an undefined value.

Delete do_refresh

greens · 2023-11-01T21:10:12Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua

+  NAME = "SLED Button",
+  capability_handlers = {
+    [capabilities.refresh.ID] = {
+      [capabilities.refresh.commands.refresh.NAME] = do_refresh,


you are also referencing do_refresh here

Checking drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua 3 warnings drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua:30:45: (W612) line contains trailing whitespace drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua:44:45: (W612) line contains trailing whitespace drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua:59:54: (W113) accessing undefined variable do_refresh

Please address these comments. Please write some tests for this behavior.

The code was modified and there were no errors through lua check.

modify lua file(delete do_refresh)

Change whitespace

lelandblue · 2023-11-02T01:38:24Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/supported_values.lua

@@ -25,6 +25,7 @@ local devices = {
      { mfr = "CentraLite", model = "3450-L" },
      { mfr = "CentraLite", model = "3450-L2" },
      { mfr = "ROBB smarrt", model = "ROB_200-008-0" }
+


Can we please remove this extra line that is empty?

Remove the extra line.

lelandblue · 2023-11-02T01:41:52Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/SLED/init.lua

+  end
+end
+
+


I believe this is extra whitespace as well that needs to be removed, please review.

Remove the extra line

Remove extra line

remove extra line

greens · 2023-11-06T20:23:44Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/supported_values.lua

@@ -24,7 +24,7 @@ local devices = {
    MATCHING_MATRIX = {
      { mfr = "CentraLite", model = "3450-L" },
      { mfr = "CentraLite", model = "3450-L2" },
-      { mfr = "ROBB smarrt", model = "ROB_200-008-0" }
+      { mfr = "ROBB smarrt", model = "ROB_200-008-0" }    


Suggested change

{ mfr = "ROBB smarrt", model = "ROB_200-008-0" }

{ mfr = "ROBB smarrt", model = "ROB_200-008-0" }

if you delete and change the whitespace in the code, it will not be saved and it will be shown to request a creat pull request patch.
Please check it.

I'm not sure what you mean.

Remove the whitespace.

greens · 2023-11-06T20:24:04Z

drivers/SmartThings/zigbee-button/src/zigbee-multi-button/init.lua

@@ -39,7 +39,8 @@ local ZIGBEE_MULTI_BUTTON_FINGERPRINTS = {
  { mfr = "ShinaSystem", model = "SBM300ZB3" },
  { mfr = "ROBB smarrt", model = "ROB_200-007-0" },
  { mfr = "ROBB smarrt", model = "ROB_200-008-0" },
-  { mfr = "WALL HERO", model = "ACL-401SCA4" }
+  { mfr = "WALL HERO", model = "ACL-401SCA4" },
+  { mfr = "Samsung Electronics", model = "SAMSUNG-ITM-Z-005" },


Suggested change

{ mfr = "Samsung Electronics", model = "SAMSUNG-ITM-Z-005" },

{ mfr = "Samsung Electronics", model = "SAMSUNG-ITM-Z-005" }

if you delete and change the comma in the code, it will not be saved and it will be shown to request a creat pull request patch.
Please check it.

Remove the comma.

delete whitespace

delete comma

seungkwan-choi added 5 commits October 19, 2023 09:54

SLED fingerprints.yml

3c764ee

Add SAMSUNG-ITM-Z-005

Create SLED-three-buttons.yml

24991db

SLED profiles for SAMSUNG-ITM-Z-005

Create init.lua

796cb08

Add SLED for SAMSUNG-ITM-Z-005

Update init.lua

25babb2

add SLED for SAMSUNG-ITM-Z-005

Update supported_values.lua

c061c35

add SLED for SAMSUNG-ITM-Z-005

lelandblue added do not merge needs invite link Needs the bestow invite like WWST labels Oct 20, 2023

lelandblue requested review from greens, cbaumler and wkhenon October 20, 2023 15:45

greens reviewed Oct 20, 2023

View reviewed changes

greens requested changes Oct 20, 2023

View reviewed changes

Merge branch 'main' into main

8de812e

seungkwan-choi added 2 commits October 30, 2023 13:22

Update init.lua

ac22da1

add SLED folder

Update init.lua

53e712e

modify code

lelandblue removed the needs invite link Needs the bestow invite like label Oct 30, 2023

greens reviewed Oct 30, 2023

View reviewed changes

Update supported_values.lua

147a81d

add push_held_3

greens reviewed Oct 31, 2023

View reviewed changes

Update init.lua

52d8acc

Delete do_refresh

greens reviewed Nov 1, 2023

View reviewed changes

lelandblue and others added 3 commits November 1, 2023 21:17

Merge branch 'main' into main

004e813

Update init.lua

e01baca

modify lua file(delete do_refresh)

Update init.lua

b0efd4a

Change whitespace

lelandblue reviewed Nov 2, 2023

View reviewed changes

seungkwan-choi and others added 3 commits November 2, 2023 13:08

Update supported_values.lua

3008cf0

Remove extra line

Update init.lua

71953d9

remove extra line

Merge branch 'main' into main

fe0a38a

greens reviewed Nov 6, 2023

View reviewed changes

seungkwan-choi added 2 commits November 8, 2023 09:29

Update supported_values.lua

e3c4c17

delete whitespace

Update init.lua

8c6fc70

delete comma

greens approved these changes Nov 8, 2023

View reviewed changes

lelandblue removed the do not merge label Dec 1, 2023

lelandblue merged commit 75ac77e into SmartThingsCommunity:main Dec 1, 2023
14 checks passed

	{ mfr = "ROBB smarrt", model = "ROB_200-008-0" }
	{ mfr = "ROBB smarrt", model = "ROB_200-008-0" }

	{ mfr = "Samsung Electronics", model = "SAMSUNG-ITM-Z-005" },
	{ mfr = "Samsung Electronics", model = "SAMSUNG-ITM-Z-005" }

Request to add a Zigbee button for SLED switch #1021

Request to add a Zigbee button for SLED switch #1021

Conversation

seungkwan-choi commented Oct 19, 2023

CLAassistant commented Oct 19, 2023 • edited Loading

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023 • edited Loading

github-actions bot commented Oct 20, 2023 • edited Loading

Test Results

github-actions bot commented Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greens left a comment

Choose a reason for hiding this comment

lelandblue commented Oct 24, 2023

greens commented Oct 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greens commented Oct 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Oct 19, 2023 •

edited

Loading

github-actions bot commented Oct 20, 2023 •

edited

Loading

github-actions bot commented Oct 20, 2023 •

edited

Loading

github-actions bot commented Oct 20, 2023 •

edited

Loading