Skip to content

Instantly share code, notes, and snippets.

@balloob

balloob/modbus-connection-review.md Secret

Last active July 2, 2026 07:03
Show Gist options

  • Select an option

    Learn more about clone URLs
  • Save balloob/b9fa91ba1a0914a9787f8f6ceb637b83 to your computer and use it in GitHub Desktop.

Select an option

Learn more about clone URLs
Save balloob/b9fa91ba1a0914a9787f8f6ceb637b83 to your computer and use it in GitHub Desktop.
Download ZIP
Evaluating modbus-connection against 40 real-world Python Modbus libraries
Raw
modbus-connection-review.md

Evaluating modbus-connection against 60 real-world Modbus libraries — and the HA Modbus YAML configs it could turn into integrations

Date: 2026-07-02 Method: Cloned 60 Python Modbus projects (three rounds of 20) into a working folder, read each one's transport, connection, decoding and device-modelling code, and asked: if this project migrated onto modbus-connection, would it fit — and what does modbus-connection assume that turns out to be wrong or missing? Each library was scored on whether it confirms or challenges the design's core assumptions. Findings flagged as "verified" were re-checked against the source directly. Round 3 added heat pumps, EV chargers, a pool controller, generic async backends, a Modbus proxy, and a MicroPython RTU library — device classes the first two rounds under-covered.

A second track explores the Home Assistant angle (modbus-connection lives under home-assistant-libs): which HA integrations speak Modbus and how they'd migrate (see the "Home Assistant integrations that use Modbus" section), and — because HA's device-Modbus ecosystem mostly lives as hand-written YAML against the generic modbus integration — which of those YAML configs could become real integrations built on modbus-connection. That's not hypothetical: it's the TROVIS precedent — a device that lived as Modbus YAML, turned into a proper HA integration (Tom-Bom-badil/trovis-modbus-hass) on top of a device library (Tom-Bom-badil/trovis-modbus), with modbus-connection as its first real-world test case. To gauge how repeatable that is, ~25 HA Modbus YAML device configs were cloned (yaml-*) and their actually-used config features censused for migration ease (below).

modbus-connection is a small, backend-neutral Modbus abstraction: a pure ModbusConnection / ModbusUnit Protocol pair, pymodbus and tmodbus backends, a decode/encode layer, and an optional declarative model (Component / ComponentGroup / SunSpec) framework.

Contents

Migration-fit at a glance (all 60)

Library Role Backend Sync/Async Verdict
pymodbus backend self both Reference fit (already a backend)
tmodbus (via huawei) backend self async Reference fit (already a backend)
modbus-tk backend self sync Friction — sync; self-reconnects; FC gaps via RAW
minimalmodbus backend self sync Blocked — serial-only, 8 FCs, byte-swap modes
pyModbusTCP backend self sync Friction — sync; returns None/False; TCP-only
uModbus backend self sync Friction — stateless codec, owns no socket
pysolarmanv5 transport own (umodbus core) both Blocked as backend — V5 envelope + heartbeat (it is the backend you'd wrap)
enc-Sungrow-Modbus transport pymodbus subclass sync Blocked — AES-encrypted wire + key handshake
givenergy-modbus inverter custom async async Blocked — custom framer + heartbeat + tx throttle, FC06-only
pysunspec2 SunSpec own transport sync Blocked — own transport + richer dynamic model
pysunspec SunSpec own (sockets/TLS) sync Friction — runtime discovery + TLS + MAPPED
async-sunspec SunSpec pymodbus 3.12 async Friction — clean transport, but competing discovery model
solaredge_modbus inverter pymodbus (old) sync Friction — sync; legacy payload decoder
home-assistant-solaredge-modbus inverter pymodbus 3.11 async Friction — already async/TCP; decode rewrite
solaredge-modbus-multi inverter pymodbus 3.8+ async Closest fit — shared conn, many slave IDs
solaredge-modbus (timsavage) inverter pymodbus 3.x sync Clean — fixed-addr SunSpec maps onto Component 1:1
ha-sunspec SunSpec pysunspec2 sync Friction — discovery + repeating groups + throttle
ha-abb-powerone-pvi-sunspec inverter pymodbus 3.11 async Friction — runtime M160 offset probe + conditional SF
homeassistant-solax-modbus inverter pymodbus 3.8+ async Friction — ASCII framing, forced-FC16, callable scale
foxess_modbus inverter pymodbus 3.6 (vendored) sync Friction — UDP; non-contiguous multi-reg fields
Sigenergy-Local-Modbus inverter pymodbus 3.8+ async Light — TCP/BE/int-gain map directly
Sungrow-SHx (YAML) inverter HA-core modbus sync Friction — config, not code; relies on swap: byte
sunsynk inverter pymodbus / umodbus / Solarman async Friction; Solarman path blocked
ha-solarman inverter custom Solarman V5 async Blocked on default transport
deye-inverter-mqtt inverter custom Solarman V5 / AT sync Blocked on default transport
huawei_solar inverter tmodbus async Blocked — custom FC-0x41 login (raw PDU) + inter-request throttle
franklinwh-modbus battery pysunspec2 + sockets sync Friction — auto-reconnect, inter-write sleeps + watchdog + read-back-verify
sdm_modbus meter pymodbus 3.x sync Friction — returns None; UDP; word-order only
energymeter2mqtt meter pymodbus 3.7 sync Clean — thin per-register reader
WM14-96_python power analyser modbus_tk sync Friction — sign-magnitude + packed int8 decode
rs485-relay-module relay/coils hand-rolled RTU sync Friction — "coils" via FC06 register bitmaps
epsolar-tracer charge controller pymodbus 3.x sync Friction — real coil + discrete + input + 0x2B user
dbus-modbus-client (Victron) vendor pymodbus sync sync Friction — closest model match; UDP/ASCII; text byte-swap
thingsboard-gateway platform pymodbus 3.9.2 async-in-thread Friction — byteOrder default LITTLE, UDP/TLS/ASCII, forced FC
volttron platform pymodbus / modbus_tk sync Friction — little-endian struct, force FC06, cross-register scaling
artisan roasting app pymodbus async async Friction — ASCII + UDP + BCD + FC22 mask-write
genmon generator custom sync Blocked — response-addr remap, FC14 quirk
venus-os serialbattery BMS minimalmodbus (3 drivers) sync Friction; byte-swap in use; 0xF0 broadcast
modbus-cli tool umodbus sync Friction — le byte order; struct types
isf (icssploit) security scapy sync Blocked — needs raw-PDU crafting; only exotic-FC user
MODBUS_PLC...Simulator server/sim pymodbus server sync Out of scope — modbus-connection is client-only
KEBA-Heat-Pump (r3) heat pump pymodbus 3.10 sync Friction — additive offset; JSON maps; return-None
Sprsun-Heat-Pump (r3) heat pump pymodbus 3.11 async Friction — coil-write/holding-verify; two-register mode
nibe-modbus-mqtt (r3) heat pump modbus_tk sync Blocked (backend) — also "read one-at-a-time" regs
daikin-modbus-control (r3) HVAC pymodbus 2.5.3 sync Blocked — legacy pymodbus; bit-packed mode+power RMW
lambda-heatpump (r3) heat pump pymodbus 3.x sync Friction — forces FC16 on a signed setpoint
alfen_modbus (r3) EV charger pymodbus 3.11 sync Friction — shared multi-unit; forces FC16; setpoint watchdog
alfen_wallbox_modbus (r3) EV charger pymodbus 3.9 async Clean (read-only) — shared multi-unit, big-endian
keba-wallbox (r3) EV charger pymodbus 3.8 async Blocked — mandatory 0.6 s/5 s throttle + write coalescing
wallbox-modbus (r3) EV charger pymodbus 3.x async Clean — FC03/FC06, block reads, signed V2G setpoints
abb_terra_ac (r3) EV charger pymodbus 3.5 sync Friction — legacy unit=; comms-watchdog; little word order
kostal-modbusquery (r3) inverter pymodbus 3.1 sync Clean — big-byte/little-word, FC03/FC16, single sentinel
iammeter-modbus (r3) meter pymodbus 2.x sync Clean — single block, big/big; no byte swap (suspect cleared)
Sofar-Inverter (r3) inverter umodbus sync Blocked — custom vendor FC66 writes
felicity-bms (r3) battery custom RTU async Friction — standard FC03 by hand; reopen-per-call
python-neopool (r3) pool/spa pymodbus 3.11 async Friction — FC20 broadcast filter, 50 ms throttle, FC16-only, 31-reg cap
async_modbus (r3) backend umodbus core async Friction — viable backend but no internal lock; thin FCs
modbus-proxy (r3) proxy raw asyncio async Validates shared-serialized-link thesis (but reconnects)
solaredge_meterproxy (r3) server pymodbus sync Out of scope — confirms client-only boundary
as_umodbus_rtu (r3) backend MicroPython async Blocked — non-pymodbus runtime; RS485 turnaround delay
nibe_serial (r3) heat pump nibe lib sync Blocked — not Modbus (proprietary RS-485, slave role)

"Blocked" almost always means one of the four headline issues, not a fundamental incompatibility with the Protocol — which is, correctly, backend-neutral.

Home Assistant integrations that use Modbus (core + HACS)

modbus-connection lives under home-assistant-libs, so two questions matter: which HA integrations speak Modbus, and — since the library's headline value is one shared connection addressing many units — how many of them actually want a shared connection at all? The complete set of core integrations was resolved by a reverse-dependency graph over HA core's full install venv (walk every dist's Requires-Dist, find those transitively reaching pymodbus/umodbus, map back to the integration's requirements) — not by grepping source, because the fully-abstracting libs (pysaunum, pysmarty2) have zero "modbus" in their integration code.

Core — 8 integrations (venv-verified complete)

Integration Lib → backend Transport Connects to Migration
modbus (generic hub) pymodbus (async) tcp / udp / rtuovertcp / serial (rtu|ascii) many slaves 1–247 — shared bus/gateway interesting, could migrate (below)
flexit via the modbus hub (the hub's) one Flexit unit with the hub
stiebel_eltron pystiebeleltron → pymodbus TCP only one device (unit 1) CLEAN — lib ships an async descriptor variant mapping ~1:1
iskra pyiskra → pymodbus (async) TCP only one meter (unit 1) CLEAN — composite meter codecs are a custom RegisterField (a supported seam)
saunum pysaunum → pymodbus (async) TCP only one device (unit 1) CLEAN — FC03/FC06 holding, int16/uint32, raise-on-error
hr_energy_qube python-qube-heatpump → pymodbus (async) TCP only one device (unit 1) CLEAN — declarative map (coil+discrete+scale+offset) → Component ~1:1
smarty pysmarty2 → pymodbus (sync) TCP only one device (unit 1) FRICTION — the lib is synchronous; sync→async rewrite
nibe_heatpump nibe → async-modbus → umodbus TCP or serial one device (gateway) FRICTION — a backend swap (umodbus→pymodbus/tmodbus); wire is standard, not blocked

Two verdict points: needing a custom RegisterField for an exotic codec is not friction — it's the supported extension seam (so iskra is CLEAN). The genuine frictions are smarty (a synchronous lib) and nibe (a different backend). The libs also confirm-and-consume exactly the features that landed since the survey: offset (#15, Qube), discrete_input+coil (#18, Qube/smarty/nibe), force_fc16/write_coils (#14, nibe). The one shared friction is the return-None→raise inversion (hub, smarty, qube) — a per-call shim.

How many HA integrations actually want a shared connection?

Essentially one: the generic hub. Every core device integration talks to a single device on unit 1 — the device (or its gateway) hosts its own Modbus TCP server. A Stiebel ISG, a Saunum touch panel, a Qube heat pump: each is its own endpoint and will never sit on a bus alongside another manufacturer's device. They only ever call for_unit(1).

So modbus-connection's headline — one shared, internally-serialized link addressing units 1–247 — is used by almost no HA device integration. What they do want is the rest of the library: a typed async client, the Component/decode/pooling model, and backend-neutrality. Shared-connection value is real for only three shapes:

  • the generic hub — a user's RS485 bus or serial-to-Ethernet gateway hosting many slaves (the one true multi-vendor case);
  • serial RS485 setups generally (several meters/devices on one wire);
  • multi-sub-device devices — on the HACS side, solaredge-modbus-multi (several inverters as real slave IDs + meters/batteries) and huawei_solar (sub-devices on one link).

That reframes what modbus-connection sells to HA: for the typical single-device integration it's "a clean typed async client + device model + swappable backend," not "a shared-bus manager." Both are valuable.

But the low shared-connection usage is causal, not preferential — don't read it as "nobody wants it." HA has no shared-connection abstraction for integrations, so every device integration opens its own link and is structurally locked out of sharing a bus. Shared-bus usage does exist today — several devices on one RS485 wire or one serial-to-Ethernet gateway — but it's only reachable through the generic YAML hub (the one multi-slave consumer), not through any device integration. In other words, the ecosystem is single-device-per-integration because the support isn't there, not because the demand isn't. Provide the abstraction and you'd expect the reverse: (a) device integrations that can coexist on one physical bus/gateway, and (b) more of today's YAML-configured devices becoming real integrations, since the connection plumbing (multi-unit sharing + pooling + backend-neutrality) is a big part of what makes writing one costly.

Devices living in YAML today = the latent integration pipeline

The clearest evidence of that latent demand is the volume of devices people run as hand-written Modbus YAML against the generic hub because no integration exists. The exemplar is Sungrow-SHx (mkaiser) — a Sungrow inverter "integration" that is pure YAML with a very large community (hundreds of forks), no Python at all.

Concrete evidence (GitHub code-search for the HA-hub signature message_wait_milliseconds + device-family repo search, cloned under yaml-*). Two passes surfaced ~30 device Modbus configs — most are pure YAML (no custom_components); a few are early integrations mid-transition (e.g. anker-x1-ha, solis_modbus). By category:

Forum demand (HA Community, Discourse — fetched via the .json API). These YAML configs come with large, active forum threads, which is the demand signal. LG Therma V heat pump is the standout — a pure-YAML config thread with ~38k views and ~29k incoming links (config repo) — alongside Apollo (ApolloHeatPump_modbus), Ecoforest, HKS Lazar heat pumps, DTSU666 meters, and many SolarEdge configs. Heat pumps and solar in particular show up as long-running "how do I Modbus this?" threads — exactly the audience a device integration would serve.

This is exactly the Trovis story. TROVIS started life as hand-written Modbus YAML and became a proper HA integration — Tom-Bom-badil/trovis-modbus-hass on top of the device library Tom-Bom-badil/trovis-modbus — with modbus-connection as its first real-world test case. Every entry above is a potential repeat of that path.

Are they easy to migrate? Yes — overwhelmingly. A feature census across 88 YAML files in the cloned configs shows they use almost exclusively what modbus-connection + ManualComponent already cover:

HA config feature used count maps to
data_type int16/int32/uint16/uint32/string ~1380 field factories (1:1)
swap: word 877 word_order="little"
swap: byte / word_byte 0 — (not used at all → the byte_order revert is vindicated for YAML too)
input_type holding / input 1300 / 76 register_space
scale + offset 1421 / 290 field scale + offset (#15)
message_wait_milliseconds 19 message_spacing (#10)
slave_count 4 repeating_group (#20)
precision, min_value, max_value 1306 / 867 / 867 HA-entity presentation (stays above the lib)
data_type: custom + structure 15 custom RegisterField / raw read (the only real gap)
int8 8 a small byte-field (custom field, or add)

So the migratable surface is ~99% standard int types + word order + scale/offset + holding/input — all first-class. The residuals are tiny and known: custom/structure (15 lines — arbitrary struct, the one ManualComponent can't express) and int8 (8 lines). Presentation knobs (precision/min/max) belong in the HA entity regardless. *(Clones under yaml-* are illustrative — GitHub code+repo search

  • forum threads — not exhaustive; the true count is large, and Marstek Venus alone has several repos.)*

The core modbus hub — interesting, but not the migration that matters

The generic hub isn't the point — it's what you reach for when there is no device library — but it's interesting to ask whether it could migrate, because it's the one true shared-connection consumer and ManualComponent was built for exactly its shape (raw registers + user config, no device model). The connection layer maps cleanly: hub → ModbusConnection + for_unit(slave), message_wait_milliseconds → message_spacing, all five framers it uses → the connect functions, input_type → register spaces + coil/discrete_input, datatypes → field factories, scale/offset/nan_value → scale/offset/ nan, write_type → force_fc16, slave_count → repeating_group, YAML entities → ManualComponent — and it gains the cross-entity read-batching it has never had. Could-migrate verdict: yes, at FRICTION — the one real snag is the None-on-error → raises inversion (a per-call shim), plus the rewrite of its decode/entity layer.

What the hub does that no Modbus device needs. It's a device-agnostic construction kit, so these serve the user, not any device — none of the 60 surveyed libs nor the 7 other core integrations use them, and modbus-connection correctly leaves them in the HA integration:

  • Generic HA entity platforms over raw registers (the big one) — it builds sensor/binary_sensor/ switch/climate/cover/fan/light straight from YAML (climate.py, cover.py, fan.py, …). No device library ships a "generic Modbus cover"; the hub is an entity factory, a HA-framework concern above any connection library.
  • data_type: custom — an arbitrary struct string decoding a block into several values. The ultimate escape hatch; the one YAML pattern ManualComponent can't express, by design.
  • float16 — generic struct 'e', used by zero real device libs (the only clone referencing it is enc-Sungrow, a fork of HA's own validators).
  • Presentation / transform knobs — precision, min_value/max_value clamp, zero_suppress, user-configurable nan_value: end-user conveniences, not device requirements.
  • Per-entity independent scan intervals — a bag of independently-declared entities, not a device.

What the hub lacks that every device library has:

  • A device model — it's the only consumer in the whole survey with none: raw registers + user config, everything untyped and hand-declared.
  • Pooled reads — every entity self-polls a single read on its own interval; no cross-entity batching (ComponentGroup/register_ranges would be pure upside).
  • Typed values / composition — no place for computed fields, enums, or units beyond what the user types.
  • Most entity platforms. The hub only builds 7 platforms (binary_sensor, climate, cover, fan, light, sensor, switch — const.py:171-177). No water_heater, number, select, humidifier, valve, lock, button, … — so a water heater has to be shoehorned into climate, a setpoint into a switch/template, a mode into an input_select + automation. Adding a platform is a core change, and the requests sit dormant (home-assistant/discussions#649 asks for water_heater — one bot comment, no traction). A purpose-built device integration on modbus-connection has no such ceiling: it maps its device to whatever platform fits. This is literally the Trovis case — TROVIS is a heating/DHW (water-heater) controller, and a proper water_heater entity is one of the things the integration gets that the YAML hub can't express.

The boundary is clean: modbus-connection serves device libraries and the hub's transport + register + pooling layer; the entity factory, arbitrary-struct decode and presentation transforms stay in the HA integration — so the hub's config options with "no equivalent" are out of scope by design, not gaps.

HACS — where the Modbus ecosystem (and the shared-connection cases) live

HA core's Modbus surface is thin; the device ecosystem is HACS/custom — largely the set this report surveyed. On transport, HACS mirrors core: most are single-device TCP servers (each inverter/dongle is its own endpoint on unit 1, some exposing sub-meters/batteries at register offsets on the same unit), so again few need connection-sharing. Candidacy (updated for the merged features):

Two patterns. Migration: pymodbus/tmodbus + standard framing → CLEAN→FRICTION (a custom field is not friction); the only hard BLOCKers are the proprietary-transport inverters (Solarman V5, AES). Shared connection: HACS confirms core — today the multi-unit cases are the exception (solaredge-modbus-multi, huawei sub-devices, alfen station+socket, serial-bussed meters); the majority are single-device servers that would use modbus-connection for its model + backend-neutrality. But per the causal point above, "today" is load-bearing: integrations are single-connection because sharing isn't yet an option — the demand shows up in the multi-device YAML setups, not (yet) in integrations.

Appendix — the 60 libraries surveyed

Rounds 1–2 (40)

Backends / protocol libraries: pymodbus, tmodbus (via huawei_solar), modbus-tk, minimalmodbus, pyModbusTCP, uModbus, pysolarmanv5, enc-Sungrow-Modbus.

SunSpec family: pysunspec2, pysunspec, async-sunspec, solaredge_modbus, home-assistant-solaredge-modbus, solaredge-modbus-multi, solaredge-modbus (timsavage), ha-sunspec, ha-abb-powerone-pvi-sunspec.

Inverters / batteries (HA & libs): homeassistant-solax-modbus, foxess_modbus, Sigenergy-Local-Modbus, Sungrow-SHx (YAML), sunsynk, ha-solarman, deye-inverter-mqtt, huawei_solar, givenergy-modbus, franklinwh-modbus.

Meters / analysers / relays: sdm_modbus, energymeter2mqtt, WM14-96_python, rs485-relay-module, epsolar-tracer.

Platforms / vendor / apps: thingsboard-gateway, volttron, dbus-modbus-client (Victron), artisan.

Misc / unusual: genmon, venus-os_dbus-serialbattery, modbus-cli, isf (icssploit), MODBUS_PLC_Python_SCADA_Simulator.

Round 3 (20)

Heat pumps / HVAC: KEBA-Heat-Pump-Modbus-Integration, Sprsun-Heat-Pump, nibe-modbus-mqtt, daikin-modbus-control, lambda-heatpump-modbus-tcp, nibe_serial (proprietary RS-485, not Modbus).

EV chargers / wallboxes: alfen_modbus, alfen_wallbox_modbus, keba-wallbox-modbus-homeassistant, wallbox-modbus, abb_terra_ac_wallbox_modbus_sdk.

Inverters / meters / batteries: kostal-modbusquery, homeassistant-iammeter-modbus, Sofar-Inverter-MODBUS-to-MQTT, felicity-bms.

Backends / proxies / infra: async_modbus, modbus-proxy, solaredge_meterproxy (Modbus server), as_umodbus_rtu (MicroPython).

Other: python-neopool-modbus (pool/spa controller).

Design review: gaps we found & the PRs that fixed them

The report began as a design review of modbus-connection against the 60 surveyed libraries: the assumptions it got wrong or too narrow, the PRs that fixed them, and the priorities. Everything actionable is merged (see the status table), so this is kept for the record — expand if you want the detail behind the verdicts above.

Expand the full design-review track

Implementation status (PRs ↔ findings)

As of 2026-07-01, everything actionable from the survey is merged — no open PRs remain. Links are to home-assistant-libs/modbus-connection.

Finding PR Status What's left
#1 Byte order (intra-register swap) #9 → reverted ↩️ left out of core byte-swap is consumer territory — a RegisterField subclass, or (for the HA hub) its own decode as today. The confined-minority evidence supports keeping core big-endian-only
#2a UDP + serial-ASCII + TLS + ASCII-over-TCP #7 ✅ merged done — review's ASCII-over-TCP gap closed (SocketFraming allows ascii)
#2b Custom backend (Solarman V5 etc.) — ✋ not a feature the Protocol is the seam: a third party implements ModbusConnection + ModbusUnit directly. (HA refs removed from README, #16.)
#3 Inter-request spacing #10 ✅ merged done; per-unit variant intentionally not covered
#4 Write-FC override #14 ✅ merged done — shipped as a force_fc16: bool (write_mode/single/write_mask all dropped per review)
Gap A Imperative / config-driven map #8 ManualComponent ✅ merged done — with per-table ranges=
Gap A Generalise bit planning + discrete_input (FC02) #17 · #18 ✅ merged done — DiscreteInputField/discrete_input now first-class (closes the discrete-input model gap)
Gap A Runtime-counted repeating groups #20 repeating_group (supersedes #11) · #19 base_offset · #22 ✅ merged done — repeating_group() field of typed sub-Components; fixed counts fold into the single-phase read, register counts read at poll time; poolable inside a ComponentGroup
Gap A Full SunSpec model-chain scan() — deferred by design a separate SunSpec layer (built on repeating_group)
Gap B Exotic FCs / raw-PDU seam — 🚫 not planned —
Gap C Partial-poll success — 🚫 not planned —
Gap D Additive offset on scaled fields #15 ✅ merged done (also fixed a latent scaled-float write bug)
Gap D Read-size cap (max_span) #5 ✅ merged done
Gap D Exotic codecs (sign-mag, int8, BCD) — by design RegisterField subclass — just document the seam
Write validation (adjacent) #13 · #12 ✅ merged done

Merged: #5, #7, #8, #10, #12, #13, #14, #15, #16, #17, #18, #19, #20, #22 (#9 byte_order landed then was reverted — byte-swap left to consumers) — i.e. all four findings plus the entire model-framework programme (offset, ManualComponent, bit-planning generalisation, discrete-input fields, base_offset, and runtime-counted repeating groups). No open PRs. The survey backlog is fully resolved. Not building: custom-backend path (#2b — Protocol is the seam), exotic-FC decision (Gap B), partial-poll (Gap C).

The four headline findings

1. Byte order hard-coded big-endian — data-correctness gap, now high priority

decode.py / encode.py always lay register bytes out big-endian and expose only word_order. The README states this as Modbus gospel ("Byte order within each register is always big-endian"). It is violated by real software, and the evidence escalated from niche to mainstream:

  • ThingsBoard (a widely deployed IoT gateway) — byteOrder is a user parameter, default LITTLE (entities/bytes_uplink_converter_config.py:23), fed straight into BinaryPayloadDecoder.fromRegisters(byteorder=…). Verified directly. Intra-register byte swap, on by default.
  • VOLTTRON — modbus_tk driver exposes endian: big|little plus per-field struct format strings, plus a separate "Mixed Endian" column for word order — all four byte×word combinations.
  • Victron dbus-modbus-client — numerics are word-order-only (confirms the default), but Reg_text byte-swaps within each word for strings (register.py:151-163).
  • minimalmodbus (minimalmodbus.py) offers four arrangements (verified by running its encoder): BIG=ABCD, LITTLE=DCBA, BIG_SWAP=BADC, LITTLE_SWAP=CDAB. With only a word_order axis, ABCD and CDAB (= word_order="little") were expressible; BADC and DCBA — a byte swap within each register — were not. Real users of the byte-swap arrangements: modbus-cli --byte-order le (DCBA, packs each register <H), Victron Reg_text(little=True) (per-word byte swap for strings), and ThingsBoard/VOLTTRON little byte order. (The Renogy/ECS battery in venus-os…/bms/ecs.py uses LITTLE_SWAP=CDAB, which is word-swap and was already expressible — not a byte-swap case.) WM14 packs two int8s per register with sign-magnitude; Sungrow's HA-core config can set swap: byte / swap: word_byte.

Caveat (and credit): across the entire SunSpec/SolarEdge/Huawei/solax inverter family (pysunspec, async-sunspec, ABB, both SolarEdge clients, Sigenergy…), word swap is the only axis ever needed — so the hard-coded big-endian byte order is correct for the bulk of the inverter audience. Round 3 reinforced this: every heat pump (KEBA, Sprsun, Daikin, Lambda), every EV charger (Alfen, KEBA, ABB-Terra — at most word_order="little"), Kostal, the Felicity BMS, and notably IamMeter — a real energy meter, the prime byte-swap suspect — needs straight big/big (home-assistant-iammeter-modbus/__init__.py:179-256). The byte-swap need stays confined to meters like the Renogy/ECS class, power analysers and the general-purpose gateways — real, but a minority. So this is "the default is right; the minority needs an escape hatch," not "the default is wrong."

Resolution — kept out of core (consumer territory). A byte_order option was implemented and merged (#9 — correct, bit-exact against minimalmodbus/modbus-cli/Victron) then reverted. The maintainer's call: keep core big-endian-only and let the byte-swap minority self-serve, the same way sign-magnitude / int8 / BCD do — via a RegisterField subclass, or, for a raw consumer like the HA modbus hub, its own decode layer (which already performs swap: byte today). The decisive evidence: no HA core device lib needs a byte swap (see the HA section) — the only mainstream consumer is the generic hub's swap: option, and it can keep doing the swap itself. Tradeoff, stated honestly: every byte-swap consumer now reimplements ~4 lines rather than passing a flag; the maintainer judged the smaller core surface worth it. (If a device ever needs byte-swap inside the declarative model with no clean @property route, revisit — the #9 implementation is a known-good starting point.)

2. "Backend = pymodbus or tmodbus; framing = socket/rtu/serial" — two sub-problems, one easy

The Protocol layer is correctly neutral, but the shipped backends and README frame the world as pick one of two libraries that speak standard framing. Round 2 splits this cleanly:

  • (a) Framings stock pymodbus already speaks but connect_* doesn't expose: UDP (sdm_modbus, Victron, artisan, foxess, ThingsBoard), ASCII (ThingsBoard, Victron, artisan, solax), TLS / Modbus-Security (pysunspec, ThingsBoard). A near-free win — just surface them in the connect functions and the Framing literal.
  • (b) Genuinely custom transports needing a non-pymodbus/tmodbus backend:
    • Solarman V5 (pysolarmanv5, used by ha-solarman, deye, sunsynk) — wraps RTU PDUs in a 0xA5…0x15 envelope with a rolling sequence number, the logger serial baked into each frame, and mandatory replies to unsolicited HEARTBEAT/HANDSHAKE control frames. Verified: own _v5_header/_v5_trailer/sequence_number. No stock client can speak it.
    • AES-encrypted Sungrow (enc-Sungrow-Modbus) — a GET_KEY handshake, AES-ECB-encrypted bodies, a double-connect, and a daily key refresh.
    • GivEnergy — a custom "Transparent" framer with embedded device serial + CRC and a mandatory dongle heartbeat. Deye-AT wraps Modbus in AT commands over UDP. Huawei uses a private FC 0x41 for login.
    • Round 3 widened the category further: Sofar writes via a custom vendor FC66 PDU a stock client can't emit (Sofar-Inverter…/me3000.py:177-348); NeoPool must install a transport-level filter to drop the device's spontaneous FC20 broadcast frames or pymodbus mis-reads (python-neopool…/client.py:317-418); as_umodbus_rtu is MicroPython — the backend allow-list excludes the runtime itself; nibe_serial isn't Modbus at all (a proprietary NIBE RS-485 protocol, client in a slave role); and nibe-modbus-mqtt uses modbus_tk, a third library. These mostly reinforce that the Protocol is the right seam and the backend set is the limiter.

Fix: (a) add UDP/ASCII/TLS to the connect functions. (b) The custom-transport cases (Solarman V5, AES, GivEnergy, vendor FCs, broadcast filters, embedded runtimes) need no library work — the backend-neutral Protocol is already the extension seam: a third party implements ModbusConnection + ModbusUnit over its own transport and it drops into anything typed against the Protocol. (Today that means implementing both a connection and a unit class, since the per-FC I/O lives in the unit; an optional future refactor could let a backend implement only a connection and reuse a generic ModbusUnit.) A short README note pointing this out is the only thing worth doing. → PR #7 — ✅ merged. connect_udp, serial "rtu"|"ascii", connect_tls, and ASCII-over-TCP (SocketFraming now allows "ascii", closing the review's one gap) — all of (a). (b) is not a feature to build — the Protocol already enables it.

3. The "no self-reconnect / owner recreates" model is coherent — the one residual is inter-request spacing

"No self-reconnect; the owner recreates the connection" composes cleanly with the session needs real devices have, so most of what first looks like a gap here isn't one:

  • Post-connect login is not blocked. The connect helper returning an already-connected instance only means there's no separate .connect() call — the owner still receives the live ModbusConnection, and for_unit() works immediately, so a handshake is just unit.write_register(UNLOCK_REG, code) before it hands units to consumers.
  • Re-login after a drop needs no framework hook, precisely because the connection doesn't self-reconnect: when on_connection_lost fires, the owner recreates the connection and re-runs that same routine. (Huawei's lib needs an on_reconnected callback only because it rides tmodbus's self-reconnecting transport, where the reconnect is invisible and must be discovered — the opposite of this design.) Initial login, re-login and heartbeat all live naturally in the owner.

What does genuinely remain:

  • Inter-request spacing across a shared connection — the real gap. Serialization lives inside the backend, and several consumers share one connection. If a device needs ≥N ms between any two frames on the wire regardless of which consumer issued them, the owner can't enforce it — it can pace its own calls, but not the spacing of other consumers' interleaved calls. Round 3 made this look common rather than exotic: KEBA mandates MIN_READ_INTERVAL=0.6s / MIN_WRITE_INTERVAL=5.0s (keba-wallbox…/const.py:33-35), NeoPool sleeps 50 ms between requests, as_umodbus_rtu inserts an explicit delay for RS485 line turnaround — on top of GivEnergy (tx_message_wait=0.25s), Huawei, ThingsBoard, Sungrow. "Backend serializes" gives ordering, not spacing — tmodbus has the knob, pymodbus doesn't, and the abstraction exposes neither. This belongs on the connect functions as a configurable inter-request delay.
  • Device-side comms watchdogs (a new Round-3 dimension). EV chargers fail safe: KEBA, ABB Terra (set_communication_timeout) and Alfen revert the current setpoint unless the client re-writes/keeps polling within a timeout window — so the owner must keep talking on a schedule. This is a keepalive obligation the owner can run, but the design should at least name it: an idle shared connection isn't free for these devices. (Related owner-side patterns Round 3 piled up: KEBA/NeoPool write coalescing and write-then-readback verify.)
  • The "backend serializes" guarantee is backend-specific. async_modbus (a plausible alt backend: async, multi-unit, raises) does no internal locking — concurrent coroutines would interleave on one stream (async_modbus/core.py:124-144). pymodbus and tmodbus do serialize, but the Protocol doc states it as a flat guarantee; a third-party backend must be told it owns that lock.
  • Login via a non-standard function code (Huawei's private FC 0x41) is not blocked by "post-connect" — it's blocked by the typed surface having no raw-PDU seam. Tracked under finding B (exotic FCs / raw PDU), not here. Where "login" is an ordinary register write, it already works.
  • Transport-level handshakes (enc-Sungrow's AES key exchange, Solarman's serial-learning) genuinely can't be done "after connect via a unit" — they're part of establishing the transport itself. That's the custom-backend point under finding #2 (a backend whose own connect() performs the handshake), not a hook on the existing backends.
  • Heartbeat / keepalive (pysolarmanv5 V5 control frames, GivEnergy dongle FC01, Huawei reg 49999) and write-then-read-back-verify (franklinwh) are owner responsibilities the design accommodates; they're noted for completeness, not as gaps.

Fix: add a configurable inter-request delay on the connect functions (the one primitive the owner can't synthesize). Keep "no self-reconnect / owner recreates" as the default — it is coherent with login and heartbeat living in the owner. The custom-transport and raw-PDU cases are covered by findings #2 and B. → PR #10 — ✅ merged. A message_spacing (seconds) param on all connect functions: tmodbus forwards it to native wait_between_requests; pymodbus enforces it with a small paced lock measured from request completion (verified atomic across concurrent units). Note — it's per-connection (one value for the whole link), which covers the dominant case (the bottleneck is the shared bus/gateway). The one surveyed exception is ThingsBoard, which carries the delay per-unit (slave.py:108) but enforces it against the shared master's last-tx clock (master.py:49). If that multi-heterogeneous-device-on- one-link case matters, the follow-up is to let the delay be set at for_unit() granularity while the connection owns the clock+lock; it degrades to one connection-wide value when all units agree.

4. model.write guesses the function code — confirmed and contradicted; needs a write-FC override

  • Confirms the by-length heuristic (FC06 for one word, else FC16): Victron (device.py:124-128), artisan, franklinwh, timsavage SolarEdge, ABB Terra (width-driven), KEBA, wallbox-modbus.
  • Contradicts it — and Round 3 tipped the balance toward forcing FC16 even for one register: Alfen (select.py:127, number.py:144), NeoPool (client.py:1294), Lambda (write_registers(102, value) with a single signed setpoint), on top of sunsynk (pysunsynk.py:88) and solax (const.py:112). The other direction (FC06-only: GivEnergy, VOLTTRON modbus_tk) needs no override — auto already emits FC06 for a single-register field, and these devices' writable registers are all single-word. So the only override with a real use case is force FC16 on a single register; forcing FC06 is redundant with auto.
  • Bit-packed registers are not a framework gap. Daikin packs power (bit 0) and mode (high byte) in one register (daikin-modbus-control/app.py:103,245); Sprsun spreads HVAC mode over two registers; sunsynk/foxess bit-twiddle by hand. Setting a subset of bits without clobbering the rest is read-modify-write — but RMW is just the existing read + write (plus a race and an extra round trip), so it adds nothing the consumer lacks. It belongs in a Component.write override, or model the whole register as one field and expose bits via a @property. And the FC22 atomic masked write is already on the unit as mask_write_register (0x16) for the rare device that supports it — no model feature required.

Fix: a per-field "force FC16" override — that's the whole gap. Forcing FC06 is not needed (auto already emits FC06 for a single-register field, and rejecting a multi-word FC06 write is the only thing a single mode could add). A masked-write field isn't worth adding either: if it's FC22 it duplicates unit.mask_write_register; if it's RMW it duplicates read + write. Leave bit-packing to a Component.write override. → PR #14 — ✅ merged as a single force_fc16: bool (default False, requires writable). write_mask, the three-way write_mode and the redundant "single" were all dropped — the minimal form: auto already emits FC06 for a one-word field, so the only real override is "force FC16."

Structural gaps (beyond the four)

A. Runtime model discovery + variable-length repeating groups — the biggest modelling gap

The most sophisticated comparison libraries don't declare a static map — they read it off the device. pysunspec, pysunspec2 (ha-sunspec) and async-sunspec scan() the SunSpec model chain (walk model-id + length headers to the 0xFFFF end marker) and expose repeating groups whose count is read from a register at poll time. ABB probes three candidate offsets for its M160 block and caches the winner. modbus-connection's model is static and author-time: fixed addresses, index/stride with the repeat count known up front. It cannot express "discover which models exist" or "repeat N times where N is read at runtime." Likely out of scope by design — but it's the one capability the best SunSpec libraries have that this framework structurally lacks. Worth either a discovery helper or an explicit "build Components dynamically from raw reads" note. → Two PRs split this exactly as recommended: #8 ManualComponent — an imperative, runtime-built, re-plannable read/write group that mixes all four tables (the "build dynamically from raw reads" + config-driven-map answer; ready to merge), and #11 RepeatingGroup — repeat count read from a register each poll (draft, stacked on #8). Full SunSpec model-chain scan() is deliberately left out of core (per #11) for a future dedicated SunSpec layer built on these primitives.

B. The exotic function codes have almost no consumers — and the one that exists wants raw PDUs

Across all 60 libraries, the only consumer of codes beyond FC01-06/0F/10 (+ occasional 0x2B) is isf, the ICS security framework — and it uses only file-record 0x14/0x15, mask 0x16, read/write 0x17, FIFO 0x18, leaving 0x07, 0x08, 0x0B, 0x0C, 0x11 with zero consumers anywhere (epsolar is the lone 0x2B user). Worse, isf needs raw, malformable PDU access (illegal lengths, bad byte counts, arbitrary unit ids) for fuzzing — which a typed ModbusUnit API deliberately prevents. So the full-19-FC surface (a) isn't earning its keep, and (b) its one plausible consumer would want a raw-PDU escape hatch instead of twelve typed methods. This cuts against the project's own "Simplicity First / nothing speculative" ethos.

Fix: trim the speculative codes (0x07/0x08/0x0B/0x0C/0x11) or re-label them "best-effort, backend-dependent"; if diagnostics/security matter, add a single raw-PDU seam rather than typed methods nobody calls. → Not planned (maintainer) — the full 19-FC surface stays as-is.

C. Partial-poll success is thrown away

GivEnergy returns RefreshPartiallySucceeded (per-device ReadFailures + the partial plant); Victron has per-register max_age staleness; franklinwh/GivEnergy serve last-good data on a rejected read (so "success ≠ fresh"). modbus-connection's "every method raises; all-or-nothing update" throws away the partial result a polling consumer wants. The model layer already tolerates per-field Modbus exceptions (sets the field to None) — but at the connection level a dropped link mid-poll loses everything read so far. ComponentGroup is the right place to expose "what did we get before it failed." → Not planned (maintainer).

D. Field-model: one common add, the rest is an extension seam

The framework does not need to ship every datatype the survey turned up. RegisterField is a public abstract base (model.fields.RegisterField; only decode is abstract, encode has a default) — verified — so a consumer can ship its own codec as a RegisterField subclass without touching the library. The right policy is: ship the common types; let libraries define the exotic ones. That reframes most of this section from "gaps" to "extensible by design," with one genuine exception:

  • Add an additive offset to the scaled fields — this one is common enough to belong in core. gauge only multiplies, but value * scale + offset recurs across unrelated devices: KEBA (val*scale+offset, write inverts to (v-offset)/scale), sunsynk TempSensor (− 100), deye (offset). Without it, every affine-scaled field needs a private subclass for what is really a one-line generalisation of gauge. → PR #15 — ✅ merged (raw * scale + offset, inverted on write, across the numeric + float factories; also fixed a latent bug where writable scaled floats never inverted their scale on encode).
  • Leave to consumer subclasses (document the seam): sign-magnitude integers (deye, WM14), int8 / high-byte+low-byte of one register (solax, WM14, Daikin), raw "list of words" (solax), non-contiguous multi-register combines (foxess addresses: list[int]), BCD (artisan), Sofar's HB*0x10000+LB. These are device-specific codecs; a RegisterField subclass is the correct home for them, and the README should show one worked example so authors know it's supported.
  • Two scaling shapes worth a small core nicety (or just a doc): gain as a divisor (Sigenergy/Huawei value/gain; scale=1/gain loses precision on 1/3-type ratios), and cross-register live scaling (VOLTTRON scale_reg scales by another register's current value, beyond scale_register's 10**sf).

Two addressing notes that aren't about types:

  • for_unit() is the wrong axis for register-offset sub-devices. SolarEdge meters/batteries are register windows on one slave id, not separate units (so: separate Components on one ModbusUnit, which is fine) — but battery base addresses 57600/57856/58368 are irregular, defeating a single stride+index. The escape hatch (absolute addresses per component) works; just document that stride is for regular repeats only.
  • Non-standard addressing. seplosv3 uses a custom broadcast 0xF0 (outside 1–247 and not unit-0); genmon devices reply with a different unit id than requested (ResponseAddress) — stock pymodbus/tmodbus reject mismatched response units. Niche, but real; don't assume request-unit == response-unit is always enforceable. Also: many datasheets/tools use the legacy 4xxxx/1xxxx offset convention vs raw 0-based — an explicit note (or helper) avoids off-by-40001 errors.

One pooling refinement Round 3 surfaced: some registers must be read in isolation (nibe-modbus-mqtt: "these registers must be read only one at a time"), and some devices cap a read below 125 (NeoPool at 31, Huawei at 64). max_span covers the cap — now configurable per device via PR #5, merged (max_gap / max_span) — but a per-field "never merge this into a block" opt-out would still be needed for the read-isolated case (nibe-modbus-mqtt) the gap/range planner can't express. → opt-out: no PR yet.

A second, quieter model gap the survey under-flagged: the model layer could read coils (FC01) but had no discrete-input (FC02) field type — so a device with read-only status bits in discrete inputs (epsolar-tracer, Sprsun) couldn't be modelled declaratively, only read raw. (The raw ModbusUnit.read_discrete_inputs was always present; this was purely a field-descriptor omission.) → Closed by #18, merged — discrete_input/DiscreteInputField (FC02, read-only), planned and read alongside coils.

A related, intentional constraint (not a gap): a declarative Component's register fields all share one register_space — holding XOR input (register_space is a single class attribute). This is asymmetric with its bit fields, which mix coil (FC01) and discrete_input (FC02) freely per field. So a logical sub-system whose data spans both holding and input registers (e.g. epsolar-tracer, which selects the space by address range) can't be one typed Component. Two first-class workarounds cover it, so it's a modelling choice rather than a capability gap: (a) split into a holding Component and an input Component and group them in a ComponentGroup (which mixes register spaces and pools each in its own block reads); or (b) use ManualComponent, which is per-field/per-table across all four tables (space= on add, plus per-table *_ranges). A logical sub-system almost always lives in one register space (input = measurements, holding = settings/commands), so this rarely bites; keeping Component single-space keeps register_ranges and the read-only-input write check simple. Making register_space per-field is feasible (bits and ManualComponent prove the machinery) but not worth the added surface on the typed path.

What modbus-connection gets RIGHT (validated by the survey)

  • One shared, internally-serialized connection addressing many units. Matches solaredge-modbus-multi (many real slave IDs on one socket), Huawei sub-devices, tmodbus's own for_unit_id, sdm_modbus (child meters share parent.client), Victron (refcounted pool, units 1-247), ThingsBoard, Alfen (station + sockets on one link). The correct core abstraction, and the main thing the ad-hoc libraries get wrong (most open one socket per device). modbus-proxy (Round 3) is independent corroboration: its entire reason to exist is to put many clients behind one upstream link serialized by a single asyncio.Lock (src/modbus_proxy.py:137,158-160) — exactly this design's thesis. (Its lazy auto-reconnect is the one place it diverges, which is the deliberate scope cut discussed in finding #3.)
  • Raise-on-failure, never return None. The return-None/False/"" libraries (pyModbusTCP, genmon, deye, solaredge_modbus, sdm_modbus, ThingsBoard, isf, artisan's public API) are exactly the ones where partial reads silently corrupt data. The raising contract (solaredge-modbus-multi, sunsynk reads, huawei, umodbus, minimalmodbus, Victron, epsolar) is the better one.
  • Block-read pooling with gap/range planning. Everyone serious batches: Huawei (MAX_BATCHED_REGISTERS_GAP=16 — identical to your max_gap), Victron pack_list (sort/merge/split-at-125/hole_max gap — almost line-for-line your planner), foxess, solax, sunsynk, pysunspec2 (access_regions ≈ your register_ranges), VOLTTRON, artisan. A clean generalisation of the common pattern.
  • scale_register/sunssf and the per-type SunSpec sentinel table. Round 1 worried "a single nan is insufficient"; Round 2 verified model/sunspec.py:66-73 against pysunspec's SUNS_UNIMPL_* and async-sunspec's not_implemented tables and found them to match exactly (incl. acc = 0, sunssf = 0x8000). Resolved — sufficient for SunSpec; the generic single-nan is the right default elsewhere.
  • holding vs input as separate address spaces. Correct; several libs conflate them (genmon's global FC3→FC4 swap) and get into trouble.
  • Client-only scope is coherent. The simulator and solaredge_meterproxy (a Modbus server emulating a meter) both confirm there's no server story (and the mock backend is test scaffolding, not a runnable server) — fine, but worth stating in the README so nobody expects a datastore.
  • The big-endian-byte default is right for most devices. Round 3's heat pumps, EV chargers and even a real energy meter (IamMeter) need at most word order — the byte-swap need (finding #1) is a real but contained minority, not the common case. The maintainer's decision to keep core big-endian-only (reverting the byte_order option #9) and leave byte-swap to consumers follows from this: the default is a good bet, and the escape hatch is a RegisterField subclass / consumer decode rather than a core flag.

Adversarial review of the open PRs

Each PR was re-checked against the real library code it targets, with spot-checks run against the PR's own code. Summary verdicts:

  • #7 (UDP/ASCII/TLS) — UDP, serial-ASCII and TLS verified against foxess, Victron, artisan and pysunspec. One real gap: ASCII-over-TCP. homeassistant-solax-modbus (__init__.py:526) and ThingsBoard use FramerType.ASCII over a TCP socket, but connect_tcp/connect_udp accept only socket/rtu — so ASCII-over-TCP is unreachable. ~5-line widening of the framer Literal. Minor: the connect-and-raise block is duplicated 4× (a _open() helper would cut ~30 lines). Verdict: merge after adding ASCII-over-TCP. → Acted on (merged #7): SocketFraming now allows "ascii", so ASCII-over-TCP works.
  • #8 (ManualComponent) — Pools reads across all four tables and genuinely reuses the planner; maps 1:1 onto ThingsBoard's per-item-table config. Three fixes: (1) register_ranges / coil_ranges aren't exposed — a regression vs Component for range-gated sparse devices (Sungrow/HA-YAML); (2) the coil/discrete planner (_plan_bit_blocks/_bulk_read_bits) is a near-copy of the register one — generalise CoilItem with a space instead of duplicating; (3) add() mutates field.name = key, a foot-gun if a field is reused. Verdict: approve with changes. → Acted on: #8 now exposes per-table ranges=; the planner-dedup (2) became its own PR #17 (_plan_blocks_by_space/_read_blocks_by_space shared by both planners, + discrete_input fields). The field.name foot-gun (3) is still worth confirming.
  • #9 (byte order) — Verified bit-exact for all four arrangements vs minimalmodbus, modbus-cli and Victron, across ints/floats/strings, decode+encode round-trips; 125 tests pass. Only a cosmetic test-comment fix (it had the minimalmodbus constant labels inverted — the same error this report had, now corrected in finding #1). Verdict: approve. → Later reverted by the maintainer — the implementation was correct, but the call is to keep core big-endian-only and let byte-swap live in the consumer (the HA hub already does it in its own decode). See finding #1.
  • #10 (message_spacing) — Correct and atomic (lock held across wait+request, gap measured from completion), identical on both backends; concurrency test passes. Known limits to call out (not blockers): a single value can't express KEBA's distinct read (0.6 s) vs write (5.0 s) spacing, nor ThingsBoard's per-unit spacing. Verdict: approve.
  • #11 (RepeatingGroup) — Maps the ABB M160 / SunSpec-160 / Huawei count-driven repeat cleanly (shared and per-instance sunssf both work); offset-probing correctly left out as discovery. Minor follow-ups: same field.name clobbering as #8 (count/header fields uncopied), no max_instances sanity cap, instances are holding/coil-only. Verdict: approve with minor follow-ups.
  • #14 (write-mode + masked) — Request changes. write_mode="single"/"multiple" is correct and covers sunsynk/solax/alfen (force FC16), givenergy (FC06-only), VOLTTRON (force FC06). But write_mask emits FC22 only, and the devices that actually need masked bits do read-modify-write over FC06/FC16 because their firmware lacks FC22 — Daikin RMWs over FC06 (app.py:243-249), sunsynk RMWs over FC16 (# if bitmask we should READ the register first!!!), and even artisan ships a local RMW fallback for non-FC22 devices. So write_mask serves only true FC22 hardware (artisan, which was already a one-line passthrough) and does not solve the bit-packed case the README sells (Daikin/Sprsun). But RMW shouldn't be a library feature either — it's just the existing read + write plus a race and an extra round trip, so it adds nothing the consumer lacks; bit-packed registers belong in a Component.write override (or a one-field-plus-@property). write_mask only earns its place as a thin FC22 convenience, and even that is marginal since mask_write_register is already on the ModbusUnit protocol (0x16). Recommendation: keep write_mode, drop write_mask (or keep it strictly as a documented FC22-only helper, not sold as the bit-packed answer). Also trim write_mode from the 7 multi-word factories where it's inert. → Acted on (merged #14): shipped as a single force_fc16: bool — write_mask, write_mode and the redundant "single" were all dropped, exactly the minimal form the review converged on.
  • #15 (additive offset) — Round-trips verified vs KEBA, sunsynk, deye; composes correctly with sunssf, nan and signed; rounding rule sound. Bonus: fixes a latent bug where a writable scaled float never inverted its scale on encode. Verdict: approve.

Recommended priorities (combined) — with status

  1. ↩️ Intra-register byte-order / swap option — implemented (#9) then reverted: kept out of core, byte-swap is consumer territory (RegisterField subclass / consumer decode). No HA core device lib needs it; the one mainstream user (the generic hub) does the swap itself.
  2. ✅ UDP / ASCII / TLS (incl. ASCII-over-TCP) — merged (#7). The custom-backend path (#2b) needs no code — the Protocol is the seam (HA refs removed in #16).
  3. ✅ Configurable inter-request delay — merged (#10); optional follow-up: per-for_unit() spacing for heterogeneous shared links (ThingsBoard shape).
  4. ✅ Per-field "force FC16" override — merged (#14) as a minimal force_fc16: bool (write_mode/single/write_mask all dropped). Masked-write wasn't a framework need.
  5. ✅ Additive offset on scaled fields — merged (#15); exotic codecs stay consumer-defined via the RegisterField subclass seam.
  6. Decide the exotic-FC surface — 🚫 not planned.
  7. Surface partial-poll results — 🚫 not planned.
  8. ✅ Device-framework finish — all merged: #8 ManualComponent, #17 bit-planning generalisation, #18 discrete-input fields, #19 base_offset and #20 repeating_group (typed sub-Components; fixed counts fold into the single-phase read). Docs notes on the model constraints (discrete-input, single-register-space, static-model limits) are in the field-model section above.

Net: the entire survey backlog is merged — no open PRs. The only remaining items are the deliberate non-goals: the exotic-FC decision (6), partial-poll results (7), and the custom-backend path (#2b — the Protocol is already the seam).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment