Skip to content

Matter Switch: Lazy load subdrivers if possible #2525

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Matter Switch: Lazy load subdrivers if possible #2525

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee476a4
Matter Switch: Lazy load subdrivers if possible
nickolas-deboom Nov 4, 2025
6cab4fe
Move lazy_load_if_possible to switch utils file
nickolas-deboom Nov 4, 2025
cec569c
remove unneeded spaces
nickolas-deboom Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions drivers/SmartThings/matter-switch/src/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,9 @@ local matter_driver_template = {
},
supported_capabilities = fields.supported_capabilities,
sub_drivers = {
require("sub_drivers.aqara_cube"),
require("sub_drivers.eve_energy"),
require("sub_drivers.third_reality_mk1")
switch_utils.lazy_load_if_possible("sub_drivers.aqara_cube"),
switch_utils.lazy_load_if_possible("sub_drivers.eve_energy"),
switch_utils.lazy_load_if_possible("sub_drivers.third_reality_mk1")
}
}

Expand Down
12 changes: 12 additions & 0 deletions drivers/SmartThings/matter-switch/src/sub_drivers/aqara_cube/can_handle.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- Copyright 2025 SmartThings, Inc.
-- Licensed under the Apache License, Version 2.0

return function(opts, driver, device )
if device.network_type == require("st.device").NETWORK_TYPE_MATTER then
local name = string.format("%s", device.manufacturer_info.product_name)
if string.find(name, "Aqara Cube T1 Pro") then
return true, require("sub_drivers.aqara_cube")
end
end
return false
end
12 changes: 1 addition & 11 deletions drivers/SmartThings/matter-switch/src/sub_drivers/aqara_cube/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ local INITIAL_PRESS_ONLY = "__initial_press_only" -- for devices that support MS
local CUBEACTION_TIMER = "__cubeAction_timer"
local CUBEACTION_TIME = 3

local function is_aqara_cube(opts, driver, device)
if device.network_type == device_lib.NETWORK_TYPE_MATTER then
local name = string.format("%s", device.manufacturer_info.product_name)
if string.find(name, "Aqara Cube T1 Pro") then
return true
end
end
return false
end

local callback_timer = function(device)
return function()
device:emit_event(cubeAction.cubeAction("noAction"))
Expand Down Expand Up @@ -251,7 +241,7 @@ local aqara_cube_handler = {
}
},
},
can_handle = is_aqara_cube
can_handle = require("sub_drivers.aqara_cube.can_handle")
}

return aqara_cube_handler
Expand Down
11 changes: 11 additions & 0 deletions drivers/SmartThings/matter-switch/src/sub_drivers/eve_energy/can_handle.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-- Copyright 2025 SmartThings, Inc.
-- Licensed under the Apache License, Version 2.0

return function(opts, driver, device )
local EVE_MANUFACTURER_ID = 0x130A
if device.network_type == require("st.device").NETWORK_TYPE_MATTER and
Copy link
Contributor

@hcarter-775 hcarter-775 Nov 4, 2025

Choose a reason for hiding this comment

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

similar to my other comment, I generally believe the require's should be outside the functions in the case it is a static require like this, where we know what we're requiring ahead of time. same goes for the other can_handle functions

Copy link
Contributor Author

@nickolas-deboom nickolas-deboom Nov 4, 2025

Choose a reason for hiding this comment

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

I believe that limiting the scope of require's like this was brought up as part of edge drivers api 2.0, to avoid holding onto references unless needed as well as avoiding pulling in files at the top level when possible.

Copy link
Contributor

@hcarter-775 hcarter-775 Nov 4, 2025

Choose a reason for hiding this comment

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

Hmmm, that makes sense I think. The thing is though, this st.device is already being pulled in at the top level in init.lua, so this is kinda just an extra operation, right?

Copy link
Contributor

@gharveymn gharveymn Nov 4, 2025

Choose a reason for hiding this comment

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

We probably need to better structure some stuff to make it more clear what is actually loaded at a given time, but in this case, st.device will always already be loaded in, so it's fine if you do a static require. Alternatively, maybe you can access it through the device argument of the function?

Copy link
Contributor Author

@nickolas-deboom nickolas-deboom Nov 4, 2025

Choose a reason for hiding this comment

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

This is just comparing the network type retrieved from the device argument against a string value from the lua libs - so I could replace require("st.device").NETWORK_TYPE_MATTER with the string, but not sure if that's worth it considering this module is already loaded at this point as Harrison pointed out.

Copy link
Contributor

@cjswedes cjswedes Nov 5, 2025

Choose a reason for hiding this comment

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

IMO the require("module").some_field is not the easiest to read and would prefer a local variable for the require; whether that is local to the module or function isnt a huge deal, but it seems like we are favoring making it local to the function at this point.

Copy link
Contributor

@hcarter-775 hcarter-775 Nov 5, 2025

Choose a reason for hiding this comment

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

I am of the opinion we should do the requires at the module level for now, if only for general consistency across the Matter drivers. To me, making things "the same" creates a more cohesive environment with more clearly set expectations.

device.manufacturer_info.vendor_id == EVE_MANUFACTURER_ID then
return true, require("sub_drivers.eve_energy")
end
return false
end
13 changes: 1 addition & 12 deletions drivers/SmartThings/matter-switch/src/sub_drivers/eve_energy/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ local SWITCH_INITIALIZED = "__switch_intialized"
local COMPONENT_TO_ENDPOINT_MAP = "__component_to_endpoint_map"
local ON_OFF_STATES = "ON_OFF_STATES"

