Add frient io module driver #2600
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 3f16e4d |
Test Results 71 files 481 suites 0s ⏱️ Results for commit 3f16e4d. ♻️ This comment has been updated with latest results. |
greens
reviewed
Jan 12, 2026
Comment on lines
130
to
133
| if value == nil then return nil end | ||
| if type(value) == "number" then return math.tointeger(value) end | ||
| local num = tonumber(value) | ||
| return num and math.tointeger(num) or nil |
Contributor
There was a problem hiding this comment.
math.tointeger accepts non-numeric values, passes through nil, and returns nil if the value cannot be parsed.
This function seems redundant.
greens
reviewed
Jan 12, 2026
Comment on lines
136
to
144
| local function sanitize_timing(value) | ||
| local int = to_integer(value) or 0 | ||
| if int < 0 then | ||
| int = 0 | ||
| elseif int > 0xFFFF then | ||
| int = 0xFFFF | ||
| end | ||
| return int | ||
| end |
Contributor
There was a problem hiding this comment.
we provide a utils.clamp_value function that you could use here
greens
reviewed
Jan 12, 2026
greens
reviewed
Jan 12, 2026
greens
reviewed
Jan 12, 2026
Comment on lines
404
to
471
| else | ||
| -- Input 1 | ||
| if args.old_st_store.preferences.reversePolarity1 ~= device.preferences.reversePolarity1 then | ||
| write_basic_input_polarity_attr(device, ZIGBEE_ENDPOINTS.INPUT_1, device.preferences.reversePolarity1) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput11 ~= device.preferences.controlOutput11 then | ||
| device:send(device.preferences.controlOutput11 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_1, ZIGBEE_ENDPOINTS.OUTPUT_1) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_1, ZIGBEE_ENDPOINTS.OUTPUT_1)) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput21 ~= device.preferences.controlOutput21 then | ||
| device:send(device.preferences.controlOutput21 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_1, ZIGBEE_ENDPOINTS.OUTPUT_2) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_1, ZIGBEE_ENDPOINTS.OUTPUT_2)) | ||
| end | ||
|
|
||
| -- Input 2 | ||
| if args.old_st_store.preferences.reversePolarity2 ~= device.preferences.reversePolarity2 then | ||
| write_basic_input_polarity_attr(device, ZIGBEE_ENDPOINTS.INPUT_2, device.preferences.reversePolarity2) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput12 ~= device.preferences.controlOutput12 then | ||
| device:send(device.preferences.controlOutput12 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_2, ZIGBEE_ENDPOINTS.OUTPUT_1) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_2, ZIGBEE_ENDPOINTS.OUTPUT_1)) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput22 ~= device.preferences.controlOutput22 then | ||
| device:send(device.preferences.controlOutput22 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_2, ZIGBEE_ENDPOINTS.OUTPUT_2) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_2, ZIGBEE_ENDPOINTS.OUTPUT_2)) | ||
| end | ||
|
|
||
| -- Input 3 | ||
| if args.old_st_store.preferences.reversePolarity3 ~= device.preferences.reversePolarity3 then | ||
| write_basic_input_polarity_attr(device, ZIGBEE_ENDPOINTS.INPUT_3, device.preferences.reversePolarity3) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput13 ~= device.preferences.controlOutput13 then | ||
| device:send(device.preferences.controlOutput13 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_3, ZIGBEE_ENDPOINTS.OUTPUT_1) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_3, ZIGBEE_ENDPOINTS.OUTPUT_1)) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput23 ~= device.preferences.controlOutput23 then | ||
| device:send(device.preferences.controlOutput23 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_3, ZIGBEE_ENDPOINTS.OUTPUT_2) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_3, ZIGBEE_ENDPOINTS.OUTPUT_2)) | ||
| end | ||
|
|
||
| -- Input 4 | ||
| if args.old_st_store.preferences.reversePolarity4 ~= device.preferences.reversePolarity4 then | ||
| write_basic_input_polarity_attr(device, ZIGBEE_ENDPOINTS.INPUT_4, device.preferences.reversePolarity4) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput14 ~= device.preferences.controlOutput14 then | ||
| device:send(device.preferences.controlOutput14 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_4, ZIGBEE_ENDPOINTS.OUTPUT_1) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_4, ZIGBEE_ENDPOINTS.OUTPUT_1)) | ||
| end | ||
|
|
||
| if args.old_st_store.preferences.controlOutput24 ~= device.preferences.controlOutput24 then | ||
| device:send(device.preferences.controlOutput24 | ||
| and build_bind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_4, ZIGBEE_ENDPOINTS.OUTPUT_2) | ||
| or build_unbind_request(device, BasicInput.ID, ZIGBEE_ENDPOINTS.INPUT_4, ZIGBEE_ENDPOINTS.OUTPUT_2)) | ||
| end |
Contributor
There was a problem hiding this comment.
Could you pull parts of this out into a function to avoid the repetition?
greens
reviewed
Jan 12, 2026
Comment on lines
505
to
512
| local parent = device:get_parent_device() | ||
| if parent then | ||
| local info = OUTPUT_BY_KEY[device.parent_assigned_child_key] | ||
| if info then | ||
| handle_output_command(parent, info.suffix, "on") | ||
| return | ||
| end | ||
| end |
Contributor
There was a problem hiding this comment.
code flow is a bit unclear to me here, but I believe you might try using the set_find_child command to handle some of the mapping of parent/child commands a bit more automatically
Contributor
Author
There was a problem hiding this comment.
I think that set_find_child only helps when every component call should go straight to a child device. We still need separate handling for real child outputs, parent output components that share timing logic, and input components that never leave the parent, so the helper wouldn’t simplify anything.
greens
reviewed
Jan 12, 2026
Contributor
greens
left a comment
greens
left a comment
There was a problem hiding this comment.
In general, our code uses 2-space tabs.
greens
reviewed
Jan 16, 2026
Comment on lines
+1
to
+13
| -- Copyright 2025 SmartThings | ||
| -- | ||
| -- Licensed under the Apache License, Version 2.0 (the "License"); | ||
| -- you may not use this file except in compliance with the License. | ||
| -- You may obtain a copy of the License at | ||
| -- | ||
| -- http://www.apache.org/licenses/LICENSE-2.0 | ||
| -- | ||
| -- Unless required by applicable law or agreed to in writing, software | ||
| -- distributed under the License is distributed on an "AS IS" BASIS, | ||
| -- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| -- See the License for the specific language governing permissions and | ||
| -- limitations under the License. |
Contributor
There was a problem hiding this comment.
could you use the new shorter copyright/license statement here?
greens
reviewed
Jan 16, 2026
Comment on lines
+174
to
+181
| if child then | ||
| local on_time = math.floor((sanitize_timing(child.preferences.configOnTime)) * 10) | ||
| local off_wait = math.floor((sanitize_timing(child.preferences.configOffWaitTime)) * 10) | ||
| return on_time, off_wait | ||
| end | ||
| local on_time = math.floor((sanitize_timing(device.preferences["configOnTime" .. suffix]))*10) | ||
| local off_wait = math.floor((sanitize_timing(device.preferences["configOffWaitTime" .. suffix]))*10) | ||
| return on_time, off_wait |
Contributor
There was a problem hiding this comment.
Suggested change
| if child then | |
| local on_time = math.floor((sanitize_timing(child.preferences.configOnTime)) * 10) | |
| local off_wait = math.floor((sanitize_timing(child.preferences.configOffWaitTime)) * 10) | |
| return on_time, off_wait | |
| end | |
| local on_time = math.floor((sanitize_timing(device.preferences["configOnTime" .. suffix]))*10) | |
| local off_wait = math.floor((sanitize_timing(device.preferences["configOffWaitTime" .. suffix]))*10) | |
| return on_time, off_wait | |
| local on_time = math.floor((sanitize_timing(device.preferences["configOnTime" .. suffix]))*10) | |
| local off_wait = math.floor((sanitize_timing(device.preferences["configOffWaitTime" .. suffix]))*10) | |
| if child then | |
| local on_time = math.floor((sanitize_timing(child.preferences.configOnTime)) * 10) | |
| local off_wait = math.floor((sanitize_timing(child.preferences.configOffWaitTime)) * 10) | |
| end | |
| return on_time, off_wait |
also I appreciate the caution, but it's impossible for OUTPUT_INFO[suffix] to ever be nil anywhere you call it
greens
reviewed
Jan 16, 2026
| if config_on_time == 0 then | ||
| device:send(OnOff.server.commands.Off(device):to_endpoint(endpoint)) | ||
| else | ||
| device:send(OnOff.server.commands.OnWithTimedOff(device, data_types.Uint8(0), |
Contributor
There was a problem hiding this comment.
Just to be clear, this is meant to be an OnWithTimedOff?
It seems like the behavior between on and off is identical as long as the config_on_time > 0. Is that correct?
greens
reviewed
Jan 16, 2026
| } | ||
| }, | ||
| FRIENT_IO_MODULE = { | ||
| FINGERPRINTS = { |
Contributor
There was a problem hiding this comment.
nit: the spacing here is inconsistent with the rest of the file
greens
reviewed
Jan 16, 2026
Comment on lines
+440
to
+496
| local function switch_on_handler(driver, device, command) | ||
| local parent = device:get_parent_device() | ||
| if parent then | ||
| local info = OUTPUT_BY_KEY[device.parent_assigned_child_key] | ||
| if info then | ||
| handle_output_command(parent, info.suffix, "on") | ||
| return | ||
| end | ||
| end | ||
|
|
||
| local num = command.component and command.component:match("output(%d)") | ||
| if num then | ||
| handle_output_command(device, num, "on") | ||
| return | ||
| end | ||
| num = command.component:match("input(%d)") | ||
| if num then | ||
| local component = device.profile.components[command.component] | ||
| local value = device:get_latest_state(command.component, Switch.ID, Switch.switch.NAME) | ||
| if value == "on" then | ||
| device:emit_component_event(component, | ||
| Switch.switch.on({ state_change = true, visibility = { displayed = false } })) | ||
| elseif value == "off" then | ||
| device:emit_component_event(component, | ||
| Switch.switch.off({ state_change = true, visibility = { displayed = false } })) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| local function switch_off_handler(driver, device, command) | ||
| local parent = device:get_parent_device() | ||
| if parent then | ||
| local info = OUTPUT_BY_KEY[device.parent_assigned_child_key] | ||
| if info then | ||
| handle_output_command(parent, info.suffix, "off") | ||
| return | ||
| end | ||
| end | ||
|
|
||
| local num = command.component and command.component:match("output(%d)") | ||
| if num then | ||
| handle_output_command(device, num, "off") | ||
| return | ||
| end | ||
| num = command.component:match("input(%d)") | ||
| if num then | ||
| local component = device.profile.components[command.component] | ||
| local value = device:get_latest_state(command.component, Switch.ID, Switch.switch.NAME) | ||
| if value == "on" then | ||
| device:emit_component_event(component, | ||
| Switch.switch.on({ state_change = true, visibility = { displayed = false } })) | ||
| elseif value == "off" then | ||
| device:emit_component_event(component, | ||
| Switch.switch.off({ state_change = true, visibility = { displayed = false } })) | ||
| end | ||
| end | ||
| end |
Contributor
There was a problem hiding this comment.
can you consolidate these functions? it might be useful to use a factory function like we do for thermostats: https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers/blob/main/drivers/SmartThings/zwave-thermostat/src/init.lua#L44
greens
reviewed
Jan 16, 2026
Comment on lines
+42
to
+43
| OUTPUT_1 = "output1", | ||
| OUTPUT_2 = "output2" |
Contributor
There was a problem hiding this comment.
these aren't actually components, they're child devices, right?
greens
reviewed
Jan 16, 2026
Comment on lines
+207
to
+217
| local function emit_switch_event_for_endpoint(device, endpoint, event) | ||
| local info = OUTPUT_BY_ENDPOINT[endpoint] | ||
| if info ~= nil then | ||
| local child = device:get_child_by_parent_assigned_key(info.key) | ||
| if child then | ||
| child:emit_event(event) | ||
| return | ||
| end | ||
| end | ||
| device:emit_event_for_endpoint(endpoint, event) | ||
| end |
Contributor
There was a problem hiding this comment.
Will the native handler actually work for this unique behavior you have here?
The mapping system you've constructed is very confusing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests