-
Notifications
You must be signed in to change notification settings - Fork 107
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
Comments
Cannot test this (do not have any backup stuff working here). Can you make a PR? |
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. |
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. |
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: ***@***.***>
|
merged your PR in #361 |
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 |
@dylan09 raf |
Sadly, the firmware updates do not provide a list of changes. :-(
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 Or:
|
Apart from figuring out the version number: |
@Gnarfoz |
Register definition:
With this I get values in the millions W. E.g. 2.490.368,0 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. |
And here the installed firmware versions:
|
Are you running your inverter as an island (not connected to the grid)? |
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 this register should now read as:
It works at my end and for users I have worked with. hope that helps. Raf |
@Gnarfoz Grid integrated. But parts of the circuits inside the house are always connected to the backup. @RafAustralia |
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... |
|
fixed it in aef78cf :) |
reverted the commit due to #386 |
This issue is stale because it has been open for 30 days with no activity. |
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
Sungrow-SHx-Inverter-Modbus-Home-Assistant/modbus_sungrow.yaml
Line 337 in 9e9859d
The text was updated successfully, but these errors were encountered: