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 item ID to active notification attributes to keep them unique #2856

Merged
merged 1 commit into from
Sep 10, 2022

Conversation

dshokouhi
Copy link
Member

Summary

Was helping a user on discord who mentioned they did not see all notification data in the attributes for active notification sensor, turns out we need to add the ID as well to keep the attributes unique otherwise they will overwrite each other.

Ref: https://discord.com/channels/330944238910963714/562408603345092636/1016456857574985738

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

Copy link
Member

@jpelgrom jpelgrom left a comment

Choose a reason for hiding this comment

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

It works

(though I'm not sure how useful it is and if it meets the current standards, dumping so many attributes in a sensor and with slightly different formats for package name + id depending on where the attribute is from)

@dshokouhi
Copy link
Member Author

I havent seen anything in the docs about too many attributes, I normally refer to the data docs to make sure the DB will be fine. The attributes in this case describe the state as best as possible. At least now we are not missing any attribute data. I think in this case the user wants to display the notifications from the device.

@ScottG489
Copy link

Yeah I think it's great that this information is available.

I do somewhat agree with @jpelgrom, but I'm not sure there's an alternative. Sensors and attributes don't provide a great way to create a data structure. I almost wonder if it would be better to cram them all into a single attribute as json or something. I might even like that idea less, but it would at least make it easier to get at the information in a structured way.

Perhaps we could consider documenting the format we're using to create attributes?

Copy link
Collaborator

@JBassett JBassett left a comment

Choose a reason for hiding this comment

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

Approving for now since people who are using this really need to know what they are doing already. However, I agree we are starting to get out of the spirit of what a sensor is for Android

@JBassett JBassett merged commit 0ce8cbe into home-assistant:master Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants