weltmeyer / ha_sonnenbatterie

Homeassistant integration to show many stats of Sonnenbatterie
GNU General Public License v3.0
51 stars 24 forks source link

PPV 1 readings are incorrect #28

Closed xlibbyx closed 1 year ago

xlibbyx commented 1 year ago

I'm not sure if this an issue with my system or an issue with the HA integration, so apologies if it's something up with my battery.

Ever since January 26th the readings for PPV1 have been incorrect. Some reading shows when the sun is down, and when the sun is up the reading is too high, plus at all times the reading looks 'noisy'. However, the readings for PPV2 look absolutely fine. Was there an update or something that changed around Jan 26th that could do this?

image

TobyRh commented 1 year ago

I've done some digging, as I was wondering why my Sonnen dashboard was only showing the data of PPV1.

As far as I can tell (with my system) the ppv value from the battery_system call is not the same as the ppv value from the inverter. The ppv from battery_system is the total wattage after having gone through the inverter. There is no ppv2 in battery_system.

I've done some reads from the battery_system and inverter calls and get the following (full dump here):

battery_system:
    'ppv': '416.47'

inverter:
    'ppv': '222.86', 
    'ppv2': '221.64'

Using the logic from sensor.py it then finds ppv1-values from battery_system and ppv2-values from the inverter, leading to the error of ppv1 being too high.

if not "inverter_ppv" in self.disabledSensors:
    val_found = True
    if 'ppv' in battery_system['grid_information']:
        val=battery_system['grid_information']['ppv']
    elif 'ppv' in inverter['status']:
        val=inverter['status']['ppv']

and

if not "inverter_ppv2" in self.disabledSensors:
    val_found = True
    if 'ppv2' in battery_system['grid_information']:
        val=battery_system['grid_information']['ppv2']
    elif 'ppv2' in inverter['status']:
        val=inverter['status']['ppv2']

My system has only been up and running for two weeks, so I don't know if something has changed in the API that has caused it, but in the release notes of Software version: 1.8.7, it states:

API V2: New data points for requesting battery module data and inverter data were additionally implemented. Release Notes

Maybe that has something to do with it?

TobyRh commented 1 year ago

image

This is what my history graph look like this morning on a cloudy/rainy day.

xlibbyx commented 1 year ago

I switched the if and elseif in sensor.py last night and everything looks normal today so that seems to have fixed it.

markup_1000003278

TobyRh commented 1 year ago

I did it a bit differently. I created entities for the battery system values (ppv, ipv, upv) and seperated them out from the inverter entities. pastebin

RustyDust commented 1 year ago

@TobyRh @xlibbyx

Would you guys be so kind and send a dump of your setup using this gist -> https://gist.github.com/RustyDust/2dfdd9e9d0f3b5476b5e466203123f6f

(don't forget to edit any possibly sensible data)

TobyRh commented 1 year ago

@RustyDust

Here you go 👍

RustyDust commented 1 year ago

Here you go 👍

Thanks a lot! It's really fascinating how many differences there are between setups.

TobyRh commented 1 year ago

Thanks a lot! It's really fascinating how many differences there are between setups.

Definitely! I mean you have almost double the amount of lines in your output! And the content is night and day a lot of the time

xlibbyx commented 1 year ago

Thanks for this! 👍🏻

RustyDust commented 1 year ago

@TobyRh

Would you mind looking at this and check for needed changes in mappings.py? ;)

TobyRh commented 1 year ago

Had a look. Don't really know what I'm looking for, but I did wonder why fac is mentioned twice in the inverter section. I don't have it under status --> status.

  "inverter": {
    "status": {
      "status": {
        "fac": {
          "sensor":         "inverter_fac",
          "friendly_name":  "Inverter - Net frequency",
          "unit":           "Hz",
          "class":          SensorDeviceClass.FREQUENCY,
          "aka":            ["state_netfrequency"],
          "convert":        float,
        },
      },
      "fac": {
        "sensor":         "inverter_fac",
        "friendly_name":  "Inverter - Net frequency",
        "unit":           "Hz",
        "class":          SensorDeviceClass.FREQUENCY,
        "aka":            ["state_netfrequency"],
        "convert":        float,
RustyDust commented 1 year ago

My system doesn't have it either but I've got dumps where it's there so that's why it's included.