Skip to content
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

Device Version: Existing Capabilities Migration #150

Open
MichaelKohler opened this issue Nov 2, 2018 · 8 comments
Open

Device Version: Existing Capabilities Migration #150

MichaelKohler opened this issue Nov 2, 2018 · 8 comments

Comments

@MichaelKohler
Copy link
Member

Follow-up from #18 (comment):

Currently the device version number will trigger an upgrade of the components. However, this currently doesn't account for migration of existing capabilities (for example rename "PWR OM" to "POWER ON"). This issue will track the work on this.

@tmrobert8
Copy link

Can it also include removed capabilities as well? I know it could potentially break rules - but I don't think that would be a deal breaker (and you could potentially use the red exclamation mark to show rules that were broken)..

@nklerk
Copy link
Contributor

nklerk commented Nov 14, 2018

Hi @MichaelKohler , I have noticed the same using my homey implementation.
What is interesting is that when renaming a switch label and adding or removing a component the name change will reflect.

I've tested this with a switch and a slider and both have the renaming issue. didn't test the other components yet.

Lets say you start with a button and a switch. with names: button, switch.

  1. rename the switch to switch1, Result names: button, switch
  2. rename the button to button1, Result names: button1, switch
  3. rename the switch to switch2, Result names: button1, switch
  4. rename the button to button2, Result names: button2, switch
  5. remove the button, Result names: button2, switch2

or same start situation:

  1. rename the switch to switch1, Result names: button, switch
  2. rename the button to button1, Result names: button1, switch
  3. rename the switch to switch2, Result names: button1, switch
  4. rename the button to button2, Result names: button2, switch
  5. add a new button, Result names: button2, switch2

Video demoing this: https://youtu.be/Dpng_d0xZpQ

@nklerk
Copy link
Contributor

nklerk commented Nov 20, 2018

Will this be fixed in 0.52.x? If not i could implement a workaround in my Homey app.

@pfiaux
Copy link
Contributor

pfiaux commented Nov 21, 2018

@tmrobert8 removal is a tricky one. The main argument against it is if there's a shortcut to X and X is removed, either the shortcut is broken or it's automatically removed. Meaning for the user easier it's broken or we're messing with their configuration by removing things.
I think it could be even more annoying if it's not a shortcut but a step in a rule as you mentioned. The red exclamation mark would make it more visible but it still wouldn't solve the problem that we're potentially breaking things for the user. If we solve the cannot remove component from SDK problem but introduce the broken/gone shortcut/step for the user it feels more like shifting the problem than solving it.
Currently I would recommend not removing or if you need to do a major change, perhaps it can be done using a new separate driver (if there's a lot to remove a V2 driver without legacy support for example).

@nklerk I'm not sure what you mean, I'll try to clarify how it currently works. The way it works is we need a unique identifier for each component to be able to match them, that's the name. The label is handled differently, it's a property of the component so it can be updated as long as the name doesn't change (same goes for sensor range, and other properties they should all update).
All components will behave in the same way.
By migration to rename we mean specifically the name which we use as identifier on the Brain to know the component.
For now I would recommend if possible when generating components dynamically, like in your app, to use a random UUID for each such that the "id" (in most cases the name) is decoupled from the label (and renaming it via label wont affect the id). That should work.

@nklerk
Copy link
Contributor

nklerk commented Nov 21, 2018

In my integration when a user adds, removes or renames(label change) a component I ++1 the version number and register the device adapter. The user can only change the label, not the actual name. making the name UUID based is a good thought and i will definitely move that way.

I've added the issue on Planet NEEO beta section. but was instructed to look here.

Short description would be that a label change on a sensor based capability (switch,slider etc) won't be updated on the brain side. Only after adding or removing a button the label changes of the sensor based capabilities get updated. I know this sounds weird but it is what it is.

The PN description i used.

I found some issues regarding the SDK Update feature.
Changing the name of a button using the SDK works like shown: 00:47, 00:58, 01:04. This works really wel but i found issues on other capabilities:

Changing the name of a slider or switch (posibly more capabilitie types but not tested) only work when a button is added or deleted.
Switch name changed : 01:18, neeo UI reloads but switch name doesn't get changed.
Rename a button: 01:27, The button gets renamed as suppose to but switch name still keeps it's name.
Adding a new button: 1:39, The Switch name gets updated. (same works with removing a button but the result is not shown in this capture)
showrtcuts in "Edit shortcurs" will keep their original name. 01:55

https://youtu.be/Dpng_d0xZpQ

@pfiaux
Copy link
Contributor

pfiaux commented Dec 11, 2018

@nklerk the issue was on the NEEO Brain, the updating code added all new components but stopped after the first updated component. In the case of a switch, there's a switch and a sensor, the sensor would be updated but not the switch. Then if you added another button after that it would run the "next" update it didn't get to which was the switch.

Note: it will be fixed in 0.52.x

@nklerk
Copy link
Contributor

nklerk commented Dec 11, 2018

Cool! Thanks for your help.

@gvdhoven
Copy link

In addition; can we show the custom driver version somewhere? I now added it to my discovery description, but it feels 'off'; Preferably it should be shown beneath the device name..

Sample screenshot:
https://i.postimg.cc/vTwKS8PV/Screenshot-20190220-184141-NEEO.jpg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants