marthoc / homeseer

HomeSeer Custom Integration for Home Assistant
MIT License
29 stars 12 forks source link

RFC: Naming of entities #4

Closed marthoc closed 4 years ago

marthoc commented 4 years ago

@AndreBremer and @kmitch95120 have both opened PR's which address the way entities are named in this component and propose different changes to the config parameters that control naming.

I would appreciate if we could all agree on a solution before I delve into looking at your specific changes.

The real issue (custom naming of entities) could be resolved here if we could provide entities with a unique ID - they could then take advantage of the entity registry and the ability to rename the entity and change its entity id from within Home Assistant. The problem is that HS3 doesn't expose any uniquely identifying info for a device (e.g. a UUID or serial #) over the API. We could use e.g. the device ref of the device as the unique id, though this is not truly unique. We could also use e.g. a hash of the device ref and name of the device (or other characteristics) which ups the uniqueness.

Thoughts?

kmitch95120 commented 4 years ago

I like Andre's approach. It's cleaner than what I did. As Andre stated, I also think we should remove location_names and just use name_template instead. Yes, it's a breaking change, but just document it and move on.

I don't think we need to go so far as to create unique entity ids and we definitely don't want to use the HS device name in a hash, as that's easily changed by the HS user and not unique. For example, I have 10 devices names "Temperature" with each being in a different location.

AndreBremer commented 4 years ago

The ref id is the only appropriate ID you can use, given the limited capabilities of the HomeSeer API. While it's not globally unique, it is immutable and guaranteed to be unique within a given HomeSeer instance. Once you remove a device, HomeSeer will not reuse its ref id.

To construct an entity id, it would be good practice to add an identifier representing the HomeSeer instance namespace. This setup would also allow for the possibility to use multiple HomeSeer instances, since the ref id namespaces won't clash. There are two options:

  1. Use the http IP address and port. The resulting entity id would look like: [http_ip]:[http_port]:[ref_id]. The downside is that devices would get a new identity if at any point the IP address changes.
  2. Use a user-supplied sufficiently unique identifier/symbol in the configuration (something like namespace: myhs). The resulting entity id would look like: [namespace]:[ref_id]. The downside here is that devices would get a new identity if the user inadvertently ever changes the namespace in their config.

I could go with either approach.

A proper long term solution would involve a bridge plug-in for HomeSeer, that would present a more complete datamodel. Not sure if that is worth it though.

kmitch95120 commented 4 years ago

I support either approach but option 2 seems a bit better. It seems much less likely that a user would need to change their namespace vs. the need to change the IP address, or even more likely, the http_port, of the HS machine. A warning in the documentation regarding changing the namespace and the ramifications of doing so, would be good.

marthoc commented 4 years ago

I've pushed changes into branch '0.4' that implement the "namespace" idea above. Namespace is a required key in configuration.yaml that can be set to any string you choose. Unique id for each device will be "namespace-ref". This should enable the entity registry allowing you to rename entities from the UI. Please test!

To change to this 0.4 version:

  1. Stop Home Assistant.
  2. cd into custom_components/homeseer.
  3. git pull
  4. git checkout 0.4
  5. Start Home Assistant.

0.4 also incorporates pyhs3 0.11.

kmitch95120 commented 4 years ago

Seems to be working fine but I'm not sure I understand what this change does. The entity_id is still as if location_names was set to true. Yes, I can go edit them after the fact, but doing so appears to break any automation using the entity_id. I must be missing something.

AndreBremer commented 4 years ago

The point of change is to referential integrity between HomeAssistant and HomeSeer when entity names change. I'll check it out tomorrow.

My only concern is that people like me who have 50+ Z-Wave devices now have to manually rename entities in HomeAssistant. I think it would still make sense to have a default name template to avoid unnecessary busy work. I went through this with HomeAssistant's Z-Wave stack and it was a pain.

kmitch95120 commented 4 years ago

I expected to see something like "sensor.namespace-ref" for each entity with a friendly name that could be changed to whatever I wanted. I need to go off and read about the entity registry and unique ids.

Even with this implementation, I think we still need the name template. I have 84 z-wave devices and I wouldn't want to have to manually rename each one.

AndreBremer commented 4 years ago

You are confusing unique IDs with entity IDs.

Unique IDs are not user facing. They are stored in the entity registry to strongly link the representation of the entity in HomeAssistant to its underlying implementation. Since the HomeSeer component did not present unique IDs, its entities were not persisted in the registry. As a result, you were not able to edit their names. It also meant that if - at any point in the future - you change the device name in HomeSeer, any dependent automation in HomeAssistant will break, because the entity names and IDs will change as a result as well.

So ideally, you want HomeSeer to 'seed' default names (and entity ids), but then treat them independently between the two systems and just rely on the unique ID to link them.

kmitch95120 commented 4 years ago

Ah, yes, I was confused. That explanation helps a lot. I can confirm that a change to a device name in HS does not break anything in HA. A name template for the default name and entity id would still be useful.

marthoc commented 4 years ago

@AndreBremer I’m not against still using the template to give you advanced control over how the entities are initially added. Keep in mind that due to the entity registry, the template will only have an effect on entity id and entity name the first time starting HA with a new namespace (after that, the entities will be registered with their unique ID and the template won’t have a further effect, even if changed).

Agreed that renaming a ton of entities is a pain but that is how the entity registry works unfortunately, and the use of such a template is an anomaly among HA integrations. Still, this is a custom component and we get to make the rules, so why not?

AndreBremer commented 4 years ago

Yep, that makes sense. It would also cover any new devices popping up in HomeSeer. In my quest to find the best Z-Wave integration, I have done this renaming business so many times, I forgot to count. :) Templating would be a huge help.

Btw... this is by far the best Z-Wave solution yet. The OZW stack in HA is a hot mess and the UX in HomeSeer feels like it's straight out of the 90s. This is the best of both worlds and low latency too!

If you hadn't developed this, I would have started this myself. Kudos!

AndreBremer commented 4 years ago

@marthoc I tested 0.4 and it appears to work as intended. I also merged your changes into my template branch. To avoid churn, I will hold back updating the PR until you merge 0.4 into master.

marthoc commented 4 years ago

@AndreBremer Thanks! No question that Z-Zwave support is the best part of HomeSeer, but also that Home Assistant is leaps and bounds ahead in many other respects. I initially just wrote this and pyhs3 for me to get my own Z-Wave devices into HA. Then @kmitch95120 asked about pyhs3 and I thought I’d make it public! Glad you’re getting use out of it.

marthoc commented 4 years ago

@AndreBremer @kmitch95120 This has now been addressed in #3 and I will close this Issue when 0.4 is merged to master.