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

possible solution for issue #26028 #26324

Merged
merged 5 commits into from
Feb 13, 2025
Merged

possible solution for issue #26028 #26324

merged 5 commits into from
Feb 13, 2025

Conversation

xs400dohc
Copy link
Contributor

Made some investigation:

  • as written in my post, energy has a device_class and state_class power and current don't.
  • searched for the reason why energy behaves different to power and current
  • in homeassistant.ts I found this code in line 1018:
                // If a variable includes Wh, mark it as energy
                if (firstExpose.unit && ['Wh', 'kWh'].includes(firstExpose.unit)) {
                    Object.assign(extraAttrs, {device_class: 'energy', state_class: 'total_increasing'});
                }

I think a possible solution could be to add following

                // If a variable includes A or mA, mark it as energy
                if (firstExpose.unit && ['A', 'mA'].includes(firstExpose.unit)) {
                    Object.assign(extraAttrs, {device_class: 'current', state_class: 'measurement'});
                }

                // If a variable includes mW, W, kW, MW, GW, TW, mark it as energy
                if (firstExpose.unit && ['mW', 'W', 'kW', 'MW', 'GW', 'TW'].includes(firstExpose.unit)) {
                    Object.assign(extraAttrs, {device_class: 'power', state_class: 'measurement'});
                }

But I don't know how to test the changes in my homeassistant if that would fix the problem. Didn't found homeassistant.ts in my installation (using VS-Code-extension). Perhaps it's also way for other sensors with other units, e.g. Data rate in bit/s, kbit/s, Mbit/s, Gbit/s, B/s, kB/s, MB/s, GB/s, KiB/s, MiB/s or GiB/s (see here).

Made some investigation:
- as written in my post, energy has a device_class and state_class power and current don't.
- searched for the reason why energy behaves different to power and current
- in [homeassistant.ts](https://github.com/Koenkk/zigbee2mqtt/blob/master/lib/extension/homeassistant.ts) I found this code in line 1018:
```
                // If a variable includes Wh, mark it as energy
                if (firstExpose.unit && ['Wh', 'kWh'].includes(firstExpose.unit)) {
                    Object.assign(extraAttrs, {device_class: 'energy', state_class: 'total_increasing'});
                }

```
I think a possible solution could be to add following
```
                // If a variable includes A or mA, mark it as energy
                if (firstExpose.unit && ['A', 'mA'].includes(firstExpose.unit)) {
                    Object.assign(extraAttrs, {device_class: 'current', state_class: 'measurement'});
                }

                // If a variable includes mW, W, kW, MW, GW, TW, mark it as energy
                if (firstExpose.unit && ['mW', 'W', 'kW', 'MW', 'GW', 'TW'].includes(firstExpose.unit)) {
                    Object.assign(extraAttrs, {device_class: 'power', state_class: 'measurement'});
                }

```
But I don't know how to test the changes in my homeassistant if that would fix the problem. Didn't found homeassistant.ts in my installation (using VS-Code-extension).
Perhaps it's also way for other sensors with other units, e.g. Data rate in bit/s, kbit/s, Mbit/s, Gbit/s, B/s, kB/s, MB/s, GB/s, KiB/s, MiB/s or GiB/s (see [here](https://www.home-assistant.io/integrations/sensor#device-class)).
@xs400dohc
Copy link
Contributor Author

Just saw, I forgot to change "mark it as power" and "mark it as current" instead of "mark it as energy" in the comments. Don't know how to edit the pr.

}

// If a variable includes mW, W, kW, MW, GW, TW, mark it as energy
if (firstExpose.unit && ['mW', 'W', 'kW', 'MW', 'GW', 'TW'].includes(firstExpose.unit)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if (firstExpose.unit && ['mW', 'W', 'kW', 'MW', 'GW', 'TW'].includes(firstExpose.unit)) {
if (firstExpose.unit && ['mW', 'W', 'kW'].includes(firstExpose.unit)) {

It's unlikely that these will ever be there. Also could you make it and if/else if statement?

@Koenkk Koenkk merged commit 1e656c2 into Koenkk:dev Feb 13, 2025
13 checks passed
@Koenkk
Copy link
Owner

Koenkk commented Feb 13, 2025

Thanks!

@xs400dohc xs400dohc deleted the patch-3 branch February 13, 2025 18:28
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

Successfully merging this pull request may close these issues.

2 participants