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

Set HA entity_category to mark e.g. battery readings as diagnostic #2115

Open
pointhi opened this issue Nov 17, 2024 · 5 comments
Open

Set HA entity_category to mark e.g. battery readings as diagnostic #2115

pointhi opened this issue Nov 17, 2024 · 5 comments

Comments

@pointhi
Copy link

pointhi commented Nov 17, 2024

Is your feature request related to a problem? Please describe.
Devices with e.g. a battery status show them as sensor readings. However, those readings should be listed as diagnostic as they are more for diagnostic purpose than for everyday reading and only clutter the normal sensor view.

This could also be said for the whole set of controls and sensors published by OpenMQTTGateway itself to control the system (which can use the entity_category config instead).

Describe the solution you'd like

See: https://developers.home-assistant.io/blog/2021/10/26/config-entity/ and https://developers.home-assistant.io/docs/core/entity/#generic-properties

I wrote a very hacky patch for ZMqttDiscovery createDiscovery to show a MVP. With this patch, all batteries are now marked as diagnostic in HA.

Of course, proper implementation of this feature request should be a bit more extensive than this and also include other device classes like data_size, signal_strength,... or be dependent on the sensor itself.

  if (device_class && device_class[0]) {
    // We check if the class belongs to HAAS classes list
    int num_classes = sizeof(availableHASSClasses) / sizeof(availableHASSClasses[0]);
    for (int i = 0; i < num_classes; i++) { // see class list and size into config_mqttDiscovery.h
      if (strcmp(availableHASSClasses[i], device_class) == 0) {
        sensor["dev_cla"] = device_class; //device_class
+        if (strcmp("battery", device_class) == 0) {
+          sensor["ent_cat"] = "diagnostic";
+        }
      }
    }
  }

This results in something like:

image

@1technophile
Copy link
Owner

Good catch, a PR will be welcome if you want to propose this change

@DigiH
Copy link
Collaborator

DigiH commented Nov 18, 2024

Just be careful about BM2 and BM6 battery monitors, which only have the battery level (and voltage for the BM2) as their properties.

And should voltage then be in the Diagnostic section only as well?

While this can be easily achieved for the above two devices with an exception in the rule, it will be more difficult, if not impossible, to create any exceptions for any device responding to the Service Data Battery decoder.

https://github.com/theengs/decoder/blob/development/src/devices/ServiceData_json.h

Personally I do prefer the battery status within the device's properties, if I don't want to see it in a dashboard implementation I can just remove this single or several properties, so it could also be possible to keep this unified or separated with a setting for individual user preference, and to avoid the possible pitfalls mentioned above.

With signal_strength I also agree, as we already are discovering this for rtl_433 devices, also still unified.

Just some of my thoughts about this.

@pointhi
Copy link
Author

pointhi commented Nov 20, 2024

before implementing anything, it would be good to define when we want to mark a property as diagnostics. E.g. only for specific types of devices if this is possible instead of hard-coding the battery device_class.

@DigiH
Copy link
Collaborator

DigiH commented Nov 20, 2024

I suppose the device_classes, like battery, voltage and rssi would fall into the diagnostic category, but as the above mentioned BM2/BM6 have the battery and voltage property as their main device properties, all I can see at the moment is to filter them manually when implementing your above suggested device_class diagnostic sorting.

I did something similar to the rtl_433 battery_ok property, as for some devices have this property as a binary 0/1 while other devices do send a full 0-100% range. This was due to the fact that rtl_433 doesn't differentiate the type of any battery_ok property, which would have been very helpful otherwise and also future proof.

Here, instead of manually having to filter for model_id, and maintain this for any future battery property devices, one other solution could be to define the difference in the decoders, so that BM2/BM6 could have their battery and voltage properties as batt_prop and voltage_prop, but then we'd actually need to look at the property name in addition to the device_class and unit.

@1technophile what is your view on this on how this could be implemented the best way?

@puterboy
Copy link
Contributor

puterboy commented Nov 28, 2024

I am STRONGLY not in favor of making this change.
I like all the entities of a device to be grouped together. Especially, since the device names (the non-friendly ones) are often rather opaque so scattering them elsewhere make them almost useless.

If I don't want to see a certain entity, I can hide it from the dashboard. Plus one person's diagnostic is another person's signal.

Indeed, I think we need to be very careful about changing the classification of devices as it creates a lot of downstream potential incompatibilities.
For example when battery for some devices was changed from sensor to binary_sensor, it required a fair bit of rework to some of my automations and code to adjust. But at least that was a legit fix in that the battery_ok was actually binary data.

In contrast, labeling something as diagnostic is a matter of semantics and perspective -- not a foundational fact.

Better to focus on adding new functionality or fixing real bugs versus rearranging the furniture for things that already work just fine!

If you want to put all the battery statuses somewhere else, then go create a panel with them and label the panel diagnostics.

If it ain't broke... don't fix it... Because your fix can be someone else's nightmare!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants