add firstled-io_M4S4BAC by thinkaName · Pull Request #2971 · SmartThingsCommunity/SmartThingsEdgeDrivers

thinkaName · 2026-05-14T10:24:23Z

Check all that apply

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

Summary of Completed Tests

github-actions · 2026-05-14T10:24:50Z

Duplicate profile check: Passed - no duplicate profiles detected.

github-actions · 2026-05-14T10:25:35Z

Invitation URL:
https://bestow-regional.api.smartthings.com/invite/5z2R5p3OzzM1

github-actions · 2026-05-14T10:29:36Z

Test Results

73 files 516 suites 0s ⏱️
2 947 tests 2 933 ✅ 0 💤 14 ❌
4 843 runs 4 829 ✅ 0 💤 14 ❌

For more details on these failures, see this check.

Results for commit 6be3b1e.

♻️ This comment has been updated with latest results.

github-actions · 2026-05-14T10:29:38Z

File	Coverage
All files	`96%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/aqara/init.lua	`94%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/firstled-io/init.lua	`91%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/frient/init.lua	`94%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/color_temp_range_handlers/init.lua	`71%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/tuya-multi/can_handle.lua	`90%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/zll-polling/init.lua	`81%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/inovelli/vzm30-sn/init.lua	`95%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/inovelli/init.lua	`98%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/frient-IO/init.lua	`88%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/frient-IO/unbind_request.lua	`71%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/laisiao/init.lua	`96%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/configurations/init.lua	`98%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/preferences.lua	`97%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/lazy_load_subdriver.lua	`57%`	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/init.lua	`92%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/aqara/version/init.lua	`94%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/aqara-light/init.lua	`92%`	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/zigbee-switch/src/inovelli/vzm32-sn/init.lua	`95%`	✅

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 6be3b1e

cjswedes

Test failures will need to be resolved prior to merging.

cjswedes · 2026-05-18T21:49:49Z

+      { visibility = { displayed = false } }))
+
+    device:send(cluster_base.write_manufacturer_specific_attribute(device,
+      PRIVATE_CLUSTER_ID, PRIVATE_ATTRIBUTE_ID, MFG_CODE, data_types.Uint8, 0x01)) -- private


I dont really know what this attribute write does, but is it something that should be done in device init? The difference being it would be written anytime the hub restarts or the driver is updated

Looks like this was moved to infoChanged to handle preference changes. It seems like it might still be needed in the added handler so that the initial preference default value is written.

When the device is entering the pairing process, it will restore the data to the default values, and both ends will have the same default values.

nand-nor · 2026-05-20T13:19:43Z

Can code coverage be improved in firstled-io/init.lua? Device init tests, preference changes, scenes cluster handling, profile switching, that kind of thing

thinkaName · 2026-05-21T09:59:48Z

Can code coverage be improved in firstled-io/init.lua? Device init tests, preference changes, scenes cluster handling, profile switching, that kind of thing

The maximum improvement can only reach 88%. What suggestions do you have?

cjswedes · 2026-05-21T15:40:42Z

The maximum improvement can only reach 88%. What suggestions do you have?

You can run coverage locally and get the report yourself. When I do it I see that none of the child button behavior is tested.

To run it locally first run your test file with coveraged enabled (note you need luacov installed)

## run test with coverage enabled from zigbee-switch/src
lua -lluacov test/test_firstled_switch.lua
## create the report from the stats output
luacov -c ./luacov.stats.out --config ~/SmartThingsEdgeDrivers/tools/config.luacov

This will create a luacov.report.out with counts next to each line in each file that was required during the lua script. Here is what I was seeing relating to the firstled-io/init.lua file:

       local function get_child_profile_name(device)
*****0   for _, fingerprint in ipairs(FINGERPRINTS) do
*****0     if device:get_model() == fingerprint.model then
*****0       return fingerprint.child_profile
           end
         end
       end

       local function find_child(parent, ep_id)
    23   return parent:get_child_by_parent_assigned_key(string.format("%02X", ep_id))
       end

       local function device_added(driver, device)
         -- Only create children for the actual Zigbee device and not the children
     8   if device.network_type == device_lib.NETWORK_TYPE_ZIGBEE then
     1     local children_amount = get_children_amount(device)
     1     if children_amount >= 2 then
     4       for i = 2, children_amount, 1 do
     3         if find_child(device, i) == nil then
*****0           local name = string.format("%s%d", string.sub(device.label, 0, -2), i)
*****0           local child_profile = get_child_profile_name(device)
*****0           local metadata = {
                   type = "EDGE_CHILD",
                   label = name,
                   profile = child_profile,
                   parent_device_id = device.id,
                   parent_assigned_child_key = string.format("%02X", i),
*****0             vendor_provided_label = name
                 }
*****0           driver:try_create_device(metadata)
               end
             end
           end

         -- Create Button if necessary
     1     local button_amount = get_button_amount(device)
     1     if button_amount >= 1 then
     5       for i = children_amount + 1, children_amount + button_amount, 1 do
     4         if find_child(device, i) == nil then
*****0           local name = string.format("%s%d", string.sub(device.label, 0, -2), i)
*****0           local metadata = {
                   type = "EDGE_CHILD",
                   label = name,
                   profile = "button",
                   parent_device_id = device.id,
                   parent_assigned_child_key = string.format("%02X", i),
                   vendor_provided_label = name,
                 }
*****0           driver:try_create_device(metadata)
               end
             end
           end

cbaumler · 2026-06-05T15:28:53Z

Hi @thinkaName , it looks like this PR branch is out of date with main. Please update it with the latest main and push the result. Then we can re-review.

cjswedes

It seems like there was a lot of stuff added since the last reviews wrt child devices, but we cant tell because you are obfuscating the commit history by always force pushing; this makes it hard to review this PR and others you have submitted. Please keep incremental changes in their own commits until we have approved the PR.

cjswedes · 2026-06-09T17:57:11Z

+-- Licensed under the Apache License, Version 2.0
+
+return {
+  { mfr = "FIRSTLED", model = "M4S4BAC",  buttons = 4 ,  children = 4, child_profile = "switch-button-wireless" }


How are the number of buttons and children used? Please describe the way the device is intended to be integrated, since it is not clear from the code in its current state. My assumption is that there is a single zigbee device with 4 buttons, and you are trying to have a child device for each button.

What is the goal of using child devices to represent individual buttons? The approach we take on the platform is to use a multi component profile that has a component for each button.

Since this is the first product submitted, a series of devices will be added to reuse this driver. Currently, this is the 4+4 model, consisting of 4 switches that can be set to button mode, plus 4 pure buttons. The number of child devices to be created is determined by "fingerprints.children", and the number of buttons to be created is determined by "fingerprints.buttons". Subsequently, models such as 3+3, 2+2, and 1+1 will be added. Additionally, there will be models without the button, and the setting for fingerprints will be fingerprints.buttons=0.

Thank you, this clears up a lot of my confusion with what is going on with the child devices. You may want to add a comment at the top of this subdriver explaining how this device works.

thinkaName · 2026-06-12T02:47:15Z

It seems like there was a lot of stuff added since the last reviews wrt child devices, but we cant tell because you are obfuscating the commit history by always force pushing; this makes it hard to review this PR and others you have submitted. Please keep incremental changes in their own commits until we have approved the PR.

Because it was previously informed that a device's PR could only have one commit, so the forced override push was carried out.

cjswedes · 2026-06-12T20:20:42Z

It seems like there was a lot of stuff added since the last reviews wrt child devices, but we cant tell because you are obfuscating the commit history by always force pushing; this makes it hard to review this PR and others you have submitted. Please keep incremental changes in their own commits until we have approved the PR.

Because it was previously informed that a device's PR could only have one commit, so the forced override push was carried out.

I apologize for the confusion. We do want there to only be a single commit when we merge, but it is useful to see how feedback is addressed by keeping commits separate during the review period.

cjswedes

The removal of the button capability based on the preference makes sense, but I think its a good use case for modular profiles.

cjswedes · 2026-06-15T15:57:48Z

+    end
+  else
+    if show then
+      device:try_update_metadata({profile = "switch-button-wireless"})


Rather than have 4 profiles to account for the button capability not being used we have a feature called modular profiles which allows for a profile to specify a capability as optional, and then you can update which optional capabilities are included using try update metadata.

Suggested change

device:try_update_metadata({profile = "switch-button-wireless"})

device:try_update_metadata({profile = "switch-button-wireless", optional_component_capabilities = {{"main", {"button"}}})

cjswedes · 2026-06-15T15:58:19Z

+        device:emit_event(capabilities.button.supportedButtonValues({"pushed"}, {visibility = {displayed = false}}))
+      end)
+    else
+      device:try_update_metadata({profile = "switch-wireless"})


Suggested change

device:try_update_metadata({profile = "switch-wireless"})

device:try_update_metadata({profile = "switch-wireless", optional_component_capabilities = {{"main", {}}})

@ctowns or @hcarter-775 this will work to remove all optional capabilities, correct?

I got confirmation that this does work.

The actual testing shows that this method has a very low success rate. There are always some issues that cannot be resolved, and the UI fails to switch successfully.

Thank you for trying this approach, since you are getting good results with switching profiles, it makes sense to me to go with that. With the listen_profile_button_transition in place, this functionality is looking pretty solid.

cjswedes · 2026-06-15T16:02:14Z

+  else
+    if show then
+      device:try_update_metadata({profile = "switch-button-wireless"})
+      device.thread:call_with_delay(5, function()


This is okay, but it will fail if there any situations of higher latency which do sometimes occur. Once the profile/optional capabilities are updated, another infoChanged event will be triggered based on the changed capabilities, so the better place to emit these button events would be from the infoChanged handler based on the capabilities of the device.

cjswedes · 2026-06-15T16:07:35Z

+  - id: switch
+    version: 1
+  - id: button
+    version: 1


Suggested change

version: 1

version: 1

optional: true

cbaumler · 2026-06-16T14:54:07Z

Hi @thinkaName, it looks like this PR branch is out of date with main. Please update it with the latest main and push the result. Then let us know when you're ready for us to re-review.

cjswedes · 2026-06-16T16:57:41Z

+  }
+)
+
+local function test_clild_changeToWirelessSwitch_true(ep, name)


Suggested change

local function test_clild_changeToWirelessSwitch_true(ep, name)

local function test_child_changeToWirelessSwitch_true(ep, name)

thinkaName force-pushed the firstled-io_M4S4BAC branch 2 times, most recently from d0d3e68 to 09fa419 Compare May 18, 2026 05:48

cbaumler requested review from aleclorimer, cjswedes and nand-nor May 18, 2026 20:59

cjswedes reviewed May 18, 2026

View reviewed changes

thinkaName force-pushed the firstled-io_M4S4BAC branch 2 times, most recently from 2b92e15 to 29ea878 Compare May 19, 2026 08:58

nand-nor reviewed May 20, 2026

View reviewed changes

Comment thread drivers/SmartThings/zigbee-switch/src/firstled-io/init.lua Outdated

thinkaName force-pushed the firstled-io_M4S4BAC branch 5 times, most recently from 541f830 to 59e191b Compare May 21, 2026 09:56

cjswedes reviewed May 21, 2026

View reviewed changes

Comment thread drivers/SmartThings/zigbee-switch/src/test/test_wallhero_switch.lua

thinkaName force-pushed the firstled-io_M4S4BAC branch 5 times, most recently from 927cc97 to 90c7984 Compare May 22, 2026 10:01

cjswedes reviewed May 22, 2026

View reviewed changes

Comment thread drivers/SmartThings/zigbee-switch/src/firstled-io/init.lua

thinkaName force-pushed the firstled-io_M4S4BAC branch 2 times, most recently from d5ff53c to aca4e88 Compare May 23, 2026 08:09

thinkaName force-pushed the firstled-io_M4S4BAC branch from aca4e88 to 364204d Compare June 8, 2026 01:26

cjswedes reviewed Jun 9, 2026

View reviewed changes

cjswedes reviewed Jun 12, 2026

View reviewed changes

Comment thread drivers/SmartThings/zigbee-switch/src/firstled-io/init.lua

Comment thread drivers/SmartThings/zigbee-switch/profiles/switch-button-wireless.yml

add firstled-io_M4S4BAC

e0a5cfa

thinkaName force-pushed the firstled-io_M4S4BAC branch from 718fdbe to e0a5cfa Compare June 13, 2026 11:31

cjswedes reviewed Jun 15, 2026

View reviewed changes

When switching modes and listening for changes in the profile

6be3b1e

cjswedes approved these changes Jun 16, 2026

View reviewed changes

	device:try_update_metadata({profile = "switch-button-wireless"})
	device:try_update_metadata({profile = "switch-button-wireless", optional_component_capabilities = {{"main", {"button"}}})

	local function test_clild_changeToWirelessSwitch_true(ep, name)
	local function test_child_changeToWirelessSwitch_true(ep, name)

Conversation

thinkaName commented May 14, 2026 • 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 May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 14, 2026

Uh oh!

github-actions Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjswedes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

nand-nor commented May 20, 2026

Uh oh!

Uh oh!

thinkaName commented May 21, 2026

Uh oh!

Uh oh!

cjswedes commented May 21, 2026

Uh oh!

Uh oh!

cbaumler commented Jun 5, 2026

Uh oh!

cjswedes 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!

Uh oh!

Uh oh!

Uh oh!

thinkaName commented Jun 12, 2026

Uh oh!

cjswedes commented Jun 12, 2026

Uh oh!

Uh oh!

Uh oh!

cjswedes 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!

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!

thinkaName commented May 14, 2026 •

edited

Loading

github-actions Bot commented May 14, 2026 •

edited

Loading

github-actions Bot commented May 14, 2026 •

edited

Loading

github-actions Bot commented May 14, 2026 •

edited

Loading