The table the charger actually keeps its measurements in

Modbus mode never returned a reading: every status poll came back as "the charger did not answer", though the charger was answering all along. It was refusing the question. The A5191 splits its map across two tables where the spec's single 2xxxx column suggests one — 20000-20100 are input registers and reject FC03 with an illegal-address exception at every address in the range, while 21000-21005 really are holding registers and read back over FC03. We inferred one space from the spec's layout and asked for all of it with FC03. The client learns FC04, sharing a body with FC03 since the two differ only in which table the server consults, and the plugin's two measurement reads move to it. Writes stay on FC06, where the controls already live. Confirmed against an A5191 on firmware 1.0.6.1: identity, live block and the control registers all decode as the spec tabulates them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-01 18:42:39 +02:00
parent f025dc100f
commit cf4fd14b56
3 changed files with 79 additions and 14 deletions
@@ -9,10 +9,11 @@
 // (internal/plugins/builtin/ankersolix): read a contiguous block of holding
 // registers, and write one register to issue a command. Concretely that means:
 //
-//   - Function codes 0x03 (Read Holding Registers) and 0x06 (Write Single
-//     Register) only. The charger exposes its whole map — read-only and
-//     read-write alike — in one 2xxxx address space, which is the holding
-//     register convention.
+//   - Function codes 0x03 (Read Holding Registers), 0x04 (Read Input Registers)
+//     and 0x06 (Write Single Register) only. The charger splits its map across
+//     the first two: everything it measures or reports (20000-20100) answers on
+//     FC04 and rejects FC03 with an illegal-address exception, while the
+//     controls it accepts (21000-21005) are holding registers.
 //   - One request in flight at a time. Modbus TCP allows pipelining by
 //     transaction identifier; nothing here needs it, and a strictly synchronous
 //     exchange means a desynchronised peer cannot silently mismatch replies.
@@ -52,9 +53,10 @@ const (
 	mbapLen     = 7
 	protocolID  = 0
 	fcReadHold  = 0x03
+	fcReadInput = 0x04
 	fcWriteReg  = 0x06
 	excMask     = 0x80 // set on the echoed function code when the reply is an exception
-	maxReadRegs = 125  // §6.3: the largest quantity one FC03 request may ask for
+	maxReadRegs = 125  // §6.3/§6.4: the most registers one read request may ask for
 )

 // maxPDU bounds what this client will read back from a peer. The spec's own
@@ -163,11 +165,24 @@ func (c *Client) Close() error {
 // ReadHolding reads count consecutive holding registers starting at addr
 // (function code 0x03). Register values are returned in the order read.
 func (c *Client) ReadHolding(ctx context.Context, addr, count uint16) ([]uint16, error) {
+	return c.readRegisters(ctx, fcReadHold, addr, count)
+}
+
+// ReadInput reads count consecutive input registers starting at addr (function
+// code 0x04). It is the read the Anker charger's measurement block answers:
+// the same request shape as ReadHolding, a different table on the device.
+func (c *Client) ReadInput(ctx context.Context, addr, count uint16) ([]uint16, error) {
+	return c.readRegisters(ctx, fcReadInput, addr, count)
+}
+
+// readRegisters is the body both reads share; FC03 and FC04 differ only in
+// which table the server looks the addresses up in, not in their framing.
+func (c *Client) readRegisters(ctx context.Context, fc byte, addr, count uint16) ([]uint16, error) {
 	if count == 0 || count > maxReadRegs {
 		return nil, fmt.Errorf("modbus: cannot read %d registers in one request (1-%d)", count, maxReadRegs)
 	}
 	req := make([]byte, 5)
-	req[0] = fcReadHold
+	req[0] = fc
 	binary.BigEndian.PutUint16(req[1:], addr)
 	binary.BigEndian.PutUint16(req[3:], count)

@@ -71,8 +71,13 @@ func dial(t *testing.T, addr string) *Client {
 }

 // readReply builds a well-formed FC03 reply carrying the given register values.
-func readReply(values ...uint16) []byte {
-	out := []byte{fcReadHold, byte(len(values) * 2)}
+func readReply(values ...uint16) []byte { return replyFor(fcReadHold, values...) }
+
+// inputReply is the same for FC04, which frames its reply identically.
+func inputReply(values ...uint16) []byte { return replyFor(fcReadInput, values...) }
+
+func replyFor(fc byte, values ...uint16) []byte {
+	out := []byte{fc, byte(len(values) * 2)}
 	for _, v := range values {
 		out = binary.BigEndian.AppendUint16(out, v)
 	}
@@ -126,6 +131,47 @@ func TestReadHoldingDecodesRegisters(t *testing.T) {
 	}
 }

+func TestReadInputAsksTheInputTable(t *testing.T) {
+	addr, seen := serve(t, func(pdu []byte) []byte {
+		// A charger that keeps its measurements in input registers answers FC04
+		// and refuses FC03, so replying to the wrong code would hide a mix-up.
+		if pdu[0] != fcReadInput {
+			return []byte{pdu[0] | excMask, 0x02}
+		}
+		return inputReply(0x0102, 0x0304)
+	})
+	c := dial(t, addr)
+
+	got, err := c.ReadInput(context.Background(), 20041, 2)
+	if err != nil {
+		t.Fatalf("ReadInput: %v", err)
+	}
+	if len(got) != 2 || got[0] != 0x0102 || got[1] != 0x0304 {
+		t.Errorf("registers = %v, want [258 772]", got)
+	}
+
+	_ = c.Close()
+	frames := <-seen
+	if len(frames) != 1 {
+		t.Fatalf("server saw %d frames, want 1", len(frames))
+	}
+	if fc := frames[0][mbapLen]; fc != fcReadInput {
+		t.Errorf("function code = 0x%02x, want 0x%02x", fc, fcReadInput)
+	}
+	if a := binary.BigEndian.Uint16(frames[0][mbapLen+1:]); a != 20041 {
+		t.Errorf("address = %d, want 20041", a)
+	}
+}
+
+func TestReadInputRejectsHoldingReply(t *testing.T) {
+	addr, _ := serve(t, func(pdu []byte) []byte { return readReply(0x0102) })
+	c := dial(t, addr)
+
+	if _, err := c.ReadInput(context.Background(), 20041, 1); err == nil {
+		t.Fatal("expected an error when the reply echoes the other function code")
+	}
+}
+
 func TestReadHoldingRejectsBadCounts(t *testing.T) {
 	addr, _ := serve(t, func(pdu []byte) []byte { return readReply(0) })
 	c := dial(t, addr)
@@ -19,8 +19,12 @@ package ankersolix
 //     rather than letting a caller discover it.
 //
 // The spec tabulates addresses, types and gains but does not name the function
-// codes; the whole map lives in one 2xxxx space with RO and RW entries side by
-// side, which is the holding-register convention, so FC03/FC06 is what this uses.
+// codes. An A5191 on firmware 1.0.6.1 answers the measurement block (20000-20100)
+// on FC04 only — FC03 there is refused with an illegal-address exception, for
+// every address in the range — while the controls at 21000-21005 do read back
+// over FC03. So the map is two tables, not the one 2xxxx space it looks like:
+// input registers for what the charger reports, holding registers for what it
+// accepts, which is FC04 to read and FC06 to write.

 import (
 	"context"
@@ -76,8 +80,8 @@ const (
 	regPhaseCountSet   = 21005 // 0 automatic, 1 fixed single, 2 fixed three
 )

-// The two blocks read in one request each. Both are well inside FC03's limit of
-// 125 registers, and splitting them keeps the hot path (live state) small.
+// The two blocks read in one request each. Both are well inside the 125-register
+// limit of a read, and splitting them keeps the hot path (live state) small.
 const (
 	identityStart = regProductNumber
 	identityCount = 41 // 20000-20040
@@ -202,14 +206,14 @@ func ModbusDial(ctx context.Context, cfg ModbusConfig) (*modbus.Client, error) {
 func ModbusRead(ctx context.Context, c *modbus.Client, withIdentity bool) (ModbusSnapshot, error) {
 	var snap ModbusSnapshot

-	live, err := c.ReadHolding(ctx, liveStart, liveCount)
+	live, err := c.ReadInput(ctx, liveStart, liveCount)
 	if err != nil {
 		return snap, err
 	}
 	decodeLive(&snap, live)

 	if withIdentity {
-		ident, err := c.ReadHolding(ctx, identityStart, identityCount)
+		ident, err := c.ReadInput(ctx, identityStart, identityCount)
 		if err != nil {
 			// Identity is a nicety; live state is the point. Report what we have.
 			return snap, nil