tsightler / ring-mqtt

Ring devices to MQTT Bridge
MIT License
600 stars 106 forks source link

add entity_category support to discovery message and entities. #767

Closed xannor closed 10 months ago

xannor commented 11 months ago

This should add entity_category support to the device entities I found in /devices.

I did my best guess as to if the entity was a config entity or diagnostic entity (or neither.) I only own two floodlight cams and a spotlight cam, so I am not familiar with many of the entities. So if a switch or selector is more of a diagnostic thing than a config thing (the panic buttons in the alarm panel would be my best guess of this.) they might be best served in the diagnostic category instead of config.

I also am not sure how to test to make sure I didn't mess things up, as I run the addon and I have never developed/debugged one to know how to use a dev version, but the changes are fairly minor so I do not for-see any complications.

tsightler commented 11 months ago

Thanks for the PR. I'll have to research more about this before I consider any possibility of merging this. Entity categories change how sensors are presented in the HA UI as well and I don't really agree that things like the "stream" switch being catorgorized as "config" as they reflect state rather than configure behavior. Maybe I could move these to diagnostic, because that's mostly what they are, but I'm not sure I like the UI behavior.

Honestly, this project is mostly in maintenance mode at the moment and I'm not big on making any significant changes to it right now due to lack of time to address any issues, but if I ever find enough time to release a new version, I'll consider merging this.

xannor commented 11 months ago

That is why I was unsure of some of the categorizations, as I did not fully understand the context it may have been used for.

Mostly the categories determine how an entity shows in the device view, and whether or not is shows in "auto" views, etc. I.E. categorized entities are generally ignored, like the default dashboard. It would not effect anything that actively uses the entity, nor make it unavailable for automation, etc. Adding this would not hide or remove any entities other than auto views, but I under stand your hesitance, especial if you consider this project in maintenance.

I only suggested this, because I was adding the home bridge integration, and without using the manual yaml config, you cannot cherry pick entities, and my three cameras caused a flood of things to need to be configured on the home side, that I didnt need or want, and this is the only way to "stem" that.

tsightler commented 11 months ago

Yeah, but the other place it changes is in the device view, where it moves those entities from their existing "Controls" and "Sensors" panel to "Configuration" and "Diagnostic" panels, but it only makes sense to move entities there if they belong in those panels. I'd argue that most of these do not fit cleanly into either of these, although a few clearly do.

I'm not really sure it would fit your goal. From my perspective Home Assistant is about providing this level of fine-gain control, not trying to make assumptions about what users want exposed or not. I certainly explicitly configure every entity I want to expose to my home voice assistant, for example. I get that HA has moved more and more toward basic users, but I personally hate that direction and it's part of what is pushing me away from the HA ecosystem and supporting it at all.

Regardless, I'm going to leave this open and think about it some more.

xannor commented 11 months ago

Ok, to add to your thoughts.

An opt in config setting that enable the categorization as it is. This would allow those that want it get it but by default leaves things as is.

I can figure out how to modify your config portion, to add the option, but I am not sure how to translate that to the addon side of things.

tsightler commented 11 months ago

I definitely don't want even more config options, that's just more things to maintain. As I said, I'll consider adding the categorization, although likely with some changes for specific entities. Probably won't happen until I work on the next version, which could be months from now at this point. But I may merge this into dev to use as a baseline prior to then. Thanks for your efforts.