-
Notifications
You must be signed in to change notification settings - Fork 508
philips-hue: Split mapping of hue resource IDs by bridge #2389
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.
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
Conversation
Channel deleted. |
Minimum allowed coverage is Generated by 🐒 cobertura-action against 856f35f |
return | ||
end | ||
local v2_resource_id = svc_info.rid or "" | ||
if driver:get_device_by_dni(v1_dni) or driver.hue_identifier_to_device_record[v2_resource_id] then return end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what is currently gating us from creating the devices, specifically the last part of the conditional
log.warn(string.format("Failed to get bridge id for %s", bridge.label)) | ||
return nil | ||
end | ||
driver.hue_identifier_to_device_record_by_bridge[bridge_id] = driver.hue_identifier_to_device_record_by_bridge[bridge_id] or {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to log if driver.hue_identifier_to_device_record_by_bridge[bridge_id]
is nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so since it would be expected the first time that we try to get a table for a certain bridge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good from my perspective contingent on how testing goes. I'm curious to see if things will behave nicely with duplicated resource ids. Although, from my testing the old hub should go offline anyways after the migration to the new one.
Tested on my setup and things are looking good. Batch commands and offline/online tracking via zigbee connectivity status are working correctly. This just needs testing with the migration path to make sure the new devices are created. |
I tested the migration path and things are looking good on that front as well. The presence of the old hub and devices no longer prevents the creation of bridged devices on the new hub. |
Check all that apply
Type of Change
Checklist
Description of Change
This change is to help facilitate the migration of an old hue bridge to the pro hue bridge. Currently, the driver holds a mapping of hue resource IDs to smartthings devices records. This becomes a problem when adding the new pro to smartthings because the children of the pro will have the same resource ids as they did when on the old bridge. Child creation is gated by if we already have a record of that hue resource ID. The driver also makes assumptions throughout that the hue resource ID is unique.
By splitting up the mapping by hue bridge, we shouldn't run into any issues with collusions and should be able to re-create the devices on the new pro bridge.
Summary of Completed Tests
By hue setup is currently offline so I haven't had a chance to test this yet.