huawei_solar icon indicating copy to clipboard operation
huawei_solar copied to clipboard

[Bug]: Double battery device since 1.4.0 alpha 1

Open hvorragend opened this issue 4 months ago • 8 comments

Describe the issue

Since the first alpha (and also in alpha2) I have my battery device twice.

All devices: image

Device: Batteriespeicher image

New battery device since alpha version: The entities in the sensor area were absolutely identical. I have currently deactivated them. But you can see that there are no select entities in the configuration area.

image

Bescribe your Huawei Solar Setup

Inverter Type: SUN2000-8KTL-M1 Inverter Firmware version: V100R001C00SPC159 SDongle present: yes Power meter present: three phase Battery: LUNA 2000 2x 5 kWh Battery Firmware version: V100R002C00SPC127

How do you connect to the inverter?

Via the SDongle, wired connection

Upload your Diagnostics File

config_entry-huawei_solar-421c4f96daf35cfb9816aefa6bc40b59.json

Upload your relevant debug logs

There are no relevant log lines.

Please confirm the following:

  • [X] I'm running the latest release of Home Assistant.
  • [X] I'm running the latest release of this integration.
  • [X] I did not find an existing issue describing this problem.
  • [X] I did upload the diagnostics-file that I could retrieve from the 'Devices & Services Page'
  • [X] I did upload the relevant debug logs (via 'Enable Debug Logging'-feature or by manually configuring HA logging)

hvorragend avatar Apr 05 '24 13:04 hvorragend

I agree that many of those entities are a duplicate of those that were already included in the integration before 1.4.0 . This is why they were absent in earlier versions, and why I have disabled them by default. However, they can be of interest for people who have 2 batteries and want to monitor them separately. This new battery device now also makes it possible to monitor battery temperature, which is an often requested feature (#631, #541, #74)

Thanks to recent improvements of Home Assistant, there is no performance penalty for having entities which are disabled.

wlcrs avatar Apr 05 '24 18:04 wlcrs

But almost all new entities in the new battery device are redundant and already exist in the first battery device.

Or can you even take it to the extreme?

  • One device for the battery control unit
  • One device per battery pack (2 in my case)

hvorragend avatar Apr 05 '24 18:04 hvorragend

Many of them are indeed redundant in the installations containing only one battery. However, they are potentially of interest for people owning two batteries.

This is the endless battle of trying to balance the feature set of this integration with the perceived complexity. And I have the impression that there will always be complaints 🙃

As already said: I have disabled all of these duplicate entities by default. As long as they are disabled there is no performance impact.

I do note that I will need to clearly document why those "duplicate" sensors exist, as most installations only have one battery installed.

wlcrs avatar Apr 05 '24 18:04 wlcrs

Thanks for the explanation. I just wanted to help.

But please let me ask you one more thing:

What do you mean by "one battery"?

I have a Luna control unit with 2x5kWh battery packs.

Is that "one battery" for you then?

The FusionSolar app also differentiates:

ESU No.1

  • ControlUnit with the various daily and total values
  • Battery pack 1 with e.g. temperature
  • Battery pack 2 with e.g. temperature

hvorragend avatar Apr 05 '24 19:04 hvorragend

A SUN2000 inverter can be connected to 2 LUNA2000 batteries, each having up to 3 battery packs.

wlcrs avatar Apr 05 '24 19:04 wlcrs

HI, I understood the purpose of the change, but honestly I would think it would be clearer if the "Batteries" device had associated only the common entities, if there were any, to all the batteries installed, whether one or two, while the "Battery 1" and , where present, "Battery 2" had associated the specific entities of these batteries. In any case, the device "Battery 1" has associated entities "sensor.battery_1_none", "sensor.battery_1_none_2", "sensor.battery_1_energy" and "sensor.battery_1_energy_2" which I would ask you to kindly explain. Thank you.

Ivano62 avatar Apr 07 '24 21:04 Ivano62

@Ivano62 what language is set in your HA? This issue seems to be caused by missing translations.

wlcrs avatar Apr 08 '24 05:04 wlcrs

English.

Thanks

--- Messaggio originale --- Da: Thijs W. @.*** Data: 8 aprile 2024 07:36:07 A: wlcrs/huawei_solar @.*** Cc: Ivano62 @., Mention @. Oggetto: Re: [wlcrs/huawei_solar] [Bug]: Double battery device since 1.4.0 alpha 1 (Issue #673)

@Ivano62https://github.com/Ivano62 what language is set in your HA? This issue seems to be caused by missing translations.

— Reply to this email directly, view it on GitHubhttps://github.com/wlcrs/huawei_solar/issues/673#issuecomment-2041900210, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKIRD3OCPRCFBBMVQMJZAZLY4IUEBAVCNFSM6AAAAABFZGNIZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRHEYDAMRRGA. You are receiving this because you were mentioned.Message ID: @.***>

Ivano62 avatar Apr 08 '24 06:04 Ivano62

Hi Thijs,

Could you please confirm how the 'Batteries' sensor actually calculates the output for its sensors? For example are they: a) the average reading for all batteries and sensors like: Bus Current, Bus Voltage, Charge/Discharge Power, SoC %. b) the sum total of all battery for sensors like: Day Charge, Day Discharge, Total Charge, Total Discharge.

