wlcrs / huawei_solar

Home Assistant integration for Huawei Solar inverters via Modbus
GNU Affero General Public License v3.0
499 stars 80 forks source link

[Bug]: 1.4.0a3 - Battery Sensors #685

Closed Roving-Ronin closed 2 months ago

Roving-Ronin commented 2 months ago

Describe the issue

Hi Thijs,

Not sure if this is a bug or ???, but notice that even in 1.4.0a3 under 'Batteries' (plural) it is still showing 'Battery 2 temperature', albeit in a disabled by default state. Is this supposed to be there, as looking at the inidividual battery devices, I note that 'Battery 1' has its ( battery_battery_1_temperature ) displaying within the list of sensors for that specific device.

image

Given this, I would have assumed that 'battery_battery_2_temperature' should also be displaying under the individual view for Battery 2 (if one was actually installed)?

Also could you advise what the 'sensor.battery_1_none' and 'sensor.battery_1_none_2' that appear in the individual battery device are supposed to be for? So far the 'sensor.battery_1_none' continues to report "Remote scheduling maximum self use", whilst the other doesn't report anything.

image

BTW how do you actually get sensors to be disabled or hidden by default in HA? Trying to work that out without success, or is that something that can only be done at the integration/python level and not yaml / template sensors level?

Bescribe your Huawei Solar Setup

Inverter Type: SUN2000-5KTL-L1 (Qty 2) Inverter Firmware version: V200R001C00SPC138 SDongle present: yes - V200R022C10SPC108 Power meter present: single phase Battery: LUNA2000 10kWh Battery Firmware version: V100R002C00SPC125

How do you connect to the inverter?

Via the SDongle, wireless connection

Upload your Diagnostics File

N/A.

Upload your relevant debug logs

N/A.

Please confirm the following:

wlcrs commented 2 months ago

I've searched for all the occurences of "temperature" in the code base, and found 3 occurrences:

I also cannot reproduce it on my own development or production HA instance. So I'm a bit confused about what you are seeing there. What do you see when you enable it? If possible also include a screenshot from the developer console with state_class etc to help me narrow it down.

wrt. 'sensor.battery_1_none' and 'sensor.battery_1_none_2' : those were missing translation strings. I've fixed it in the latest commit.

wrt disabling entities: this is a feature of HA where I pass entity_registry_enabled_default=False to the sensors that I deem to be uninteresting.

Roving-Ronin commented 2 months ago

Sorry my bad, I'd purged / deleted etc and somehow that Temp 2 was still appearing, hence thought it was the integration.

image image

Being battery related, should these also be energy_storage, not energy?

Also as the total_discharge and total_charge are total that is ever increasing (aka lifetime figures), should they be total_increasing, as opposed to total ?

https://developers.home-assistant.io/docs/core/entity/sensor/#available-state-classes

"I've fixed it in the latest commit." as in the next release 1.4.0a4 ?

"entity_registry_enabled_default=False" ... thanks for that, bugger looks like .py code level, not .yaml type code I can understand/do.

wlcrs commented 2 months ago

Before doing a new release I did some additional testing, and I got the following warnings in the logs:

Entity sensor.batteries_total_charge (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'total' which is impossible considering device class ('energy_storage') it is using; expected None or one of 'measurement'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues
Entity sensor.batteries_total_discharge (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'total' which is impossible considering device class ('energy_storage') it is using; expected None or one of 'measurement'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues
Entity sensor.batteries_day_charge (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'total_increasing' which is impossible considering device class ('energy_storage') it is using; expected None or one of 'measurement'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues
Entity sensor.batteries_day_discharge (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'total_increasing' which is impossible considering device class ('energy_storage') it is using; expected None or one of 'measurement'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues

I will revert them to the combination total/energy as that seems the most correct after thinking and rereading the docs 10 times. I also looked into the powerwall-integration which is accepted in HA core, and they do the same there.

This will eventually land in 1.4.0a4.

Roving-Ronin commented 1 month ago

@wlcrs apologies, but tagging you won't see this I guess, as its closed.

just looking through logs on 1.4.0a3 and seeing these:

2024-05-08 22:51:35.920 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.battery_1_energy (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'measurement' which is impossible considering device class ('energy') it is using; expected None or one of 'total', 'total_increasing'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues

2024-05-08 22:51:35.921 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.battery_1_energy_2 (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'measurement' which is impossible considering device class ('energy') it is using; expected None or one of 'total', 'total_increasing'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues

2024-05-08 22:51:35.922 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.battery_1_total_charge (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'total' which is impossible considering device class ('energy_storage') it is using; expected None or one of 'measurement'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues

2024-05-08 22:51:35.922 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.battery_1_total_discharge (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'total' which is impossible considering device class ('energy_storage') it is using; expected None or one of 'measurement'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues

2024-05-08 22:51:35.922 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.battery_1_charge_discharge_power (<class 'custom_components.huawei_solar.sensor.HuaweiSolarSensorEntity'>) is using state class 'measurement' which is impossible considering device class ('energy') it is using; expected None or one of 'total', 'total_increasing'; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/wlcrs/huawei_solar/issues

Roving-Ronin commented 1 month ago

Any update on the release of v1.4.0a4 .... just asking for testing these sensors again ?