maorcc / citymind_water_meter

Home Assistant Integration with cp.city-mind.com, an Israeli water meters reading service
Apache License 2.0
40 stars 2 forks source link

Bad Entity Names - seems to be translated from Hebrew #71

Closed GuyKh closed 2 months ago

GuyKh commented 3 months ago

See the entity_name here; it used to be citymind_meter_123456_last_reading or so, now it seems that it's very vaguely translating Hebrew to something.

image

This is probably related to moving to the Devices data structure; where the prefix of the entity would be the device name. I wonder if this can be reverted (not manually)

elad-bar commented 3 months ago

Translation for entity from name to id is being done by HA

GuyKh commented 3 months ago

@elad-bar but this is a very bad user experience. Check this: https://developers.home-assistant.io/docs/core/entity/#has_entity_name-true-mandatory-for-new-integrations

I think that you should (re)consider the Device name (as it's always added as the prefix to the entity_name), to be changed to something shorter - e.g. just meter id. Where the address would be in the device description or something

elad-bar commented 3 months ago

device name is built from the address & meter id, same as appear in RYM Pro website, that provide uniqueness (which was not available before in the example you have provided) of the device name (meter id can be the same in different municipalities).

integration is aligned with the guidelines available in the link you have shared above:

The friendly_name state attribute is generated by combining then entity name with the device name as follows:

The entity is not a member of a device: friendly_name = entity.name
The entity is a member of a device and entity.name is not None: friendly_name = f"{device.name} {entity.name}"
The entity is a member of a device and entity.name is None: friendly_name = f"{device.name}"

in your case:

device name = "הנחל 47, חדרה 123456"
entity name = "Consumption Forecast"

if the length of the entity name is too long, you can change it using the exiting functionality of HA, the right user experience is to be clear and have meaningful of its purpose rather shorter name for those who type the name, especially when tools to:

are available out of the box within HA

hope it make sense, if you have better idea, open to hear it

thanks

GuyKh commented 3 months ago

I'd suggest that device_name would be water_meter_123456 (or similar) rather than the address itself - so sensors would appear as: sensor.water_meter_123456_consumption_forecast. The address can be as another field of the device

elad-bar commented 2 months ago

pls review v3.0.6, you can change (as explained in the release notes) under the account - turn off unique names flag it will work as described and as you asked for

hope it works for you

GuyKh commented 2 months ago

@elad-bar Woah - impressive behavior. FIrst time I see that configuration on the account actually does something. Looks very well! Thank you!

elad-bar commented 2 months ago

Thanks, glad to hear that it works for you

GuyKh commented 2 months ago

@elad-bar I had in my system both citymind and read-your-meter integrations. I think that the sensor.meter_x_last_read overrides each other.

elad-bar commented 2 months ago

You can rename the device name in one of the integrations, i can't control or know name of other integrations, Read your meter pro, is an implementation based on this integrations so duplications will take place if you run them both, will see later if i can add additional prefix easily

GuyKh commented 2 months ago

My suggestion is just something about adding a minor rename of sorts i.e. water_meter_12345 etc - per your decision

elad-bar commented 2 months ago

Makes sense...

elad-bar commented 2 months ago

v3.0.7 released with that name adjustement

GuyKh commented 2 months ago

@elad-bar - after updating the component - reinstall? just flip the config toggle off and on? should I do anything?

elad-bar commented 2 months ago

just flip (should be off)

elad-bar commented 2 months ago

pls just let me know if it works and we can close the issue

thanks

GuyKh commented 2 months ago

Worx :)