Meaning we can't dispense with needing to create a whole bunch of template sensors to combine battery sensors, like:

  # Provides the combined avaeraged State of Capacity both batteries, as a %.
  - sensor:
    - name: "Batteries - State of Capacity"
      unique_id: batteries_state_of_capacity
      unit_of_measurement: "%"
      device_class: "battery"
      state_class: measurement
      state: >-
        {% set battery_1_soc = states('sensor.battery_state_of_capacity') |float(default=0) %}
        {% set battery_2_soc = states('sensor.battery_state_of_capacity_2') |float(default=0) %}

        {% if states('sensor.battery_state_of_capacity_2') == 'unknown' %}
          {{ battery_1_soc }}
        {% else %}
          {% set count = 0 %}
          {% set sum = 0 %}

          {% if states('sensor.battery_state_of_capacity') != 'unknown' %}
            {% set sum = sum + battery_1_soc %}
            {% set count = count + 1 %}
          {% endif %}

          {% if states('sensor.battery_state_of_capacity_2') != 'unknown' %}
            {% set sum = sum + battery_2_soc %}
            {% set count = count + 1 %}
          {% endif %}

          {% if count > 0 %}
            {{ (sum / count) }}
          {% else %}
            N/A
          {% endif %}
        {% endif %}
      availability: >-
        {{ states('sensor.battery_state_of_capacity')   | is_number
        or states('sensor.battery_state_of_capacity_2') | is_number }}

Or a combined battery_temperature that shows the higher temperature of any connected battery, like:

  - sensor:
    - name: "Batteries - Temperature"
      unique_id: batteries_temperature
      unit_of_measurement: "°C"
      device_class: "temperature"      
      state: >-
        {% set battery_temp_1 = states('sensor.battery_battery_1_temperature') | float(default=0) %}
        {% set battery_temp_2 = states('sensor.battery_battery_2_temperature') | float(default=0) %}
        {{ max(battery_temp_1, battery_temp_2) | round(1) }}
      availability: >-
        {{ states('sensor.battery_battery_1_temperature') != 'unavailable'
        or states('sensor.battery_battery_2_temperature') != 'unavailable' }}

Then we'd just need an 'Inverters' sensor also to be added, for those running multiple inverters and needing a single sensor for each of the inverterS readings ;-)

BTW: The friendly name is "Batteries", but the sensor names are prefixed by 'battery_' not 'batteries_' ?

"A SUN2000 inverter can be connected to 2 LUNA2000 batteries, each having up to 3 battery packs." Just wait for the LUNA S1 7/14/21kWh, that supports up to 4 in parallel per inverter, so that will also mean up to 12 battery packs in total. ;-)

Roving-Ronin avatar Apr 17 '24 13:04 Roving-Ronin

All numbers are retrieved from the inverter and passed to HA without any additional computation. What they precisely mean is something that you have to ask Huawei. I don't know anything more than the modbus PDF's tell me.

wrt sensor ids: those are automatically generated by HA the first time an integration is added. If the naming evolves over time, this ID does not change with it. HA even keeps track of them if you remove and re-install the integration. Users that install v1.4.0 on a HA that has not seen any older versions of this integration, the sensor id will be sensor.batteries_... .

Hadn't seen the LUNA S1 announcement yet. Luckily I did code everything quite nicely so that scaling up the number of battery sensors can be done with just a few lines of code. I will need somebody to provide me with the updated modbus registers PDF when that becomes available.

wlcrs avatar Apr 17 '24 18:04 wlcrs