SH10RS accuracy fixes: signed/wide register decoding, PVOutput battery fields, plain Modbus fallback - #109
Open
itsgottabered wants to merge 4 commits into
Open
Conversation
SungrowModbusTcpClient's connect() handshake (probing for WiNet-S AES encryption, then closing and reopening a second TCP connection) causes rapid connection churn that some SH-series inverters can't tolerate - the connection gets reset on the next real request. Confirmed by reproducing the reset with plain back-to-back TCP connects alone, with no Sungrow-specific code involved. Adds an opt-in config flag to fall back to the plain ModbusTcpClient for inverters that don't need (or can't tolerate) the encrypted transport handshake.
load_registers() previously only understood plain U16 values (with an optional divide-by-N scale suffix, or a separate overflow/indicator pair for signed magnitudes). Many Sungrow registers are documented as true 32-bit U32/S32 values spanning two consecutive registers, or signed 16-bit S16 values - reading these as plain U16 silently wraps (U32 read as a single register loses data above 6553.5 of whatever its scale is) or misreads negative values as huge positive ones. Adds '_u32'/'_s32'/'_s16' name suffixes (composable with the existing '_N' scale suffix, e.g. 'total_energy_u32_10') that combine the low and high word per Sungrow's documented little-endian-word-order convention and apply correct sign conversion. Also unifies the previously-duplicated read/holding decode logic into one decode_register() helper so holding registers get the same overflow/scale/widetype handling as read registers - previously holding registers were always stored raw with no scaling at all. Also adds the b1-b6 (Battery Power/SOC/Size/Lifetime Charge/Lifetime Discharge/State) parameters to PVOutputPublisher, matching PVOutput's Add Status API - previously only v1-v12 were supported.
…l doc Cross-checked every named register in the SH10RS modmap against Sungrow's "Communication Protocol of Residential Hybrid Inverter" V1.1.4, and corrected against live readings where the doc and this specific unit's firmware disagreed. Verified end-to-end against a running installation, including live forced-charge/discharge tests to confirm sign conventions. Notable fixes: - running_state and power_flow_status were swapped (13001 was labeled "running_state" but is actually power_flow_status; true running_state at 13000 wasn't being read at all). Verified live: 13000 decodes to a real Appendix 4.1 state code (0x8200 "Dispatch Running"); the old 13001 mapping decoded to a plausible Appendix 4.2 power-flow bitmask instead of a running-state code. - power_factor was misaddressed at 5034 (that's actually reactive_power's high word) and read unsigned; moved to the correct 5035 with sign conversion. - battery_capacity was addressed at 13038, which is actually the high word of total_import_energy's 32-bit pair; moved to the correct 13039. - Nine fields upgraded from single-register reads to proper U32/S32 pairs using the new decode support (total_pv_power, reactive_power, load_power, export_power, and five lifetime energy counters), which previously wrapped silently above 6553.5 of their scaled unit. - internal_temp, battery_temp, and grid_current_a/b/c (previously mislabeled backup_current_a/b/c - that's a different, unmapped register block) now correctly signed. - self_consumption_today was missing its x0.1 scale entirely. - Renamed several energy counters to reflect their true (often narrower, PV-only) scope per the doc, and repointed the PVOutput "Battery Lifetime Charge" mapping at the general/all-source lifetime register instead of the PV-only one it was silently using before. - Added battery_power_wide (5214-5215, signed S32) as a working alternative to battery_power (13022, confirmed unsigned per the doc and unusable for charge/discharge direction), plus a battery_power_pvoutput derived field in solariot.py that flips its sign to match PVOutput's b1 convention (positive=charge) - the device's native convention is the opposite. Some registers the doc marks "reserved" (5006, 5031, 5038, 5091, 5146, 5147) return real data on this hardware and are kept as empirical findings rather than removed. Conversely, one documented register (daily_import_energy, 13036) is confirmed non-functional on this unit - reads 0 despite verified real import, cross-checked against both the Sungrow app and a third-party meter - and is left fixed but honestly non-functional rather than papered over.
This diagnostic "scan everything" map only had generic reg_NNNNN_scan
placeholder names, even for registers the official Sungrow protocol
doc ("Communication Protocol of Residential Hybrid Inverter" V1.1.4)
fully documents - defeating much of the point of a debug dump. Named
every register the doc covers (with correct scale/sign/U32-S32-S16
type per the new decode support), while keeping the generic
placeholder for anything undocumented so the tool's original
"discover registers the doc doesn't mention" purpose still works -
the SH10RS is known to respond with real data on several such
registers (see modbus-sungrow-sh10rs.py's empirical findings).
Also extended the scanned ranges to cover the full addresses the doc
documents (previously stopped short of several documented blocks,
including the 13000 running_state register itself), except one
100-register block (communication address 5250, entirely inside an
undocumented gap) that throws a Modbus exception on this specific
hardware and would otherwise abort the whole scrape_inverter() cycle
on the first failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four related fixes/additions developed and validated against a live SH10RS installation, cross-checked against Sungrow's official "Communication Protocol of Residential Hybrid Inverter" V1.1.4 doc:
force_plain_modbusconfig option -SungrowModbusTcpClient's connect handshake (WiNet-S encryption probe, then close+reopen a second TCP connection) causes rapid connection churn that some SH-series units reset on the next real request. Reproduced the reset with plain back-to-back TCP connects alone, no Sungrow-specific code involved. This flag opts an inverter out of the wrapper entirely for units that don't need (or can't tolerate) it.U32/S32/S16 register decoding + PVOutput battery fields (b1-b6) -
load_registers()previously only understood U16 (optionally scaled, or a separate overflow/indicator pair for signed magnitudes). Adds_u32/_s32/_s16name suffixes (composable with the existing_Nscale suffix) that combine register pairs per Sungrow's documented little-endian word order and apply correct sign conversion. Also unifies the previously-duplicated read/holding decode paths into onedecode_register()helper, so holding registers get the same scaling/overflow handling as read registers (previously always stored raw). Also adds PVOutput'sb1-b6(Battery Power/SOC/Size/Lifetime Charge/Lifetime Discharge/State) parameters toPVOutputPublisher, previously onlyv1-v12were supported.modbus-sungrow-sh10rs.pyaccuracy overhaul - corrected every named register against the official doc, verified live (including forced-charge/discharge tests to nail down sign conventions). Highlights:running_state/power_flow_statuswere swapped (13001 was labeledrunning_statebut is actuallypower_flow_status; truerunning_stateat 13000 wasn't read at all).power_factorwas misaddressed at 5034 (that'sreactive_power's high word) and read unsigned.battery_capacitywas addressed at 13038, actually the high word oftotal_import_energy's 32-bit pair.battery_power_wide(5214-5215, signed S32) as a working signed alternative tobattery_power(13022, confirmed unsigned per the doc), plus abattery_power_pvoutputderived field that flips its sign to match PVOutput's convention (the device's native sign is the opposite).daily_import_energy(13036) is confirmed non-functional on this unit - reads 0 despite verified real import (cross-checked against the Sungrow app and a third-party meter) - fixed but left honestly non-functional rather than papered over.modbus-sungrow-scanall.pyrebuild - names every register the official doc covers (previously all genericreg_NNNNN_scanplaceholders), while keeping placeholders for undocumented addresses so the tool's original "discover what the doc doesn't mention" purpose still works. Extended scanned ranges to cover the full documented address space, except one block that throws a Modbus exception on this specific hardware (would otherwise abort the whole scrape cycle).Test plan
-vvoutput showing clean scrapes with no warnings/exceptionsbattery_power_wide/battery_power_pvoutput) verified against both a live forced charge and a live forced discharge, cross-checked againstpower_flow_status's charging/discharging bitswinet-extractor/websocket capture for battery voltage/current/temp/SOC/SOH/lifetime charge/dischargeNote: several corrections (particularly the "reserved-but-returns-data" and "documented-but-non-functional" findings) were validated against one specific SH10RS unit/firmware revision - worth keeping an eye out for reports of different behavior on other units.