markvader / ha-rpi_rf

Home Assistant Raspberry Pi GPIO RF Integration
Apache License 2.0
31 stars 5 forks source link

Add unique_id functionality. Required for further customization in HA UI #48

Closed oskargert closed 1 year ago

oskargert commented 1 year ago

Hi,

First of all, thank you for putting this together after they removed the funtionality from HA. I've used your component for a couple of months without any problem. But I have been a little annoyed with not being able to customize the devices in the HA UI, such as putting them into a room, giving them a "nice" name and changing the default icon. To do this you need to add a unique id to each device. Because of this I decided to clone your repo and add the unique id functionality.

markvader commented 1 year ago

I was thinking of this as well, I have a few thoughts. I personally want all my entities to be configurable via UI so do appreciate the pull request.

oskargert commented 1 year ago

Thank you for your feedback.

Could one solution be to document that a "manual" unique_id has to be defined in cases of using the same on/off codes for more than one switch and in the case of a switch with several codes?

In the case for a switch with several codes, the auto generation could also used simply be turned off, to force the user to define a manual unique_id?

markvader commented 1 year ago

Am merging into testing branch for now

markvader commented 1 year ago
  • In regards to the link to good practice, I would say that the on/off codes are the most unique values we have available?

agreed

  • I think the likelihood for a user adding two switches with the same codes is low, but not zero. We could possibly solve this by appending a UUID identifier, but then isn't the UUID enough by itself? My concern is also that a new ID would be generated for the switch on changes to the file or restart of the server. Do you know if this would be the case?

I don't believe that to be the case, but would need to do further reading of the documentation.

  • Would it be necessary to only use the first code? I don't see the problem with using the entire string?

True, will test it to be sure

Could one solution be to document that a "manual" unique_id has to be defined in cases of using the same on/off codes for more than one switch and in the case of a switch with several codes?

In the case for a switch with several codes, the auto generation could also used simply be turned off, to force the user to define a manual unique_id?

Ideally we'd convert the integration to a config flow (i.e. UI based rather than yaml).

Will test out a few scenarios and come back to you

markvader commented 1 year ago

Tested the following scenarios

  1. added an extra device to the configuration file with the same codes as a previous device, wouldn't expect it to work as it wouldn't be a unique_id. From the Log file Platform rpi_rf does not generate unique IDs. ID [1234567]_[7654321] already exists - ignoring switch.ORIGINALSWITCHNAME So works as expected

  2. Trying a device with multiple codes passed. Uniqueid formed with these lists of codes [1234567,5896874,5678901][7654321,5675896,8745678] Works as expected

  3. Trying a device with multiple codes passed for on and a single code passed for off. Uniqueid formed with these lists of codes [1234567,5896874,5678901][7654321] Works as expected

  4. Change one of the codes in YAML after original setup (original device was called switch.test_rf_device. From the Log file 2022-09-26 17:26:49.039 INFO (MainThread) [homeassistant.helpers.entity_registry] Registered new switch.rpi_rf entity: switch.test_rf_device_2 switch.test_rf_device shows in device list as "restored", as it knows it was discovered in the past but not currently available. This entity can be safely removed.

Was just thinking again, it is so unlikely that someone uses the more than one device in their home with the same RF code as that would be impossible to control a single device. If any issues do arise it will be easy enough to append a UUID to the unique_id.

I'll add documentation to readme about added additional functionality now (icon, location etc)

Thanks for the pull request @oskargert

oskargert commented 1 year ago

Great, seems like the tests worked out as expected. I agree that this is an edge case and can be solved through documentation. I'm glad I could help and contribute a little. thank you for you a good and useful component!

markvader commented 1 year ago

I've published the new release. Have a read of the documentation and see if you think it could be improved.

Thanks again.

oskargert commented 1 year ago

The documentation is good, my only comment would be that you could add the unique_id key to the configuration example and the options table, since it can be entered manually. A manually entered id would also fix the issue of keeping the same id when changing/adding the on/off codes.

markvader commented 1 year ago

The documentation is good, my only comment would be that you could add the unique_id key to the configuration example and the options table, since it can be entered manually. A manually entered id would also fix the issue of keeping the same id when changing/adding the on/off codes.

Thanks. Documentation updated commit

oskargert commented 1 year ago

Perfect, thanks for including my feedback and letting me be a small part of your project

markvader commented 1 year ago

Very welcome, thanks for taking the lead on this one.