mkaiser / Sungrow-SHx-Inverter-Modbus-Home-Assistant

Sungrow SH Integration for Home Assistant for SH3K6, SH4K6, SH5K-20, SH5K-V13, SH3K6-30, SH4K6-30, SH5K-30, SH3.RS, SH3.6RS, SH4.0RS, SH5.0RS, SH6.0RS, SH5.0RT, SH6.0RT, SH8.0RT, SH10RT, SH5.0RT-20, SH6.0RT-20, SH8.0RT-20, SH10RT-20, SH5.0RT-V112, SH6.0RT-V112, SH8.0RT-V112, SH10RT-V112, SH5.0RT-V122, SH6.0RT-V122, SH8.0RT-V122, SH10RT-V122, SH4.6R
372 stars 95 forks source link

This register int16 needs to change to int32 pas per sungrow register protocol, but remain singed power #360

Open RafAustralia opened 1 month ago

RafAustralia commented 1 month ago

The value int16 is incorrect, needs to be changed to 32 but there is a mistake in sungrow protocol which calls for it to be uint, that is a mistake, the final result should remain int32

  - name: Total backup power
    device_address: !secret sungrow_modbus_slave
    unique_id: sg_total_backup_power
    address: 5725 # reg 5726
    input_type: input
    data_type: int32 # as per sungrow register list
    precision: 1
    unit_of_measurement: W
    device_class: power
    state_class: measurement
    scale: 1
    scan_interval: 10

https://github.com/mkaiser/Sungrow-SHx-Inverter-Modbus-Home-Assistant/blob/9e9859d23a6fe735098d56de8ae2eb7bc727942f/modbus_sungrow.yaml#L337

image

mkaiser commented 1 month ago

Cannot test this (do not have any backup stuff working here). Can you make a PR?

RafAustralia commented 1 month ago

I will do my best to get to make a PR:

Logic here is - if you see sensors above, 5724, 5725 etc. they are all to do with backup power and they are all signed. Total Backup power should be also signed - pretty sure Sungrow confirmed it was a bad translation error.

Gnarfoz commented 1 month ago

This correction is also slated to be in the next Sungrow modbus register specification (1.1.5), according to the Sungrow Germany product management team. I've requested the new version of the document but have not received it, yet. 😢

RafAustralia commented 1 month ago

Yup.

This error was shown to their r&d team live at my house some 3-4months back. Lol. Still have notes. Cos we have gone through the protocol page by page and they were a touch surprised anyone would pick them to pieces. We also discussed how chaotic their translation and application of modicon convention was but thats a whole different story.

I do have a particular issue Im trying to solve or improve and I would not mind running past u, so discord would b a perfect place to start. If u have any time left for discussions.

Message ID: @.***>

mkaiser commented 1 month ago

merged your PR in #361

dylan09 commented 1 month ago

I have testet this change with my older inverter firmware. Don't know the currently installed version. But it should be anything from the beginning of 2022.

And there this register is only 16bit. Changing the datatype to int32 gives wrong results. So maybe the register type depends on the inverter firmware installed.

RafAustralia commented 1 month ago

@dylan09 Please post your register code. I will have a look. There will be a simple answer

raf

Gnarfoz commented 1 month ago

