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

[Feature request]: Improve Home Assistant integration by reusing other entity behavior #22098

Open
schauveau opened this issue Apr 5, 2024 · 5 comments
Labels
feature request Feature request

Comments

@schauveau
Copy link

schauveau commented Apr 5, 2024

Is your feature request related to a problem? Please describe

I have multiple plugs with energy monitoring and also a PJ-1203A (Bidirectional energy meter with 80A current clamp). The former are working fine in Home Assistant but for the later, I noticed that I cannot display statistic graphs for the power_a, power_b and power_ab entities. I eventually figured out that this is because those entities are missing in the Home Assistant integration.

power: {device_class: 'power', entity_category: 'diagnostic', state_class: 'measurement'},

Adding the missing entities in the lookup table works fine (until I update zigbee2mqtt) but there are other devices that use different names for similar values (e.g. power_l1,power_phase_a, load, ... ) so a generic solution could be welcome. The annoying thing is that there is no way to control the HA integration from the converter itself.

I understand that Home Assistant features should not be exposed in zigbee-herdsman-converters but I believe that there is an easy solution.

Describe the solution you'd like

The integration in homeassistant.ts is making extensive use of lookup tables using the entity name as key. This is not a good approach because a lot of converters (including external ones) are using names that are not listed there.

My proposal is to introduce a new field behavior and an associated member in all Expose objects:

     withBehavior(behavior: string) {
        this.behavior = behavior
     }

In homeassistant.ts , a few occurrences of lookup[firstExpose.name ] must be transformed into lookup[firstExpose.behavior || firstExpose.name ].

The idea is that the keys in homeassistant.ts are now treated as behaviors and the default behavior is the name of the entity. The changes are very small and should not break any existing converter.

Any entity or predefined expose helper can then declared to follow a known behavior foobar using .withBehavior('foobar')

It could be worth spending a few minutes or hours to clean up the lookup tables since some entries are now redundant (e.g. add .withBehavior('voltage') in voltageWithPhase() and then remove voltage_phase_b and voltage_phase_c from homeassistant.ts ). As a second thought, obsolete behaviors should probably be kept to avoid breaking external converters that may still be using them (emit a warning?).

Describe alternatives you've considered

The HA integration could be tuned using the unit (as for Wh and kWh) but this is not very flexible.

Additional context

none

@Koenkk
Copy link
Owner

Koenkk commented May 4, 2024

Can't we solve this by stripping the endpoint from the name? E.g. if you would have name == power_l1 and endpoint == l1, stripping the endpoint would give you power.

@schauveau
Copy link
Author

Yes and No. That could work when the suffix comes from the endpoint but this is not always the case. The PJ-12304 is using Tuya datapoints so there is no endpoint.

The proposed feature is more flexible especially ifbehavior was initialized with the initial name in the Expose objhect constructor. Something like new Numeric('power', access.STATE). withEndpoint('l1') would automatically set the behavior to power.

Similarly, a new method Base.withSuffix could be added to append a suffix to the name.

    withSuffix(suffix: string) {
       this.name = this.name+"_"+suffix;
    } 

For example voltage_phase_b is currently implemented with

     voltage_phase_b: () => new Numeric('voltage_phase_b', access.STATE).withLabel('Voltage phase B').withUnit('V').withDescription('Measured electrical potential value on phase B'),

It could become either

    voltage_phase_b: () => new Numeric('voltage', access.STATE)..withSuffix('b').withLabel('Voltage phase B').withUnit('V').withDescription('Measured electrical potential value on phase B'),

or

    voltage_phase_b: () => new Numeric('voltage_phase_b', access.STATE).withBehavior('voltage').withLabel('Voltage phase B').withUnit('V').withDescription('Measured electrical potential value on phase B'),

Both would result in name=voltage_phase_b and behavior=voltage.

@Koenkk
Copy link
Owner

Koenkk commented May 4, 2024

The PJ-12304 is using Tuya datapoints so there is no endpoint.

I haven't tried, but I think it should give no problems when using the withEndpoint() here. I'm a bit hesitant to add all kinds of attributes to exposes which are just for HA

@schauveau
Copy link
Author

Do not see the behavior as something specific to HA. This is more a kind of hint about the real nature of the attribute.

Relying on the attribute name is really not nice since that requires that all possible names should be hard-coded in homeassistant.ts.

Consider for instance the recent TIC device: https://github.com/Koenkk/zigbee-herdsman-converters/blob/cd1638889237d50de2f62e1fed490d22294e4d15/src/devices/gmmts.ts#L219 .
Do you really want to add those 104 new attributes to homeassistant.ts?
It is far easier to add a behavior column in the table and then do a .withBehavior(...) in the converter code.

@Koenkk
Copy link
Owner

Koenkk commented May 5, 2024

Do you really want to add those 104 new attributes to homeassistant.ts?

Definitely not! But how would you see this? There is a specific set of withBehavior values that can be set? So e.g. withBehavior('temperature') would replace temperature and local_temperature. I think we still need a lot of hardcoded values in HA + refactor all current exposes to use withBehavior()

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

No branches or pull requests

2 participants