perf(marstek): derive the contiguous-block table, and repair the v3 packet correction - #500
Merged
Merged
Conversation
Block reads (ffunes#361) came with a table somebody wrote out: three spans for v3/v2, plus the per-pack 34000 spans for vA/vD (ffunes#439). The rule behind it - group registers that are already adjacent and share a cadence - holds for every register the driver polls, not just the ones that were looked at, and applying it to all of them finds more. _derive_register_blocks builds the same structure from the version's own entity definitions. On a Venus D that is 15 spans instead of 10, and the two it adds are ones that were costing a request each every cycle: the four MPPT powers at 30037 (four reads at "high") and the lifetime energy counters at 33000. Per poll the request count drops by about a fifth. The safety property the table was careful about is kept, and is now a property of the code rather than of the person editing it: a span never crosses a gap, so an unmapped address cannot be pulled in. What was special-cased stays correct on its own - a v3's definitions do not carry the 34000 registers, so no v3 block is derived for them. _load_register_blocks goes; the tables it read stay in const/ and the new tests check the derivation still covers every span they declare.
…lient
_marstek_v3_packet_correction repairs the MBAP length field in the v3
family's exception frames, which the firmware sets to 4 where the
protocol wants 3. It was installed with
self.client.trace_packet = _marstek_v3_packet_correction
after the client was built. pymodbus takes trace_packet as a constructor
argument and passes it to its TransactionManager, which is what calls
the hook; assigning it afterwards adds an attribute to the client that
nothing reads, and the manager keeps its own dummy_trace_packet. So the
correction has been installed on every v3 connection and called on none
of them.
Measured against a fake battery that sends the frame the Venus D sends:
a read of an unmapped register took 9.02 s (timeout x internal retries)
and now takes under 10 ms. Both shapes are covered, the nine bytes in
one segment and split over two, because the hook sees the accumulated
buffer.
The test that was meant to cover this asserted client.trace_packet is
not None - true either way, since the attribute assignment succeeds. It
now looks where pymodbus reads the hook, and the new end-to-end test
puts a real read over a real socket, which is the only thing that can
tell an installed hook from a called one.
Deriving the block table pairs 34002 (pack 1 SOC) with 34003 (cycle count): adjacent, same cadence, one request instead of two. But a pack slot that never answers leaves the read groups, and a group leaves as a whole - so on a battery whose first slot stays unconfirmed the cycle count would leave with it and never be read again. Blocks are now partitioned by probe family as well as by scan interval: a pack SOC register groups only with pack SOC registers, a pack cell register only with pack cell registers. The per-pack spans the table declared by hand are pure either way, so nothing that existed is lost; what goes is one derived pair that was never safe to make.
Open
4 tasks
ffunes
added a commit
that referenced
this pull request
Sep 22, 2026
perf(marstek): derive the contiguous-block table, and repair the v3 packet correction Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> # Conflicts: # CHANGELOG.md
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
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.
Follows #361. Two independent changes to the Marstek read path, in separate commits, both measured on a Venus D (EMS v150, seven packs).
1. The block table is derived instead of written by hand
Block reads shipped with a table somebody wrote out: three spans for v3/v2, plus the per-pack spans for vA/vD (#439). The rule behind that table — group registers that are already adjacent and share a cadence — holds for every register the driver polls, not only the ones that were looked at. Applied to all of them it finds more:
The two the Venus D was missing are not small ones: the four MPPT powers sit at 30037–30040 and were four separate requests on the fast schedule, and the lifetime energy counters at 33000 were two more.
Requests per pass, measured against the battery:
The safety property the table was careful about is now a property of the code rather than of whoever edits it: a span never crosses a gap, so an unmapped address cannot be pulled into a block. Two things that the hand-written table had to state, and that the derivation settles by itself:
Verified on hardware: every derived span answers, and each member decodes to what a single read of the same register gives immediately before and after the block read. Cell voltages move by a millivolt between the three requests, which is why the check brackets rather than compares.
_load_register_blocksis gone. The tables it read stay inconst/, and the new tests assert that the derivation still covers every span they declare, so the change can only add batching, never remove it.2. The v3 packet correction was installed on every connection and called on none
_marstek_v3_packet_correctionrepairs the MBAP length field in the v3 family's exception frames. The firmware answers a read of an unimplemented register with nine bytes whose length field says 4 where the protocol requires 3:pymodbus frames that as
7 + (length - 1)and waits for a tenth byte the device never sends, so the read times out against a battery that answered in about 100 ms.The hook itself is correct. It was attached like this:
pymodbus takes
trace_packetas a constructor argument and hands it to itsTransactionManager, which is what calls it. Assigning it afterwards adds an attribute to the client object that nothing reads, and the manager keeps its owndummy_trace_packet. Checked on pymodbus 3.13.1:Measured against a fake battery that sends exactly the frame above, a read of an unimplemented register:
Both delivery shapes are covered — the nine bytes in one segment and split across two writes — because the hook sees the accumulated buffer, not a single segment.
The existing test could not catch this: it asserted
tcp.client.trace_packet is not None, which is true either way, since the assignment succeeds. It now looks where pymodbus reads the hook, and a new end-to-end test drives a real read over a real socket, which is the only thing that can tell an installed hook from a called one. Without the fix, two of its three cases fail.Anyone reaching the battery through a Modbus proxy will not have noticed this: the proxy parses the frame itself and hands on a correct one.
Deliberately not done: bridging small gaps
The obvious next step is to let a span cross a few unused addresses. I measured it on the Venus D rather than reasoning about it, and it does not pay:
With a gap of 10 the span at 42010+12 is refused by the firmware — and that is the span carrying
force_mode,charge_to_soc,set_charge_powerandset_discharge_power. Since a failed block drops its members from the snapshot with no per-register fallback, the two requests gap 10 saves on the fast schedule are exactly the ones whose answers disappear. Gap 2 is clean but buys nothing where the load actually is.Tests
2664 passed, 10 skipped, plus the eleven
hass-fixture tests the workflow runs separately. New:tests/test_marstek_derived_blocks.py(the derivation rule, coverage of both hand-written tables, the two spans it adds, the probe-family split) andtests/test_v3_exception_frame.py(the rejection end to end, both segment shapes, and the counter-case that pins why the correction exists).The two commits are independent; either can be taken on its own.