-
-
Notifications
You must be signed in to change notification settings - Fork 28.9k
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
Add unique id to emoncms integration #117911
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Hey there @borpin, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
self._attr_unique_id = get_id( | ||
sensorid, elem["tag"], elem["name"], elem["id"], elem["userid"] | ||
) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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}" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
Converted back to draft, as I had a followup question (see above). |
Proposed change
This PR adds
unique_id
for all sensors, so that so users can edit the feed settingsIf you dont have emoncms, you can run it as a standalone docker container : https://hub.docker.com/r/alexjunk/emoncms
BEFORE
AFTER
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: