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

Add unique id to emoncms integration #117911

14 changes: 5 additions & 9 deletions homeassistant/components/emoncms/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,13 @@ def __init__(
id_for_name = "" if str(sensorid) == "1" else sensorid
# Use the feed name assigned in EmonCMS or fall back to the feed ID
feed_name = elem.get("name") or f"Feed {elem['id']}"
self._name = f"EmonCMS{id_for_name} {feed_name}"
self._attr_name = f"EmonCMS{id_for_name} {feed_name}"
else:
self._name = name
self._identifier = get_id(
self._attr_name = name
self._attr_unique_id = get_id(
sensorid, elem["tag"], elem["name"], elem["id"], elem["userid"]
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Names cannot be part of a unique identifier.

See also our developer documentation around this, which lists the requirements: https://developers.home-assistant.io/docs/entity_registry_index/#unique-id

../Frenck

Copy link
Contributor Author

@alexandrecuer alexandrecuer May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tag and name removed from the unique id

emoncms API does not provide mac address at the moment, so I've kept sensor id, feed id and emoncms user_id

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these unique across different installations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sensor id must be unique as specified in the doc :

id integer Required

Positive integer identifier for the sensor. Must be unique if you specify multiple Emoncms sensors.

cf https://www.home-assistant.io/integrations/emoncms/#id

unless I'm mistaken, I understand it is what you call unique id of last resort in the doc ?
https://developers.home-assistant.io/docs/entity_registry_index/#unique-id-of-last-resort

I've seen it used like that in the modbus integration

self._attr_unique_id = f"{self._attr_unique_id}_{idx}"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless I'm mistaken, I understand it is what you call unique id of last resort in the doc ?

No, that is not the ID @ emoncms, that is the UUID of the config flow entry. As this integration doesn't have a configuration flow, it means there is no unique ID of last resort available.

In that case, this integration first needs to be migrated to a config flow (and thus UI configuration).

../Frenck

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK sorry, I was misled when I saw that the modbus did that way without having a flow config.

So going to close that PR, there is more work than expected

self._attr_native_unit_of_measurement = unit_of_measurement
self._hass = hass
self._data = data
self._value_template = value_template
Expand Down Expand Up @@ -197,11 +198,6 @@ def __init__(
else:
self._state = None

@property
def name(self):
"""Return the name of the sensor."""
return self._name

@property
def native_unit_of_measurement(self):
"""Return the unit of measurement of this entity, if any."""
Expand Down Expand Up @@ -243,7 +239,7 @@ def update(self) -> None:
elem["id"],
elem["userid"],
)
== self._identifier
== self._attr_unique_id
),
None,
)
Expand Down