local EVE_MANUFACTURER_ID = 0x130A
local PRIVATE_CLUSTER_ID = 0x130AFC01

local PRIVATE_ATTR_ID_WATT = 0x130A000A
Expand All @@ -47,16 +46,6 @@ local MINIMUM_ST_ENERGY_REPORT_INTERVAL = (15 * 60) -- 15 minutes, reported in s
-- Eve specifics
-------------------------------------------------------------------------------------

local function is_eve_energy_products(opts, driver, device)
-- this sub driver does not support child devices
if device.network_type == device_lib.NETWORK_TYPE_MATTER and
device.manufacturer_info.vendor_id == EVE_MANUFACTURER_ID then
return true
end

return false
end

-- Return a ISO 8061 formatted timestamp in UTC (Z)
-- @return e.g. 2022-02-02T08:00:00Z
local function epoch_to_iso8601(time)
Expand Down Expand Up @@ -377,7 +366,7 @@ local eve_energy_handler = {
capabilities.energyMeter,
capabilities.powerConsumptionReport
},
can_handle = is_eve_energy_products
can_handle = require("sub_drivers.eve_energy.can_handle")
}

return eve_energy_handler
12 changes: 12 additions & 0 deletions drivers/SmartThings/matter-switch/src/sub_drivers/third_reality_mk1/can_handle.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- Copyright 2025 SmartThings, Inc.
-- Licensed under the Apache License, Version 2.0

return function(opts, driver, device)
local THIRD_REALITY_MK1_FINGERPRINT = { vendor_id = 0x1407, product_id = 0x1388 }
if device.network_type == require("st.device").NETWORK_TYPE_MATTER and
Copy link
Contributor

@hcarter-775 hcarter-775 Nov 4, 2025

Choose a reason for hiding this comment

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

Separately but related, I was thinking we could pull this device-specific sub-driver stuff into the SwitchFields.vendor_overrides table? Then here we could call something like if switch_utils.get_product_override_field(device, "is_third_reality_mk1") then ... end or something like that?

We could do that for all these can_handle functions, since this may be good for moving all this "same" data into one central place

Copy link
Contributor

@hcarter-775 hcarter-775 Nov 4, 2025

Choose a reason for hiding this comment

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

It's technically a separate idea, but it's related enough where I may suggest implementing it here?

Copy link
Contributor Author

@nickolas-deboom nickolas-deboom Nov 4, 2025

Choose a reason for hiding this comment

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

That would definitely be nice, but I might favor doing that in a separate PR because currently, only the 3rd reality can_handle uses an exact vid and pid while the other two have slightly different checks, and I'm not sure if I want to change those here.

device.manufacturer_info.vendor_id == THIRD_REALITY_MK1_FINGERPRINT.vendor_id and
device.manufacturer_info.product_id == THIRD_REALITY_MK1_FINGERPRINT.product_id then
return true, require("sub_drivers.third_reality_mk1")
end
return false
end
16 changes: 1 addition & 15 deletions drivers/SmartThings/matter-switch/src/sub_drivers/third_reality_mk1/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,14 @@

local capabilities = require "st.capabilities"
local clusters = require "st.matter.clusters"
local device_lib = require "st.device"
local im = require "st.matter.interaction_model"
local log = require "log"

local COMPONENT_TO_ENDPOINT_MAP = "__component_to_endpoint_map"

-------------------------------------------------------------------------------------
-- Third Reality MK1 specifics
-------------------------------------------------------------------------------------

local THIRD_REALITY_MK1_FINGERPRINT = { vendor_id = 0x1407, product_id = 0x1388 }

local function is_third_reality_mk1(opts, driver, device)
if device.network_type == device_lib.NETWORK_TYPE_MATTER and
device.manufacturer_info.vendor_id == THIRD_REALITY_MK1_FINGERPRINT.vendor_id and
device.manufacturer_info.product_id == THIRD_REALITY_MK1_FINGERPRINT.product_id then
log.info("Using Third Reality MK1 sub driver")
return true
end
return false
end

local function endpoint_to_component(device, ep)
local map = device:get_field(COMPONENT_TO_ENDPOINT_MAP) or {}
for component, endpoint in pairs(map) do
Expand Down Expand Up @@ -132,7 +118,7 @@ local third_reality_mk1_handler = {
supported_capabilities = {
capabilities.button
},
can_handle = is_third_reality_mk1
can_handle = require("sub_drivers.third_reality_mk1.can_handle")
}

return third_reality_mk1_handler
12 changes: 12 additions & 0 deletions drivers/SmartThings/matter-switch/src/utils/switch_utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
-- See the License for the specific language governing permissions and
-- limitations under the License.

local MatterDriver = require "st.matter.driver"
local fields = require "utils.switch_fields"
local st_utils = require "st.utils"
local clusters = require "st.matter.clusters"
local capabilities = require "st.capabilities"
local log = require "log"
local version = require "version"

local utils = {}

Expand Down Expand Up @@ -279,4 +281,14 @@ function utils.report_power_consumption_to_st_energy(device, latest_total_import
end
end

function utils.lazy_load_if_possible(sub_driver_name)
if version.api >= 16 then
return MatterDriver.lazy_load_sub_driver_v2(sub_driver_name)
elseif version.api >= 9 then
return MatterDriver.lazy_load_sub_driver(require(sub_driver_name))
else
return require(sub_driver_name)
end
end

return utils
Loading