Fix UUIDs for OccupancyGroups #56
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note: This is a bit of a hack, but Lutron currently sets the UUID of Occupancy Groups to the same value as the Occupancy Group ID. There's nothing in the API documentation that guarantees this will be so, but changing it would be a breaking change, and the only thing that's likely to use this in the near future is Home Assistant and having any ID is better than None.
Since we have the UUID as just a string, the value itself is opaque. What if we append a suffix (or prepend a prefix) to the ID? That way the clients (i know it's just HASS at this point :) ) won't be able to rely on the fact that they are the same and be forced to properly index them separately? So later when we parse it and properly map them, the clients wouldn't care.
I don't think it'd be a breaking change from Lutron's perspective since they provide the mapping, or did you mean something else? etc.
Makes sense. |
|
On Sun, Jun 14, 2020 at 12:49 PM Dima Zavin ***@***.***> wrote:
This is a bit of a hack, but Lutron currently sets the UUID
of Occupancy Groups to the same value as the Occupancy Group ID.
There's nothing in the API documentation that guarantees this will
Since we have the UUID as just a string, the value itself is opaque. What
if we append a suffix (or prepend a prefix) to the ID? That way the clients
(i know it's just HASS at this point :) ) won't be able to rely on the fact
that they are the same and be forced to properly index them separately? So
later when we parse it and properly map them, the clients wouldn't care.
Sure, that's easy enough. It's only the occupancy groups that do this.
Everything else appears to be somewhat random.
… be so, but changing it would be a breaking change, and the only
I don't think it'd be a breaking change from Lutron's perspective since
they provide the mapping, or did you mean something else?
etc.
thing that's likely to use this in the near future is Home Assistant
and having any ID is better than None.
Makes sense.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#56 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACQARWSTCSDUUGGEP7OSMGLRWUSSZANCNFSM4N5KGAIA>
.
|
|
Ok I take that back...
If anyone indexes these, separately or not, if Lutron changes this
behavior, it will break that system. It seems like appending or prepending
something is just signing up pylutron to permanently prepend something but
not actually gaining anything in the process.
On Wed, Jun 17, 2020 at 10:16 PM Chris Heiser <chris.heiser@gmail.com>
wrote:
…
On Sun, Jun 14, 2020 at 12:49 PM Dima Zavin ***@***.***>
wrote:
> This is a bit of a hack, but Lutron currently sets the UUID
> of Occupancy Groups to the same value as the Occupancy Group ID.
> There's nothing in the API documentation that guarantees this will
>
> Since we have the UUID as just a string, the value itself is opaque. What
> if we append a suffix (or prepend a prefix) to the ID? That way the clients
> (i know it's just HASS at this point :) ) won't be able to rely on the fact
> that they are the same and be forced to properly index them separately? So
> later when we parse it and properly map them, the clients wouldn't care.
>
Sure, that's easy enough. It's only the occupancy groups that do this.
Everything else appears to be somewhat random.
> be so, but changing it would be a breaking change, and the only
>
> I don't think it'd be a breaking change from Lutron's perspective since
> they provide the mapping, or did you mean something else?
> etc.
>
> thing that's likely to use this in the near future is Home Assistant
> and having any ID is better than None.
>
> Makes sense.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#56 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACQARWSTCSDUUGGEP7OSMGLRWUSSZANCNFSM4N5KGAIA>
> .
>
|
|
FYI @JonGilmore I have home-assistant/core#36902 queued up to overhaul the lutron hass component with unique IDs and some other goodies once this UUID snafu is resolved. |
Maybe I'm missing something (probably). What are we signing up pylutron for? The value is opaque, but unique. HASS shouldn't be doing anything with those values other than use them as some unique identifier, right? The API from pylutron is that there's a uuid present and nothing else, right. However, in your patch you are assuming that they are always the same but we know that according to what's in Lutron XML, that's not at all guaranteed hence they provide the mapping. The point of the prefix/suffix is to force the clients not to assume that the UUID is the same as Occupancy Group ID. We are exactly gaining pylutron not signing us up for maintaing that equality. The proper solution would be to parse the mapping, but it's not necessary to start with as we discussed. I don't understand what this would be breaking exactly? Do the UUIDs get recorded somewhere and somehow associated with the object outside of the ephemeral building of the device database at runtime? |
|
Yes, the UUIDs get stored in config entries in home assistant. If you show
up with the same device with a new UUID, it will get a new device id. So
if say in version 13.0 they decide to create entirely new UUIDs, it will
take all existing devices offline and create new ones in their place.
Of course, you have to have actually upgraded your firmware for such an
event to happen.
…On Mon, Jun 22, 2020 at 4:15 AM Dima Zavin ***@***.***> wrote:
Ok I take that back... If anyone indexes these, separately or not, if
Lutron changes this behavior, it will break that system. It seems like
appending or prepending something is just signing up pylutron to
permanently prepend something but not actually gaining anything in the
process.
Maybe I'm missing something (probably). What are we signing up pylutron
for? The value is opaque, but unique. HASS shouldn't be doing anything with
those values other than use them as some unique identifier, right? The API
from pylutron is that there's a uuid present and nothing else, right.
However, in your patch you are assuming that they are always the same but
we know that according to what's in Lutron XML, that's not at all
guaranteed hence they provide the mapping. The point of the prefix/suffix
is to force the clients not to assume that the UUID is the same as
Occupancy Group ID. We are exactly gaining pylutron not signing us up for
maintaing that equality. The proper solution would be to parse the mapping,
but it's not necessary to start with as we discussed.
I don't understand what this would be breaking exactly? Do the UUIDs get
recorded somewhere and somehow associated with the object outside of the
ephemeral building of the device database at runtime?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#56 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACQARWRXWX7S7ETGWIEGQX3RX44NHANCNFSM4N5KGAIA>
.
|
|
I just rewrote the code to actually parse out the occupancy groups and bind to the areas. This will properly show the occupancy group for all areas regardless of the presence of a sensor. (My pending pull request for the home assistant component will fix that) |
I think that's going to be a problem regardless though. The IDs can be renumbered based just on the configuration change, doesn't have to be a firmware update. I've definitely had things renumber when I moved stuff around and modified things in Essentials. Based on my experience with messing with Lutron configs over the years, these numbers are not at all guaranteed to be stable between different re-generaitons of the configuration when you transfer the db from software to the master controller. |
|
I'll have to run some testing tomorrow to see what RA2 Essentials does
these days. Changing the UUIDs for devices because the device has
materially changed (different device, different room, etc...) seems
completely reasonable. Changing all the UUIDs just because you register a
new device, or change something small in the configuration would definitely
be problematic.
The only other identifiers available are the name and the serial number of
the physical device. I've stayed away from the serial numbers as ghost
devices are actually somewhat useful.
…On Mon, Jun 22, 2020 at 9:27 PM Dima Zavin ***@***.***> wrote:
Yes, the UUIDs get stored in config entries in home assistant. If you show
up with the same device with a new UUID, it will get a new device id. So if
say in version 13.0 they decide to create entirely new UUIDs, it will take
all existing devices offline and create new ones in their place. Of course,
you have to have actually upgraded your firmware for such an event to
happen.
I think that's going to be a problem regardless though. The IDs can be
renumbered based just on the configuration change, doesn't have to be a
firmware update. I've definitely had things renumber when I moved stuff
around and modified things in Essentials.
Based on my experience with messing with Lutron configs over the years,
these numbers are not at all guaranteed to be stable between different
re-generaitons of the configuration when you transfer the db from software
to the master controller.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#56 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACQARWUNKT6KSOPZZSRDOQ3RYAVLVANCNFSM4N5KGAIA>
.
|
|
I've done some testing today which has done the following:
- Add a new device
- Remove a device
- Replace a device (keypad replaced with dimmer)
- Rename a device
All of these only impacted UUIDs for a new/removed device. renaming a
device doesn't change the UUID.
I don't konw if/how it could change with a firmware upgrade, but at least
that's not something that a user might encounter even with just user
manipulated events, etc.. (that you can do outside of essentials).
On Tue, Jun 23, 2020 at 11:59 PM Chris Heiser <chris.heiser@gmail.com>
wrote:
… I'll have to run some testing tomorrow to see what RA2 Essentials does
these days. Changing the UUIDs for devices because the device has
materially changed (different device, different room, etc...) seems
completely reasonable. Changing all the UUIDs just because you register a
new device, or change something small in the configuration would definitely
be problematic.
The only other identifiers available are the name and the serial number of
the physical device. I've stayed away from the serial numbers as ghost
devices are actually somewhat useful.
On Mon, Jun 22, 2020 at 9:27 PM Dima Zavin ***@***.***>
wrote:
> Yes, the UUIDs get stored in config entries in home assistant. If you
> show up with the same device with a new UUID, it will get a new device id.
> So if say in version 13.0 they decide to create entirely new UUIDs, it will
> take all existing devices offline and create new ones in their place. Of
> course, you have to have actually upgraded your firmware for such an event
> to happen.
>
> I think that's going to be a problem regardless though. The IDs can be
> renumbered based just on the configuration change, doesn't have to be a
> firmware update. I've definitely had things renumber when I moved stuff
> around and modified things in Essentials.
>
> Based on my experience with messing with Lutron configs over the years,
> these numbers are not at all guaranteed to be stable between different
> re-generaitons of the configuration when you transfer the db from software
> to the master controller.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#56 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACQARWUNKT6KSOPZZSRDOQ3RYAVLVANCNFSM4N5KGAIA>
> .
>
|
|
Thanks for testing, appreciate it. My windows machine is under the weather
so I can't play with it myself :( One other thing to try is to add a new
area and add a device to it, if you don't mind.
Then I'll be happy to merge it.
…On Wed, Jun 24, 2020, 10:55 AM cdheiser ***@***.***> wrote:
I've done some testing today which has done the following:
- Add a new device
- Remove a device
- Replace a device (keypad replaced with dimmer)
- Rename a device
All of these only impacted UUIDs for a new/removed device. renaming a
device doesn't change the UUID.
I don't konw if/how it could change with a firmware upgrade, but at least
that's not something that a user might encounter even with just user
manipulated events, etc.. (that you can do outside of essentials).
On Tue, Jun 23, 2020 at 11:59 PM Chris Heiser ***@***.***>
wrote:
> I'll have to run some testing tomorrow to see what RA2 Essentials does
> these days. Changing the UUIDs for devices because the device has
> materially changed (different device, different room, etc...) seems
> completely reasonable. Changing all the UUIDs just because you register a
> new device, or change something small in the configuration would
definitely
> be problematic.
>
> The only other identifiers available are the name and the serial number
of
> the physical device. I've stayed away from the serial numbers as ghost
> devices are actually somewhat useful.
>
> On Mon, Jun 22, 2020 at 9:27 PM Dima Zavin ***@***.***>
> wrote:
>
>> Yes, the UUIDs get stored in config entries in home assistant. If you
>> show up with the same device with a new UUID, it will get a new device
id.
>> So if say in version 13.0 they decide to create entirely new UUIDs, it
will
>> take all existing devices offline and create new ones in their place. Of
>> course, you have to have actually upgraded your firmware for such an
event
>> to happen.
>>
>> I think that's going to be a problem regardless though. The IDs can be
>> renumbered based just on the configuration change, doesn't have to be a
>> firmware update. I've definitely had things renumber when I moved stuff
>> around and modified things in Essentials.
>>
>> Based on my experience with messing with Lutron configs over the years,
>> these numbers are not at all guaranteed to be stable between different
>> re-generaitons of the configuration when you transfer the db from
software
>> to the master controller.
>>
>> —
>> You are receiving this because you authored the thread.
>> Reply to this email directly, view it on GitHub
>> <#56 (comment)>,
>> or unsubscribe
>> <
https://github.com/notifications/unsubscribe-auth/ACQARWUNKT6KSOPZZSRDOQ3RYAVLVANCNFSM4N5KGAIA
>
>> .
>>
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#56 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALQIMKHNF33IVT5NQHSNNLRYI4XNANCNFSM4N5KGAIA>
.
|
|
I'll try that tonight and let you know
…On Sun, Jun 28, 2020, 11:36 AM Dima Zavin ***@***.***> wrote:
Thanks for testing, appreciate it. My windows machine is under the weather
so I can't play with it myself :( One other thing to try is to add a new
area and add a device to it, if you don't mind.
Then I'll be happy to merge it.
On Wed, Jun 24, 2020, 10:55 AM cdheiser ***@***.***> wrote:
> I've done some testing today which has done the following:
>
> - Add a new device
> - Remove a device
> - Replace a device (keypad replaced with dimmer)
> - Rename a device
>
> All of these only impacted UUIDs for a new/removed device. renaming a
> device doesn't change the UUID.
>
> I don't konw if/how it could change with a firmware upgrade, but at least
> that's not something that a user might encounter even with just user
> manipulated events, etc.. (that you can do outside of essentials).
>
>
>
> On Tue, Jun 23, 2020 at 11:59 PM Chris Heiser ***@***.***>
> wrote:
>
> > I'll have to run some testing tomorrow to see what RA2 Essentials does
> > these days. Changing the UUIDs for devices because the device has
> > materially changed (different device, different room, etc...) seems
> > completely reasonable. Changing all the UUIDs just because you
register a
> > new device, or change something small in the configuration would
> definitely
> > be problematic.
> >
> > The only other identifiers available are the name and the serial number
> of
> > the physical device. I've stayed away from the serial numbers as ghost
> > devices are actually somewhat useful.
> >
> > On Mon, Jun 22, 2020 at 9:27 PM Dima Zavin ***@***.***>
> > wrote:
> >
> >> Yes, the UUIDs get stored in config entries in home assistant. If you
> >> show up with the same device with a new UUID, it will get a new device
> id.
> >> So if say in version 13.0 they decide to create entirely new UUIDs, it
> will
> >> take all existing devices offline and create new ones in their place.
Of
> >> course, you have to have actually upgraded your firmware for such an
> event
> >> to happen.
> >>
> >> I think that's going to be a problem regardless though. The IDs can be
> >> renumbered based just on the configuration change, doesn't have to be
a
> >> firmware update. I've definitely had things renumber when I moved
stuff
> >> around and modified things in Essentials.
> >>
> >> Based on my experience with messing with Lutron configs over the
years,
> >> these numbers are not at all guaranteed to be stable between different
> >> re-generaitons of the configuration when you transfer the db from
> software
> >> to the master controller.
> >>
> >> —
> >> You are receiving this because you authored the thread.
> >> Reply to this email directly, view it on GitHub
> >> <#56 (comment)
>,
> >> or unsubscribe
> >> <
>
https://github.com/notifications/unsubscribe-auth/ACQARWUNKT6KSOPZZSRDOQ3RYAVLVANCNFSM4N5KGAIA
> >
> >> .
> >>
> >
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <#56 (comment)>,
or
> unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AALQIMKHNF33IVT5NQHSNNLRYI4XNANCNFSM4N5KGAIA
>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#56 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACQARWQ6YDVLYBRZCJNUZ4LRY6ESFANCNFSM4N5KGAIA>
.
|
|
I added a new dummy room with device and got a new area with new
integration IDs and UUIDs but everything else stayed the same.
I'd also prefer if Lutron would set the proper content type on their XML so
that I didn't have to jump through hoops just to parse it properly.
…On Sun, Jun 28, 2020 at 1:32 PM Chris Heiser ***@***.***> wrote:
I'll try that tonight and let you know
On Sun, Jun 28, 2020, 11:36 AM Dima Zavin ***@***.***>
wrote:
> Thanks for testing, appreciate it. My windows machine is under the weather
> so I can't play with it myself :( One other thing to try is to add a new
> area and add a device to it, if you don't mind.
>
> Then I'll be happy to merge it.
>
>
>
> On Wed, Jun 24, 2020, 10:55 AM cdheiser ***@***.***> wrote:
>
> > I've done some testing today which has done the following:
> >
> > - Add a new device
> > - Remove a device
> > - Replace a device (keypad replaced with dimmer)
> > - Rename a device
> >
> > All of these only impacted UUIDs for a new/removed device. renaming a
> > device doesn't change the UUID.
> >
> > I don't konw if/how it could change with a firmware upgrade, but at
> least
> > that's not something that a user might encounter even with just user
> > manipulated events, etc.. (that you can do outside of essentials).
> >
> >
> >
> > On Tue, Jun 23, 2020 at 11:59 PM Chris Heiser ***@***.***>
> > wrote:
> >
> > > I'll have to run some testing tomorrow to see what RA2 Essentials does
> > > these days. Changing the UUIDs for devices because the device has
> > > materially changed (different device, different room, etc...) seems
> > > completely reasonable. Changing all the UUIDs just because you
> register a
> > > new device, or change something small in the configuration would
> > definitely
> > > be problematic.
> > >
> > > The only other identifiers available are the name and the serial
> number
> > of
> > > the physical device. I've stayed away from the serial numbers as ghost
> > > devices are actually somewhat useful.
> > >
> > > On Mon, Jun 22, 2020 at 9:27 PM Dima Zavin ***@***.***>
> > > wrote:
> > >
> > >> Yes, the UUIDs get stored in config entries in home assistant. If you
> > >> show up with the same device with a new UUID, it will get a new
> device
> > id.
> > >> So if say in version 13.0 they decide to create entirely new UUIDs,
> it
> > will
> > >> take all existing devices offline and create new ones in their
> place. Of
> > >> course, you have to have actually upgraded your firmware for such an
> > event
> > >> to happen.
> > >>
> > >> I think that's going to be a problem regardless though. The IDs can
> be
> > >> renumbered based just on the configuration change, doesn't have to
> be a
> > >> firmware update. I've definitely had things renumber when I moved
> stuff
> > >> around and modified things in Essentials.
> > >>
> > >> Based on my experience with messing with Lutron configs over the
> years,
> > >> these numbers are not at all guaranteed to be stable between
> different
> > >> re-generaitons of the configuration when you transfer the db from
> > software
> > >> to the master controller.
> > >>
> > >> —
> > >> You are receiving this because you authored the thread.
> > >> Reply to this email directly, view it on GitHub
> > >> <#56 (comment)
> >,
> > >> or unsubscribe
> > >> <
> >
> https://github.com/notifications/unsubscribe-auth/ACQARWUNKT6KSOPZZSRDOQ3RYAVLVANCNFSM4N5KGAIA
> > >
> > >> .
> > >>
> > >
> >
> > —
> > You are receiving this because your review was requested.
> > Reply to this email directly, view it on GitHub
> > <#56 (comment)>,
> or
> > unsubscribe
> > <
> https://github.com/notifications/unsubscribe-auth/AALQIMKHNF33IVT5NQHSNNLRYI4XNANCNFSM4N5KGAIA
> >
> > .
> >
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#56 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACQARWQ6YDVLYBRZCJNUZ4LRY6ESFANCNFSM4N5KGAIA>
> .
>
|
thecynic
requested changes
Jul 1, 2020
… new Area. If for some reason we enter a future where an area references an OccupancyGroup that we fail to parse out, we'll now get a friendly warning message.
|
Bump. Note once we get this merged, if we can push another pypi release, I have a draft PR for overhauling the Lutron integration in HA |
thecynic
approved these changes
Jul 10, 2020
|
Pushed 0.2.7 to PyPi with these last 3 PRs |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a bit of a hack, but Lutron currently sets the UUID
of Occupancy Groups to the same value as the Occupancy Group ID.
There's nothing in the API documentation that guarantees this will
be so, but changing it would be a breaking change, and the only
thing that's likely to use this in the near future is Home Assistant
and having any ID is better than None.