Sadly, the firmware updates do not provide a list of changes. :-( If you have iSolarCloud with an installer account (easy to set up), you can look up the firmware version there. Or, if you have mbpoll, you can read it from the inverter:

# mbpoll 192.168.0.63 -1 -t 3:hex -r 2582 -c 44
mbpoll 1.0-0 - FieldTalk(tm) Modbus(R) Master Simulator
Copyright © 2015-2019 Pascal JEAN, https://github.com/epsilonrt/mbpoll
This program comes with ABSOLUTELY NO WARRANTY.
This is free software, and you are welcome to redistribute it
under certain conditions; type 'mbpoll -w' for details.

Protocol configuration: Modbus TCP
Slave configuration...: address = [1]
                        start reference = 2582, count = 44
Communication.........: 192.168.0.63, port 502, t/o 1.00 s, poll rate 1000 ms
Data type.............: 16-bit register, input register table

-- Polling slave 1...
[2582]:         0x5341
[2583]:         0x5050
[2584]:         0x4849
[2585]:         0x5245
[2586]:         0x2D48
[2587]:         0x5F30
[2588]:         0x3130
[2589]:         0x3131
[2590]:         0x2E39
[2591]:         0x352E
[2592]:         0x3031
[2593]:         0x0000
[2594]:         0x0000
[2595]:         0x0000
[2596]:         0x0000
[2597]:         0x5341
[2598]:         0x5050
[2599]:         0x4849
[2600]:         0x5245
[2601]:         0x2D48
[2602]:         0x5F30
[2603]:         0x3330
[2604]:         0x3131
[2605]:         0x2E39
[2606]:         0x352E
[2607]:         0x3031
[2608]:         0x0000
[2609]:         0x0000
[2610]:         0x0000
[2611]:         0x0000
[2612]:         0x5355
[2613]:         0x4243
[2614]:         0x544C
[2615]:         0x2D53
[2616]:         0x5F30
[2617]:         0x3430
[2618]:         0x3131
[2619]:         0x2E30
[2620]:         0x312E
[2621]:         0x3031
[2622]:         0x0000
[2623]:         0x0000
[2624]:         0x0000
[2625]:         0x0000

From that, grab just the byte values after the 0x, for each line, and dump then into something like this: https://dencode.com/en/string/hex And you will get the version information. :-)

Or:

# echo '5341 5050 4849 5245 2D48 5F30 3130 3131 2E39 352E 3031 0000 0000 0000 0000 5341 5050 4849 5245 2D48 5F30 3330 3131 2E39 352E 3031 0000 0000 0000 0000 5355 4243 544C 2D53 5F30 3430 3131 2E30 312E 3031 0000 0000 0000 0000' | xxd -r -p
SAPPHIRE-H_01011.95.01SAPPHIRE-H_03011.95.01SUBCTL-S_04011.01.01
Gnarfoz commented 1 month ago

Apart from figuring out the version number: How did you test this? Which value did you expect, and which value did you get instead?

RafAustralia commented 1 month ago

@gnarfoz I think he needs to post the code... I will spot the difference....

dylan09 commented 1 month ago

Register definition:

    - name: sg_backup_power
      unique_id: 2483fab3-d152-458c-83ff-4ffe9793a1b9
      device_address: !secret sungrow_modbus_device_address
      address: 5725 # reg 5726
      input_type: input
      data_type: int32
      precision: 1
      unit_of_measurement: W
      device_class: power
      state_class: measurement
      scale: 1
      scan_interval: 10

With this I get values in the millions W. E.g. 2.490.368,0 W Expected is a few Watt. Currently around 30 W.

I am away from home for a few weeks. So testing is not easy for me. iSolarCloud is not connected.But will see, if I could get the version.

dylan09 commented 1 month ago

And here the installed firmware versions:

SDSP  SUBCTL-S_04011.01.01
MDSP  SAPPHIRE-H_03011.71.15
LCD   SAPPHIRE-H_01011.71.18
Gnarfoz commented 1 month ago

Are you running your inverter as an island (not connected to the grid)? Or is this value non-zero in other situations than a power outage?

RafAustralia commented 1 month ago

@dylan09 @Gnarfoz

I actually expected this to look like that.

When we are talking about int32 register, as times the data is interpreted wrongly between devices sending and receiving these, so one more line needs to be added, and that is a line "swap: word" which flips the pairs of numbers in the packet sent.

that was addressed I think just 3 days ago, and merged into main in this PR

https://github.com/RafAustralia/Sungrow-SHx-Inverter-Modbus-Home-Assistant/pull/2

this register should now read as:

    address: 5725 # reg 5726
    input_type: input
    data_type: int32
    swap: word
    precision: 0
    unit_of_measurement: W
    device_class: power

It works at my end and for users I have worked with.

hope that helps.

Raf

dylan09 commented 1 month ago

@Gnarfoz Grid integrated. But parts of the circuits inside the house are always connected to the backup.

@RafAustralia swap: word seems the right tip. And sounds good. Don't know why it doesn't come to my mind? Will check this.

Gnarfoz commented 1 month ago

I didn't know that stuff connected to the backup port gets reported as backup power even when there isn't a power outage. 👍🏻

RafAustralia commented 1 month ago

I didn't know that stuff connected to the backup port gets reported as backup power even when there isn't a power outage. 👍🏻

Actually Sungrow have told me if you do not need it - delete it... but then a lot of new SH*T inverters use it cos they connect through backup plane... so I left it in... it just sits in near zero state...

dylan09 commented 1 month ago

swap = word does it. Have changed definition and now it shows the correct value.

mkaiser commented 1 month ago

fixed it in aef78cf

:)

mkaiser commented 1 month ago

reverted the commit due to #386