Skip to content

Commit

Permalink
fix: Add some hardening to the Hue discovery logic.
Browse files Browse the repository at this point in the history
The first thing we do is we store the API keys in the datastore right
away instead of waiting for a device record to exist. This hardens us
against the situation where the driver restarts for any reason while
the Hue Bridge device's `added` handler is still executing and is early
enough in its execution that we haven't stored the key on the device
object yet.

We also address a regression from the automatic add/delete changes,
which can lead to the creation of duplicate device records for bulbs in
some specific circumstances.
  • Loading branch information
dljsjr committed Feb 5, 2024
1 parent c175368 commit 6b5fe02
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
3 changes: 3 additions & 0 deletions drivers/SmartThings/philips-hue/src/disco.lua
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ local utils = require "utils"
local SERVICE_TYPE = "_hue._tcp"
local DOMAIN = "local"

-- This `api_keys` table is an in-memory fall-back table. It gets overwritten
-- with a reference to a driver datastore table before the Driver's `run` loop
-- can get spun up in `init.lua`.
local HueDiscovery = {
api_keys = {},
disco_api_instances = {},
Expand Down
45 changes: 44 additions & 1 deletion drivers/SmartThings/philips-hue/src/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,15 @@ local function do_bridge_network_init(driver, bridge_device, bridge_url, api_key

local bridge_api = bridge_device:get_field(Fields.BRIDGE_API)
cosock.spawn(function()
Discovery.scan_bridge_and_update_devices(driver, bridge_device:get_field(Fields.BRIDGE_ID))
-- We don't want to do a scan if we're already in a discovery loop,
-- because the even tsource connection will open if a bridge is discovered
-- and we'll effectively be scanning twice.
-- Two scans that find the same device close together can emit events close enough
-- together that the dedupe logic at the cloud layer will get bypassed and lead to
-- duplicate device records.
if not Discovery.discovery_active then
Discovery.scan_bridge_and_update_devices(driver, bridge_device:get_field(Fields.BRIDGE_ID))
end
local child_device_map = {}
local children = bridge_device:get_child_list()
bridge_device.log.debug(string.format("Scanning connectivity of %s child devices", #children))
Expand Down Expand Up @@ -1414,6 +1422,8 @@ local function remove(driver, device)
event_source:close()
device:set_field(Fields.EVENT_SOURCE, nil)
end

Discovery.api_keys[device.device_network_id] = nil
end
end

Expand Down Expand Up @@ -1554,6 +1564,39 @@ if hue.datastore["dni_to_device_id"] == nil then
hue.datastore["dni_to_device_id"] = {}
end


if hue.datastore["api_keys"] == nil then
hue.datastore["api_keys"] = {}
end

if Discovery.api_keys and next(Discovery.api_keys) ~= nil then
local datastore_dirty = false
for k, v in pairs(Discovery.api_keys) do
hue.datastore.api_keys[k] = v
datastore_dirty = true
end

if datastore_dirty then
hue.datastore:save()
end
end

Discovery.api_keys = setmetatable({}, {
__newindex = function (self, k, v)
assert(
type(v) == "string",
string.format("Attempted to store value of type %s in application_key table which expects \"string\" types",
type(v)
)
)
hue.datastore.api_keys[k] = v
hue.datastore:save()
end,
__index = function(self, k)
return hue.datastore.api_keys[k]
end
})

-- Kick off a scan right away to attempt to populate some information
hue:call_with_delay(3, Discovery.do_mdns_scan, "Philips Hue mDNS Initial Scan")

Expand Down

0 comments on commit 6b5fe02

Please sign in to comment.