Skip to content

Commit

Permalink
fix: Remove incorrect dimming logic.
Browse files Browse the repository at this point in the history
This was some of the first code I wrote for the Hue driver, and I had
a misunderstanding of the `min_dimming` property returned by the API.

The `min_dimming` value in the API is informative, and represents a percent
of the total lumens the device is capable of outputting that you will get
if you request the lowest possible brightness for the bulb (1%).

We were interpreting it as a lower bound on the values to send, which is
not correct.

This PR removes the lower bound checks, and instead clamps all dimming commands
to the range of [1,100] inclusive.
  • Loading branch information
dljsjr committed Dec 19, 2024
1 parent 143c23d commit 18ae4c5
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 19 deletions.
1 change: 0 additions & 1 deletion drivers/SmartThings/philips-hue/src/consts.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@ Consts.MIN_TEMP_KELVIN_COLOR_AMBIANCE = 2000
Consts.MIN_TEMP_KELVIN_WHITE_AMBIANCE = 2200
Consts.MAX_TEMP_KELVIN = 6500
Consts.KELVIN_STEP_SIZE = 11
Consts.DEFAULT_MIN_DIMMING = 2

return Consts
2 changes: 0 additions & 2 deletions drivers/SmartThings/philips-hue/src/fields.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
--- @field BRIDGE_SW_VERSION string The SW Version of the bridge to determine if it supports CLIP V2
--- @field DEVICE_TYPE string Field on all Hue devices that indicates type, maps to a Hue service rtype.
--- @field BRIDGE_API string Transient field that holds the HueAPI instance for the bridge
--- @field MIN_DIMMING string Minimum dimming/brightness value accepted by a light
--- @field EVENT_SOURCE string Field on a bridge that stores a handle to the SSE EventSource client.
local Fields = {
_ADDED = "added",
Expand All @@ -24,7 +23,6 @@ local Fields = {
IPV4 = "ipv4",
IS_ONLINE = "is_online",
IS_MULTI_SERVICE = "is_multi_service",
MIN_DIMMING = "mindim",
MIN_KELVIN = "mintemp",
MAX_KELVIN = "maxtemp",
MODEL_ID = "modelid",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,7 @@ local function _emit_light_events_inner(light_device, light_repr)
end

if light_repr.dimming then
local min_dim = light_device:get_field(Fields.MIN_DIMMING)
local api_min_dim = st_utils.round(st_utils.clamp_value(light_repr.dimming.min_dim_level or Consts.DEFAULT_MIN_DIMMING, 1, 100))
if min_dim ~= api_min_dim then
min_dim = api_min_dim
light_device:set_field(Fields.MIN_DIMMING, min_dim, { persist = true })
log.info("EMITTING DIMMING CAP RANGE")
light_device:emit_event(capabilities.switchLevel.levelRange({ minimum = min_dim, maximum = 100 }))
end
local adjusted_level = st_utils.round(st_utils.clamp_value(light_repr.dimming.brightness, min_dim, 100))
local adjusted_level = st_utils.round(st_utils.clamp_value(light_repr.dimming.brightness, 1, 100))
if utils.is_nan(adjusted_level) then
light_device.log.warn(
string.format(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
local log = require "log"
local capabilities = require "st.capabilities"
local refresh_handler = require("handlers.commands").refresh_handler
local st_utils = require "st.utils"

Expand Down Expand Up @@ -137,13 +138,6 @@ function LightLifecycleHandlers.added(driver, device, parent_device_id, resource

---@type HueLightInfo
local light_info = Discovery.device_state_disco_cache[device_light_resource_id]
local minimum_dimming = 2

if light_info.dimming and light_info.dimming.min_dim_level then
minimum_dimming = st_utils.round(st_utils.clamp_value(light_info.dimming.min_dim_level, 1, 100))
end

device:set_field(Fields.MIN_DIMMING, minimum_dimming, { persist = true })

-- Remembering that mirek are reciprocal to kelvin, note the following:
-- ** Minimum Mirek -> _Maximum_ Kelvin
Expand Down Expand Up @@ -218,6 +212,8 @@ function LightLifecycleHandlers.init(driver, device)
svc_rids_for_device[device_light_resource_id] = HueDeviceTypes.LIGHT
end
device:set_field(Fields._INIT, true, { persist = false })
device:emit_event(capabilities.switchLevel.levelRange({ minimum = 1, maximum = 100 }))

if device:get_field(Fields._REFRESH_AFTER_INIT) then
refresh_handler(driver, device)
device:set_field(Fields._REFRESH_AFTER_INIT, false, { persist = true })
Expand Down

0 comments on commit 18ae4c5

Please sign in to comment.