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

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

Open
RafAustralia opened this issue Oct 1, 2024 · 21 comments
Labels

Comments

@RafAustralia
Copy link
Contributor

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

image

@mkaiser
Copy link
Owner

mkaiser commented Oct 1, 2024

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

@RafAustralia
Copy link
Contributor Author

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
Copy link
Collaborator

Gnarfoz commented Oct 2, 2024

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
Copy link
Contributor Author

RafAustralia commented Oct 2, 2024 via email

@mkaiser
Copy link
Owner

mkaiser commented Oct 12, 2024

merged your PR in #361

@mkaiser mkaiser closed this as completed Oct 12, 2024
@dylan09
Copy link
Contributor

dylan09 commented Oct 18, 2024

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
Copy link
Contributor Author

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

raf

@Gnarfoz
Copy link
Collaborator

Gnarfoz commented Oct 18, 2024

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
Copy link
Collaborator

Gnarfoz commented Oct 18, 2024

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
Copy link
Contributor Author

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

@dylan09
Copy link
Contributor

dylan09 commented Oct 18, 2024

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
Copy link
Contributor

dylan09 commented Oct 18, 2024

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
Copy link
Collaborator

Gnarfoz commented Oct 18, 2024

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
Copy link
Contributor Author

@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

RafAustralia#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
Copy link
Contributor

dylan09 commented Oct 19, 2024

@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
Copy link
Collaborator

Gnarfoz commented Oct 19, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

dylan09 commented Oct 22, 2024

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

@mkaiser
Copy link
Owner

mkaiser commented Oct 22, 2024

fixed it in aef78cf

:)

@mkaiser
Copy link
Owner

mkaiser commented Oct 22, 2024

reverted the commit due to #386

Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants