Skip to content

Don't include Off for supportedFanModes for thermostats by default #2428

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 1 commit into
base: main
Choose a base branch
from
Open

Don't include Off for supportedFanModes for thermostats by default #2428

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 15 additions & 8 deletions drivers/SmartThings/matter-thermostat/src/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,11 @@ local function system_mode_handler(driver, device, ib, response)
return
end

local supported_modes = device:get_latest_state(device:endpoint_to_component(ib.endpoint_id), capabilities.thermostatMode.ID, capabilities.thermostatMode.supportedThermostatModes.NAME) or {}
local supported_modes = device:get_latest_state(
device:endpoint_to_component(ib.endpoint_id),
capabilities.thermostatMode.ID,
capabilities.thermostatMode.supportedThermostatModes.NAME
) or {}
-- check that the given mode was in the supported modes list
if tbl_contains(supported_modes, THERMOSTAT_MODE_MAP[ib.data.value].NAME) then
device:emit_event_for_endpoint(ib.endpoint_id, THERMOSTAT_MODE_MAP[ib.data.value]())
Expand Down Expand Up @@ -1470,7 +1474,7 @@ local function sequence_of_operation_handler(driver, device, ib, response)
if device:get_field(OPTIONAL_THERMOSTAT_MODES_SEEN) == nil then
device:set_field(OPTIONAL_THERMOSTAT_MODES_SEEN, {capabilities.thermostatMode.thermostatMode.off.NAME}, {persist=true})
end
local supported_modes = utils.deep_copy(device:get_field(OPTIONAL_THERMOSTAT_MODES_SEEN))
local supported_modes = utils.deep_copy(device:get_field(OPTIONAL_THERMOSTAT_MODES_SEEN) or {})
Copy link
Contributor

Choose a reason for hiding this comment

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

due to the logic in line 1475, this will literally never be empty, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, good point 😅

local disallowed_mode_operations = {}

local modes_for_inclusion = {}
Expand Down Expand Up @@ -1578,17 +1582,20 @@ end
local function fan_mode_sequence_handler(driver, device, ib, response)
local supportedFanModes, supported_fan_modes_attribute
if ib.data.value == clusters.FanControl.attributes.FanModeSequence.OFF_LOW_MED_HIGH then
supportedFanModes = { "off", "low", "medium", "high" }
supportedFanModes = { "low", "medium", "high" }
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean we'll be deleting options from currently existing thermostat devices with this capability? What are the numbers on this- specifically routines using off for thermostats? Assume it's very low (maybe 0), but still worth checking. Otherwise this looks good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great point, I will try to figure out how many there are if any before proceeding. If there are a lot of existing rules we could opt to only implement this change for new devices

elseif ib.data.value == clusters.FanControl.attributes.FanModeSequence.OFF_LOW_HIGH then
supportedFanModes = { "off", "low", "high" }
supportedFanModes = { "low", "high" }
elseif ib.data.value == clusters.FanControl.attributes.FanModeSequence.OFF_LOW_MED_HIGH_AUTO then
supportedFanModes = { "off", "low", "medium", "high", "auto" }
supportedFanModes = { "low", "medium", "high", "auto" }
elseif ib.data.value == clusters.FanControl.attributes.FanModeSequence.OFF_LOW_HIGH_AUTO then
supportedFanModes = { "off", "low", "high", "auto" }
supportedFanModes = { "low", "high", "auto" }
elseif ib.data.value == clusters.FanControl.attributes.FanModeSequence.OFF_HIGH_AUTO then
supportedFanModes = { "off", "high", "auto" }
supportedFanModes = { "high", "auto" }
else
supportedFanModes = { "off", "high" }
supportedFanModes = { "high" }
end
if get_device_type(device) ~= THERMOSTAT_DEVICE_TYPE_ID then
table.insert(supportedFanModes, 1, "off")
end

if device:supports_capability_by_id(capabilities.airPurifierFanMode.ID) then
Expand Down
Loading