You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2026/09/11 15:10:13 UTC
(plc4x) 01/05: feat(plc4go/modbus): add the read optimizer, ported from plc4j
This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit 1033e8b89760c38943694913bc69581b976ec230
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Fri Sep 11 15:27:50 2026 +0200
feat(plc4go/modbus): add the read optimizer, ported from plc4j
plc4go answers a multi-tag read by splitting it into one request per tag and
dispatching them all concurrently. That is affordable over plain TCP and
ruinous behind a TCP<->RTU gateway, which serialises every transaction onto a
slow serial line: a profile of 84 tags becomes 84 round trips arriving at once,
and the gateway resets the connection rather than answering. plc4j has not had
this problem since it gained ModbusReadOptimizer; the Go driver never caught up.
This ports that optimizer. Tags that one request can answer are merged into a
block read, and the single response is split apart again afterwards. The
grouping follows the Java exactly: partitioned by register area and by unit id
(with "no explicit unit id" its own group), a window anchored at the first tag
of the block, and no contiguity requirement or gap tolerance -- the window width
IS the gap tolerance, so tags at 0 and 100 become one 101-register read while
tags at 0 and 2100 stay two. A tag wider than the ceiling passes through
unsplit, as in Java, because halving it would hand the caller half a value.
Nothing calls this yet. Reader.Read still rejects multi-tag requests and the
connection still installs the SingleItemRequestInterceptor, so the driver's
behaviour is unchanged. Wiring it up alters per-tag failure semantics -- a
failed block fails every tag in it -- and is a separate step.
Three defects found by review while porting, each with a regression test that
was watched failing before the fix:
- splitRegisters sliced the block to the tag's packed byte length rather than
whole registers. ParseRegisters byte-swaps in place for the two *_BYTE_SWAP
orders and can only swap whole pairs, so an odd-length slice (scalar CHAR,
odd-length STRING) left its last byte unswapped where the unoptimized read
swaps it -- a silently different value under an OK response code.
- Extended registers were given the plain-register ceiling of 125. FC 0x14
spends a byte count plus per-item framing the plain function codes do not, so
a full-width block answers past the 253-byte PDU limit, twice over if it
straddles a file boundary. They get their own ceiling of 123.
- An oversized span narrowed to uint16 unchecked, so a 65536-register span came
out as quantity 0 -- a request for nothing, which a device answers happily and
which therefore reads as success. The span is clamped instead, leaving a
request the device rejects loudly.
The test suite ports every case from ModbusReadOptimizerTest, including the ones
encoding shipped Java bug fixes (unit id surviving a merge, coil arrays
returning every element and across a byte boundary, a single coil still scalar,
a non-BOOL coil reported UNSUPPORTED), and adds differential checks that decode
each tag both through the optimizer and through the unoptimized path.
---
plc4go/internal/modbus/ReadOptimizer.go | 394 ++++++++++++
.../modbus/ReadOptimizerInvariants_test.go | 670 +++++++++++++++++++++
plc4go/internal/modbus/ReadOptimizer_test.go | 479 +++++++++++++++
3 files changed, 1543 insertions(+)
diff --git a/plc4go/internal/modbus/ReadOptimizer.go b/plc4go/internal/modbus/ReadOptimizer.go
new file mode 100644
index 0000000000..49f2d555a9
--- /dev/null
+++ b/plc4go/internal/modbus/ReadOptimizer.go
@@ -0,0 +1,394 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package modbus
+
+import (
+ "cmp"
+ "context"
+ "slices"
+
+ apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+ apiValues "github.com/apache/plc4x/plc4go/pkg/api/values"
+ readWriteModel "github.com/apache/plc4x/plc4go/protocols/modbus/readwrite/model"
+ "github.com/apache/plc4x/plc4go/spi/errors"
+ "github.com/apache/plc4x/plc4go/spi/values"
+)
+
+// This is the read optimizer, ported from plc4j's ModbusReadOptimizer.
+//
+// A request asks for one tag, and without this every tag costs a round trip. That is affordable
+// over plain TCP and ruinous behind a TCP<->RTU gateway, where every transaction is serialized
+// onto a slow serial line: a few hundred tags turn into a few hundred round trips and the gateway
+// falls over. So tags that can be answered by one request are merged into a block read here, and
+// the single response is split apart again afterwards.
+//
+// Nothing in this file talks to a connection. optimizeReads decides what to ask for and
+// splitBlockResponse decodes what came back; issuing the requests is the caller's business.
+//
+// NOTE: nothing calls this yet. The driver still issues one request per tag (see Reader.Read,
+// which rejects multi-tag requests outright, and the SingleItemRequestInterceptor the connection
+// installs). Wiring it in changes per-tag failure semantics - a failed block fails every tag in
+// it - and so is a separate, separately reviewable step. Until then this file changes no
+// behaviour.
+
+// namedTag pairs a tag with the name the read request knows it by. Tags travel by value here, the
+// way they do everywhere else in this package - castToModbusTagFromPlcTag hands out a value too.
+type namedTag struct {
+ name string
+ tag modbusTag
+}
+
+// blockTag is one member of a block: the tag itself plus where its data sits inside the block's
+// response.
+type blockTag struct {
+ name string
+ tag modbusTag
+ // offset is how far into the block this tag's data starts, counted in whatever the area
+ // addresses - bits for the coil and discrete-input areas, registers for the three register
+ // areas. It is derivable from the two addresses; it is spelled out so that splitting a
+ // response never has to redo the subtraction, and so that a caller can see the layout.
+ offset uint16
+}
+
+// readBlock is one request the optimizer wants sent: a run of addresses in one area, on one unit,
+// covering every tag in tags.
+type readBlock struct {
+ tagType TagType
+ // unitId is the unit every tag in this block declared, or nil when they all left it to the
+ // connection's default. It has to be carried through to the request: a block read reaches
+ // exactly one unit, and dropping the unit-id here would silently send the merged request to
+ // the connection's default unit instead (plc4j GitHub issue #2686).
+ unitId *uint8
+ // address is the wire address the block starts at.
+ address uint16
+ // quantity is how many addresses the block covers - coils for the two bit areas, registers
+ // for the three register areas. This is the count the request carries.
+ quantity uint16
+ tags []blockTag
+}
+
+// tagReadResult is what one tag of a block ends up with once the response has been split.
+type tagReadResult struct {
+ name string
+ responseCode apiModel.PlcResponseCode
+ // value is set only for a result that is OK.
+ value apiValues.PlcValue
+ // err is why responseCode is not OK, kept for logging. The response itself carries only the
+ // code, so without this the reason a tag failed would be lost at the point it is known.
+ err error
+}
+
+// areaOrder is the order the blocks of the individual areas are emitted in. Five separate areas,
+// never merged with one another: they are addressed independently and read with different function
+// codes, so address 7 means five unrelated things. The order itself is the one plc4j's
+// optimizeReads walks, kept so both implementations produce the same request sequence.
+var areaOrder = []TagType{Coil, HoldingRegister, InputRegister, ExtendedRegister, DiscreteInput}
+
+// unitGroupKey identifies one group of tags that may be merged with one another: same area, same
+// unit. A tag that declared no unit-id forms its own group rather than joining the group of the
+// unit that happens to be the connection's default - the connection may be re-configured, and the
+// tag asked for "whatever the connection uses", which is not the same statement as a number.
+type unitGroupKey struct {
+ tagType TagType
+ hasUnitId bool
+ unitId uint8
+}
+
+// optimizeReadsWithSpecLimits merges tags using the largest request the modbus specification
+// allows, which is also what plc4j defaults to.
+func optimizeReadsWithSpecLimits(tags []namedTag) []readBlock {
+ return optimizeReads(tags, maxCoilQuantity, maxRegisterQuantity)
+}
+
+// optimizeReads groups the given tags by area and unit and cuts each group into the blocks that
+// have to be read, in a deterministic order.
+//
+// maxCoilsPerRequest and maxRegistersPerRequest bound how wide one block may get; see
+// maxCoilQuantity and maxRegisterQuantity for the ceilings the protocol itself imposes.
+func optimizeReads(tags []namedTag, maxCoilsPerRequest uint16, maxRegistersPerRequest uint16) []readBlock {
+ groups := make(map[unitGroupKey][]namedTag, len(tags))
+ for _, tag := range tags {
+ key := unitGroupKey{tagType: tag.tag.TagType}
+ if tag.tag.UnitId != nil {
+ key.hasUnitId, key.unitId = true, *tag.tag.UnitId
+ }
+ groups[key] = append(groups[key], tag)
+ }
+
+ // Map iteration order is random, and a driver that sends its requests in a different order on
+ // every read is untestable and miserable to follow in a packet capture.
+ keys := make([]unitGroupKey, 0, len(groups))
+ for key := range groups {
+ keys = append(keys, key)
+ }
+ slices.SortFunc(keys, compareUnitGroups)
+
+ var blocks []readBlock
+ for _, key := range keys {
+ group := groups[key]
+ // Sorted by address, so that the window below only ever has to look forwards. The name
+ // breaks ties, for the same reason the keys are sorted.
+ slices.SortFunc(group, func(a, b namedTag) int {
+ return cmp.Or(cmp.Compare(a.tag.Address, b.tag.Address), cmp.Compare(a.name, b.name))
+ })
+ blocks = append(blocks, blocksOf(key, group, maxPerRequestFor(key.tagType, maxCoilsPerRequest, maxRegistersPerRequest))...)
+ }
+ return blocks
+}
+
+// compareUnitGroups orders the groups: areas in the order plc4j emits them, and within an area the
+// tags that named no unit before those that did, then by unit-id.
+func compareUnitGroups(a, b unitGroupKey) int {
+ return cmp.Or(
+ cmp.Compare(slices.Index(areaOrder, a.tagType), slices.Index(areaOrder, b.tagType)),
+ cmp.Compare(boolAsInt(a.hasUnitId), boolAsInt(b.hasUnitId)),
+ cmp.Compare(a.unitId, b.unitId),
+ )
+}
+
+func boolAsInt(value bool) int {
+ if value {
+ return 1
+ }
+ return 0
+}
+
+// maxPerRequestFor is the ceiling that applies to one block of the given area. The two bit areas
+// are counted in coils, everything else in registers.
+// maxExtendedRegisterQuantity is the widest extended-register block that still answers within the
+// 253-byte Modbus PDU limit. FC 0x14 spends a byte count plus two bytes of per-item framing that
+// FC 0x03/0x04 do not, and a block crossing a 10000-register file boundary pays that framing twice.
+const maxExtendedRegisterQuantity = 123
+
+func maxPerRequestFor(tagType TagType, maxCoilsPerRequest uint16, maxRegistersPerRequest uint16) uint16 {
+ switch tagType {
+ case Coil, DiscreteInput:
+ return maxCoilsPerRequest
+ case ExtendedRegister:
+ // Extended registers travel over FC 0x14 (Read File Record), whose response carries a
+ // byte count and then per-item framing (data length + reference type) that the plain
+ // register function codes do not. A full 125-register block therefore answers with a PDU
+ // over the 253-byte Modbus limit, and a block straddling a 10000-register file boundary
+ // becomes two items and overflows by more. 123 is the widest that fits both shapes.
+ if maxRegistersPerRequest > maxExtendedRegisterQuantity {
+ return maxExtendedRegisterQuantity
+ }
+ return maxRegistersPerRequest
+ default:
+ return maxRegistersPerRequest
+ }
+}
+
+// blocksOf cuts one area/unit group, which arrives sorted by address, into blocks.
+//
+// The window is anchored at the first tag of a block and is exactly maxPerRequest addresses wide.
+// Every following tag whose end fits inside that window joins the block, whether or not anything
+// was addressed in between; the first one that does not fit anchors the next window.
+//
+// There is deliberately no contiguity requirement and no separate gap tolerance: the width of the
+// window *is* the gap tolerance (plc4j ModbusReadOptimizer.optimizeRegisters). Tags at register 0
+// and register 100 therefore become one 101 register read - reading 99 registers nobody asked for
+// costs a handful of bytes, while the second round trip it saves costs a whole transaction on a
+// serial line. Tags at 0 and 2100 stay two reads.
+func blocksOf(key unitGroupKey, group []namedTag, maxPerRequest uint16) []readBlock {
+ var blocks []readBlock
+ var current []namedTag
+ // Held as int rather than uint16: the end of the window is the first tag's address plus the
+ // ceiling, which runs past the end of the address space for a tag near the top of it.
+ var first, last, window int
+
+ for _, entry := range group {
+ address := int(entry.tag.Address)
+ end := address + int(tagSpan(entry.tag))
+ switch {
+ case len(current) == 0:
+ // The first tag of a block sets the block's end with no cap check of its own, so a
+ // single tag wider than maxPerRequest passes through unsplit. That is the plc4j
+ // contract and it is the right one: this optimizer only ever merges requests, it
+ // never takes one apart, and quietly halving a tag's read would hand the caller
+ // half a value. A tag that wide is rejected when it is parsed (see
+ // validateAddressAndQuantity), so it can only get here from a caller that built it
+ // by hand.
+ first, last, window = address, end, address+int(maxPerRequest)
+ case end > window:
+ blocks = append(blocks, newReadBlock(key, first, last, current))
+ current = nil
+ first, last, window = address, end, address+int(maxPerRequest)
+ default:
+ // Not max(last, end) for tidiness: the group is sorted by address, but a wide tag
+ // followed by a narrow one inside it would otherwise shrink the block.
+ last = max(last, end)
+ }
+ current = append(current, entry)
+ }
+ if len(current) > 0 {
+ blocks = append(blocks, newReadBlock(key, first, last, current))
+ }
+ return blocks
+}
+
+// newReadBlock assembles the block covering [first, last) out of the tags that fell into it.
+func newReadBlock(key unitGroupKey, first int, last int, tags []namedTag) readBlock {
+ // first/last are ints because an oversized pass-through tag can push the span past what a
+ // uint16 holds. Narrowing that unchecked would wrap: a 65536-register span becomes a request
+ // for nothing, and a block anchored near the top of the address space would ask the device to
+ // read past 65535. Such a tag cannot come from the address parser, which rejects it, only from
+ // a caller that built one by hand. Clamping the SPAN (not the end address) is what matters:
+ // a 65536-register span narrows to quantity 0, a request for nothing, which a device answers
+ // happily and which therefore reads as success. A clamped-but-oversized quantity is rejected
+ // by the device instead, which is the honest outcome for a request that should not exist.
+ span := last - first
+ if span > maxWireAddress {
+ span = maxWireAddress
+ }
+ block := readBlock{
+ tagType: key.tagType,
+ address: uint16(first),
+ quantity: uint16(span),
+ tags: make([]blockTag, 0, len(tags)),
+ }
+ if key.hasUnitId {
+ unitId := key.unitId
+ block.unitId = &unitId
+ }
+ for _, entry := range tags {
+ block.tags = append(block.tags, blockTag{
+ name: entry.name,
+ tag: entry.tag,
+ offset: entry.tag.Address - block.address,
+ })
+ }
+ return block
+}
+
+// tagSpan is how many addresses a tag occupies in its area: coils in a bit area, where one element
+// is one address, and registers everywhere else.
+//
+// This is the same count the unoptimized read puts into its request (see readRequestPdu), which is
+// what makes a block that covers a tag's span enough to answer it.
+func tagSpan(tag modbusTag) uint64 {
+ return registerCountOf(tag.TagType, tag.Datatype, uint64(tag.Quantity), tag.StringLength)
+}
+
+// splitBlockResponse takes the payload one block read came back with and answers every tag of the
+// block out of it.
+//
+// blockData is the raw payload of the response: the packed coil bytes for the two bit areas, the
+// registers for the other three. defaultByteOrder is the connection's, used for every tag that did
+// not declare one of its own.
+func splitBlockResponse(ctx context.Context, block readBlock, blockResponseCode apiModel.PlcResponseCode, blockData []byte, defaultByteOrder ByteOrder) []tagReadResult {
+ results := make([]tagReadResult, 0, len(block.tags))
+ if blockResponseCode != apiModel.PlcResponseCode_OK {
+ // One request answered for all of them, so its failure is every member's failure. Merging
+ // reads means a device that rejects one address takes the whole block down with it - the
+ // price of the optimization, and the reason the block's own code is reported rather than
+ // a code invented here.
+ for _, member := range block.tags {
+ results = append(results, tagReadResult{name: member.name, responseCode: blockResponseCode})
+ }
+ return results
+ }
+ for _, member := range block.tags {
+ switch block.tagType {
+ case Coil, DiscreteInput:
+ results = append(results, splitBits(member, blockData))
+ default:
+ results = append(results, splitRegisters(ctx, member, blockData, defaultByteOrder))
+ }
+ }
+ return results
+}
+
+// splitBits answers one tag of a coil or discrete-input block out of the packed bits that came
+// back.
+func splitBits(member blockTag, blockData []byte) tagReadResult {
+ if member.tag.Datatype != readWriteModel.ModbusDataType_BOOL {
+ // A coil carries a single bit, and assembling coils into wider types is not implemented.
+ // Answering with the first bit would look like a successful read of something else
+ // entirely, so this is reported instead (plc4j ModbusReadOptimizer.splitResponse).
+ return tagReadResult{
+ name: member.name,
+ responseCode: apiModel.PlcResponseCode_UNSUPPORTED,
+ err: errors.Errorf("reading coils/discrete inputs as %s is not supported, only BOOL is", member.tag.Datatype),
+ }
+ }
+ // An array tag occupies as many coils as it has elements, and the block covers all of them -
+ // so every element is extracted, not just the first (plc4j GitHub issue #2060).
+ firstBit := int(member.offset)
+ numberOfElements := int(member.tag.Quantity)
+ if lastByte := (firstBit + numberOfElements - 1) / 8; lastByte >= len(blockData) {
+ return tagReadResult{
+ name: member.name,
+ responseCode: apiModel.PlcResponseCode_INTERNAL_ERROR,
+ err: errors.Errorf("the device answered with %d bytes of coils, which doesn't reach coil %d", len(blockData), firstBit+numberOfElements-1),
+ }
+ }
+ bits := make([]apiValues.PlcValue, 0, numberOfElements)
+ for i := range numberOfElements {
+ // Coils are packed least significant bit first, so a run of them crosses into the next
+ // byte at bit 8 rather than restarting there.
+ bit := firstBit + i
+ bits = append(bits, values.NewPlcBOOL(blockData[bit/8]&(1<<(bit%8)) != 0))
+ }
+ if numberOfElements == 1 {
+ // A single coil reads as a plain BOOL, not as a list of one - the same shape
+ // ParseRegisters gives a single value, so merging a tag into a block never changes the
+ // type the caller sees.
+ return tagReadResult{name: member.name, responseCode: apiModel.PlcResponseCode_OK, value: bits[0]}
+ }
+ return tagReadResult{name: member.name, responseCode: apiModel.PlcResponseCode_OK, value: values.NewPlcList(bits)}
+}
+
+// splitRegisters answers one tag of a register block out of the registers that came back.
+func splitRegisters(ctx context.Context, member blockTag, blockData []byte, defaultByteOrder ByteOrder) tagReadResult {
+ // The block is a flat run of registers, so a member starts a whole number of registers into
+ // it. That even offset is what lets ParseRegisters be reused unchanged: it decodes positionally
+ // from bit 0 of the slice it is handed and never learns an address, so a slice that starts
+ // mid-register would silently decode garbage.
+ byteOffset := int(member.offset) * 2
+ // Whole REGISTERS, not the tag's packed byte length. The two differ for any type whose
+ // payload is an odd number of bytes - a scalar CHAR, a STRING of odd declared length - and
+ // the difference is not cosmetic: ParseRegisters byte-swaps in place for the two *_BYTE_SWAP
+ // orders and can only swap whole pairs, so an odd-length slice leaves its last byte unswapped
+ // while the unoptimized read, which is handed the device's whole registers, swaps it. Slicing
+ // to the payload length would therefore decode a DIFFERENT value than the same tag read on its
+ // own, silently and with an OK response code. ParseRegisters ignores bytes it does not need,
+ // which is exactly what the unoptimized path already relies on.
+ byteLength := int(tagSpan(member.tag)) * 2
+ if byteOffset+byteLength > len(blockData) {
+ return tagReadResult{
+ name: member.name,
+ responseCode: apiModel.PlcResponseCode_INTERNAL_ERROR,
+ err: errors.Errorf("the device answered with %d bytes, which doesn't cover the %d bytes at offset %d", len(blockData), byteLength, byteOffset),
+ }
+ }
+ // The byte order is the tag's own if it declared one - a block may well mix them, and the
+ // swapping modes are handled inside ParseRegisters rather than being redone here.
+ value, err := ParseRegisters(ctx, blockData[byteOffset:byteOffset+byteLength], member.tag.Datatype,
+ member.tag.Quantity, member.tag.resolveByteOrder(defaultByteOrder), member.tag.StringLength)
+ if err != nil {
+ return tagReadResult{
+ name: member.name,
+ responseCode: apiModel.PlcResponseCode_INTERNAL_ERROR,
+ err: errors.Wrap(err, "error parsing data item"),
+ }
+ }
+ return tagReadResult{name: member.name, responseCode: apiModel.PlcResponseCode_OK, value: value}
+}
diff --git a/plc4go/internal/modbus/ReadOptimizerInvariants_test.go b/plc4go/internal/modbus/ReadOptimizerInvariants_test.go
new file mode 100644
index 0000000000..4161c89a3c
--- /dev/null
+++ b/plc4go/internal/modbus/ReadOptimizerInvariants_test.go
@@ -0,0 +1,670 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package modbus
+
+import (
+ "context"
+ "fmt"
+ "math/rand"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+ apiValues "github.com/apache/plc4x/plc4go/pkg/api/values"
+ readWriteModel "github.com/apache/plc4x/plc4go/protocols/modbus/readwrite/model"
+)
+
+// ---------------------------------------------------------------------------
+// invariants every optimizeReads result must satisfy
+// ---------------------------------------------------------------------------
+
+func checkInvariants(t *testing.T, in []namedTag, blocks []readBlock, maxCoils, maxRegs uint16) {
+ t.Helper()
+
+ seen := map[string]int{}
+ for _, block := range blocks {
+ require.NotEmpty(t, block.tags, "a block with no tags was emitted")
+ limit := int(maxPerRequestFor(block.tagType, maxCoils, maxRegs))
+ for _, member := range block.tags {
+ seen[member.name]++
+ // offset must be the stated subtraction
+ assert.Equal(t, member.tag.Address-block.address, member.offset,
+ "offset of %q in block %+v", member.name, block)
+ // coverage: the block must actually contain the whole tag
+ span := int(tagSpan(member.tag))
+ tagStart := int(member.tag.Address)
+ tagEnd := tagStart + span
+ blockStart := int(block.address)
+ blockEnd := blockStart + int(block.quantity)
+ assert.GreaterOrEqual(t, tagStart, blockStart,
+ "tag %q starts before its block (block %+v)", member.name, block)
+ assert.LessOrEqual(t, tagEnd, blockEnd,
+ "block %+v does NOT cover tag %q (tag %d..%d, block %d..%d)",
+ block, member.name, tagStart, tagEnd, blockStart, blockEnd)
+ }
+ // width: either within the ceiling, or a single pass-through tag
+ if int(block.quantity) > limit && len(block.tags) != 1 {
+ t.Errorf("block %+v is %d wide, over the %d ceiling, with %d tags",
+ block, block.quantity, limit, len(block.tags))
+ }
+ // Must not run off the end of the 16 bit address space -- unless a member tag already did.
+ //
+ // The optimizer may never MANUFACTURE an out-of-range request by merging in-range tags;
+ // that is the bug this guards. But a single tag that is itself out of range passes through
+ // unsplit (the documented contract), and emitting it verbatim is the better of the bad
+ // options: the device answers an illegal-data-address exception, which is loud. Clamping
+ // it would return fewer coils than asked for with an OK code, which reads as success.
+ if int(block.address)+int(block.quantity) > 65536 {
+ memberAlreadyOutOfRange := false
+ for _, member := range block.tags {
+ if int(member.tag.Address)+int(tagSpan(member.tag)) > 65536 {
+ memberAlreadyOutOfRange = true
+ break
+ }
+ }
+ if !memberAlreadyOutOfRange {
+ t.Errorf("block %+v was merged past the 65535 wire-address ceiling (ends at %d) from in-range tags",
+ block, int(block.address)+int(block.quantity))
+ }
+ }
+ }
+ for _, tag := range in {
+ if seen[tag.name] != 1 {
+ t.Errorf("tag %q appears %d times in the output, want exactly 1", tag.name, seen[tag.name])
+ }
+ }
+ assert.Len(t, seen, len(in), "output names %v vs input count %d", seen, len(in))
+}
+
+// ---------------------------------------------------------------------------
+// 1. degenerate inputs
+// ---------------------------------------------------------------------------
+
+func TestAdvEmptyAndSingle(t *testing.T) {
+ assert.Empty(t, optimizeReadsWithSpecLimits(nil))
+ assert.Empty(t, optimizeReadsWithSpecLimits([]namedTag{}))
+
+ single := named(coilAt(7, 1))
+ blocks := optimizeReadsWithSpecLimits(single)
+ require.Len(t, blocks, 1)
+ checkInvariants(t, single, blocks, maxCoilQuantity, maxRegisterQuantity)
+}
+
+// Two tags that carry the same name. A PlcReadRequest keys tags by name so this cannot arrive
+// from the API, but the optimizer accepts a slice and has no guard.
+func TestAdvDuplicateNames(t *testing.T) {
+ in := []namedTag{
+ {name: "same", tag: holdingRegisterAt(0, 1)},
+ {name: "same", tag: holdingRegisterAt(5, 1)},
+ }
+ blocks := optimizeReadsWithSpecLimits(in)
+ total := 0
+ for _, b := range blocks {
+ total += len(b.tags)
+ }
+ assert.Equal(t, 2, total, "both entries must survive into blocks")
+
+ results := splitBlockResponse(context.Background(), blocks[0], apiModel.PlcResponseCode_OK,
+ make([]byte, int(blocks[0].quantity)*2), BigEndianOrder)
+ byName := map[string]int{}
+ for _, r := range results {
+ byName[r.name]++
+ }
+ t.Logf("splitBlockResponse produced %d results, distinct names %d: %v", len(results), len(byName), byName)
+ assert.Equal(t, 1, len(byName), "DEFECT if >1: two results share one name, a map keyed by name loses one")
+}
+
+// Identical addresses, different names.
+func TestAdvDuplicateAddresses(t *testing.T) {
+ in := named(holdingRegisterAt(10, 1), holdingRegisterAt(10, 1), holdingRegisterAt(10, 1))
+ blocks := optimizeReadsWithSpecLimits(in)
+ checkInvariants(t, in, blocks, maxCoilQuantity, maxRegisterQuantity)
+ require.Len(t, blocks, 1)
+ assert.Equal(t, uint16(1), blocks[0].quantity)
+ assert.Len(t, blocks[0].tags, 3)
+}
+
+// ---------------------------------------------------------------------------
+// 2. the top of the address space
+// ---------------------------------------------------------------------------
+
+func TestAdvTagAt65535(t *testing.T) {
+ in := named(holdingRegisterAt(65535, 1))
+ blocks := optimizeReadsWithSpecLimits(in)
+ require.Len(t, blocks, 1)
+ t.Logf("block: addr=%d qty=%d (ends at %d)", blocks[0].address, blocks[0].quantity,
+ int(blocks[0].address)+int(blocks[0].quantity))
+ checkInvariants(t, in, blocks, maxCoilQuantity, maxRegisterQuantity)
+}
+
+// Merging tags near the ceiling: legally-parseable tags only.
+func TestAdvMergeNearCeiling(t *testing.T) {
+ in := named(holdingRegisterAt(65420, 1), holdingRegisterAt(65534, 1))
+ blocks := optimizeReadsWithSpecLimits(in)
+ for _, b := range blocks {
+ t.Logf("block: addr=%d qty=%d end=%d tags=%d", b.address, b.quantity,
+ int(b.address)+int(b.quantity), len(b.tags))
+ }
+ checkInvariants(t, in, blocks, maxCoilQuantity, maxRegisterQuantity)
+}
+
+// A coil block anchored at the very top with an oversized pass-through tag: the block's
+// quantity is fine but address+quantity wraps past the wire ceiling.
+func TestAdvOversizedTagAtCeiling(t *testing.T) {
+ in := named(coilAt(65535, 2000))
+ blocks := optimizeReadsWithSpecLimits(in)
+ require.Len(t, blocks, 1)
+ t.Logf("block addr=%d qty=%d -> reads coils %d..%d",
+ blocks[0].address, blocks[0].quantity,
+ blocks[0].address, int(blocks[0].address)+int(blocks[0].quantity)-1)
+ checkInvariants(t, in, blocks, maxCoilQuantity, maxRegisterQuantity)
+}
+
+// ---------------------------------------------------------------------------
+// 3. uint16 truncation of the block quantity
+// ---------------------------------------------------------------------------
+
+// A hand-built tag whose span does not fit into uint16. newReadBlock stores
+// quantity: uint16(last - first) with no check.
+func TestAdvQuantityTruncation(t *testing.T) {
+ // STRING(65535) x 3 in a holding register -> 98303 registers
+ wide := newTagFromWireAddress(HoldingRegister, 0, 3,
+ readWriteModel.ModbusDataType_STRING, 65535, tagConfig{}, true)
+ span := tagSpan(wide)
+ t.Logf("tagSpan = %d registers (does not fit uint16: %v)", span, span > 65535)
+
+ in := []namedTag{{name: "wide", tag: wide}}
+ blocks := optimizeReadsWithSpecLimits(in)
+ require.Len(t, blocks, 1)
+ t.Logf("block addr=%d quantity=%d (span %d)", blocks[0].address, blocks[0].quantity, span)
+
+ // The span cannot be asked for: a Modbus quantity is 16 bits, so 98303 is not expressible at
+ // all. The requirement is therefore not "preserve the span" but "never wrap": the block must
+ // come out clamped to the widest representable request, never as quantity 0, which is a
+ // request for nothing that a device answers happily and which would read as success.
+ assert.NotZero(t, blocks[0].quantity, "an oversized span must never wrap to a request for nothing")
+ assert.Equal(t, uint16(maxWireAddress), blocks[0].quantity,
+ "an oversized span must be clamped to the widest representable quantity")
+}
+
+// The same truncation, but landing on quantity == 0, i.e. a request that asks for nothing.
+func TestAdvQuantityTruncatesToZero(t *testing.T) {
+ // STRING(32768) x 2 -> 65536 bytes -> 32768 registers. Need exactly 65536 registers:
+ // WSTRING(32768) x 2 -> widthBits 32768*16 = 524288 -> 2 values = 1048576 bits
+ // = 131072 bytes -> 65536 registers.
+ wide := newTagFromWireAddress(HoldingRegister, 0, 2,
+ readWriteModel.ModbusDataType_WSTRING, 32768, tagConfig{}, true)
+ span := tagSpan(wide)
+ t.Logf("tagSpan = %d registers", span)
+ require.Equal(t, uint64(65536), span, "test setup: want a span of exactly 65536")
+
+ blocks := optimizeReadsWithSpecLimits([]namedTag{{name: "wide", tag: wide}})
+ require.Len(t, blocks, 1)
+ t.Logf("block addr=%d quantity=%d", blocks[0].address, blocks[0].quantity)
+ assert.NotEqual(t, uint16(0), blocks[0].quantity,
+ "DEFECT: a block that asks for 65536 registers is emitted as quantity 0")
+}
+
+// ---------------------------------------------------------------------------
+// 4. degenerate ceilings from config
+// ---------------------------------------------------------------------------
+
+func TestAdvZeroCeiling(t *testing.T) {
+ done := make(chan []readBlock, 1)
+ in := named(coilAt(0, 1), coilAt(1, 1), coilAt(2, 1), holdingRegisterAt(0, 1))
+ go func() { done <- optimizeReads(in, 0, 0) }()
+ select {
+ case blocks := <-done:
+ for _, b := range blocks {
+ t.Logf("block %v addr=%d qty=%d tags=%d", b.tagType, b.address, b.quantity, len(b.tags))
+ }
+ checkInvariants(t, in, blocks, 0, 0)
+ case <-t.Context().Done():
+ t.Fatal("optimizeReads hung with a zero ceiling")
+ }
+}
+
+func TestAdvCeilingOfOne(t *testing.T) {
+ in := named(holdingRegisterAt(0, 1), holdingRegisterAt(1, 1), holdingRegisterAt(2, 1))
+ blocks := optimizeReads(in, 1, 1)
+ checkInvariants(t, in, blocks, 1, 1)
+ assert.Len(t, blocks, 3, "a ceiling of one register cannot merge anything")
+}
+
+// A ceiling of 65535 with a tag near the top: window = address + 65535 overflows uint16 but is
+// held as int, so this only proves the int widening actually works.
+func TestAdvMaxCeiling(t *testing.T) {
+ in := named(holdingRegisterAt(65000, 1), holdingRegisterAt(65535, 1))
+ blocks := optimizeReads(in, 65535, 65535)
+ for _, b := range blocks {
+ t.Logf("block addr=%d qty=%d end=%d", b.address, b.quantity, int(b.address)+int(b.quantity))
+ }
+ checkInvariants(t, in, blocks, 65535, 65535)
+}
+
+// ---------------------------------------------------------------------------
+// 5. extended registers and the 10000-register file boundary
+// ---------------------------------------------------------------------------
+
+func extendedAt(address uint16, quantity uint16) modbusTag {
+ return tagAt(ExtendedRegister, address, quantity, readWriteModel.ModbusDataType_INT, tagConfig{})
+}
+
+// A block the optimizer produces has to be expressible as FC 0x14 items. The response item's
+// dataLength is a uint8 carrying COUNT(data)+1, and the response's own byteCount is a uint8
+// carrying the total size of all items.
+func TestAdvExtendedBlockFitsTheFileRecordFraming(t *testing.T) {
+ for _, test := range []struct {
+ name string
+ tags []namedTag
+ }{
+ {"straddles the boundary", named(extendedAt(9995, 1), extendedAt(10100, 1))},
+ {"inside one file", named(extendedAt(100, 1), extendedAt(220, 1))},
+ {"at the very top", named(extendedAt(65410, 1), extendedAt(65534, 1))},
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ blocks := optimizeReadsWithSpecLimits(test.tags)
+ checkInvariants(t, test.tags, blocks, maxCoilQuantity, maxRegisterQuantity)
+ for _, b := range blocks {
+ groups := splitExtendedRegister(b.address, b.quantity)
+ byteCount := 0
+ for _, g := range groups {
+ itemData := int(g.lengthWords) * 2
+ // dataLength is uint8 = COUNT(data)+1
+ if itemData+1 > 255 {
+ t.Errorf("item %+v needs dataLength %d, which does not fit a uint8", g, itemData+1)
+ }
+ byteCount += 2 + itemData // dataLength + referenceType + data
+ }
+ pdu := 2 + byteCount // functionCode + byteCount + items
+ t.Logf("block addr=%d qty=%d -> %d file group(s), response byteCount=%d, PDU=%d bytes",
+ b.address, b.quantity, len(groups), byteCount, pdu)
+ if byteCount > 255 {
+ t.Errorf("DEFECT: response byteCount %d does not fit the uint8 field (block %+v)", byteCount, b)
+ }
+ if pdu > 253 {
+ t.Errorf("DEFECT: FC 0x14 response PDU would be %d bytes, over the 253 byte modbus limit (block %+v)", pdu, b)
+ }
+ }
+ })
+ }
+}
+
+// The worst case the ceiling allows: a full 125-register extended block straddling a file
+// boundary.
+func TestAdvExtendedFullWidthBlock(t *testing.T) {
+ // 9990..10114 inclusive is 125 registers - the plain-register ceiling. Extended registers
+ // cannot be that wide, so the optimizer must NOT merge these into one block. Asserting a
+ // single block here is what the defective version did; assert the invariant instead.
+ in := named(extendedAt(9990, 1), extendedAt(10114, 1))
+ blocks := optimizeReadsWithSpecLimits(in)
+ require.NotEmpty(t, blocks)
+ for _, b := range blocks {
+ t.Logf("block addr=%d qty=%d", b.address, b.quantity)
+ groups := splitExtendedRegister(b.address, b.quantity)
+ byteCount := 0
+ for _, g := range groups {
+ t.Logf(" file=%d record=%d words=%d", g.fileNumber, g.recordNumber, g.lengthWords)
+ byteCount += 2 + int(g.lengthWords)*2
+ }
+ pdu := 2 + byteCount
+ t.Logf("response byteCount=%d PDU=%d", byteCount, pdu)
+ assert.LessOrEqual(t, pdu, 253,
+ "every extended block the optimizer emits must answer within one modbus PDU")
+ }
+}
+
+// splitExtendedRegister on a block whose address is 0. Extended registers are addressed from
+// zero on the wire, so this is the legitimate bottom of the area.
+func TestAdvExtendedAtZeroAndTop(t *testing.T) {
+ require.NotPanics(t, func() {
+ g := splitExtendedRegister(0, 1)
+ t.Logf("addr 0: %+v", g)
+ })
+ require.NotPanics(t, func() {
+ g := splitExtendedRegister(65535, 125)
+ t.Logf("addr 65535 len 125: %+v", g)
+ })
+ require.NotPanics(t, func() {
+ g := splitExtendedRegister(0, 0)
+ t.Logf("len 0: %+v (nil groups -> a request with no items)", g)
+ })
+ // A request whose length wraps the file numbering.
+ require.NotPanics(t, func() {
+ g := splitExtendedRegister(65535, 65535)
+ t.Logf("addr 65535 len 65535: %d groups, last=%+v", len(g), g[len(g)-1])
+ })
+}
+
+// ---------------------------------------------------------------------------
+// 6. randomised property test
+// ---------------------------------------------------------------------------
+
+func TestAdvPropertyEveryTagAppearsExactlyOnce(t *testing.T) {
+ types := []TagType{Coil, DiscreteInput, HoldingRegister, InputRegister, ExtendedRegister}
+ dtypes := []readWriteModel.ModbusDataType{
+ readWriteModel.ModbusDataType_BOOL,
+ readWriteModel.ModbusDataType_INT,
+ readWriteModel.ModbusDataType_DINT,
+ readWriteModel.ModbusDataType_LREAL,
+ }
+ rng := rand.New(rand.NewSource(1))
+ for iteration := range 3000 {
+ n := rng.Intn(30)
+ in := make([]namedTag, 0, n)
+ for i := range n {
+ tagType := types[rng.Intn(len(types))]
+ dt := dtypes[rng.Intn(len(dtypes))]
+ if tagType == Coil || tagType == DiscreteInput {
+ dt = readWriteModel.ModbusDataType_BOOL
+ }
+ addr := uint16(rng.Intn(65536))
+ quantity := uint16(1 + rng.Intn(8))
+ // keep the tag inside the address space, the way the parser would
+ span := registerCountOf(tagType, dt, uint64(quantity), 1)
+ if uint64(addr)+span > 65535 {
+ addr = uint16(65535 - span)
+ }
+ cfg := tagConfig{}
+ if rng.Intn(3) == 0 {
+ u := uint8(rng.Intn(4))
+ cfg.unitId = &u
+ }
+ in = append(in, namedTag{
+ name: fmt.Sprintf("t%02d", i),
+ tag: newTagFromWireAddress(tagType, addr, quantity, dt, 1, cfg, quantity > 1),
+ })
+ }
+ maxCoils := uint16(1 + rng.Intn(2500))
+ maxRegs := uint16(1 + rng.Intn(200))
+ blocks := optimizeReads(in, maxCoils, maxRegs)
+ t.Run(fmt.Sprintf("iteration%d", iteration), func(t *testing.T) {
+ if t.Failed() {
+ return
+ }
+ checkInvariants(t, in, blocks, maxCoils, maxRegs)
+ })
+ if t.Failed() {
+ t.Logf("failing input (maxCoils=%d maxRegs=%d):", maxCoils, maxRegs)
+ for _, tag := range in {
+ t.Logf(" %s: type=%v addr=%d qty=%d dt=%v unit=%v",
+ tag.name, tag.tag.TagType, tag.tag.Address, tag.tag.Quantity, tag.tag.Datatype, tag.tag.UnitId)
+ }
+ t.FailNow()
+ }
+ }
+}
+
+// Shuffling the input must not change the output: a driver that reorders its requests between
+// reads is untestable.
+func TestAdvDeterministicUnderShuffling(t *testing.T) {
+ rng := rand.New(rand.NewSource(7))
+ base := named(
+ coilAt(0, 1), coilAt(2500, 1), holdingRegisterAt(0, 1), holdingRegisterAt(300, 1),
+ tagAt(InputRegister, 5, 1, readWriteModel.ModbusDataType_INT, onUnit(3)),
+ tagAt(InputRegister, 6, 1, readWriteModel.ModbusDataType_INT, onUnit(1)),
+ extendedAt(9995, 1), extendedAt(10005, 1),
+ tagAt(DiscreteInput, 9, 1, readWriteModel.ModbusDataType_BOOL, tagConfig{}),
+ )
+ want := render(optimizeReadsWithSpecLimits(base))
+ for range 200 {
+ shuffled := append([]namedTag(nil), base...)
+ rng.Shuffle(len(shuffled), func(i, j int) { shuffled[i], shuffled[j] = shuffled[j], shuffled[i] })
+ got := render(optimizeReadsWithSpecLimits(shuffled))
+ require.Equal(t, want, got, "block layout changed when the input order changed")
+ }
+}
+
+func render(blocks []readBlock) string {
+ out := ""
+ for _, b := range blocks {
+ unit := "-"
+ if b.unitId != nil {
+ unit = fmt.Sprint(*b.unitId)
+ }
+ names := make([]string, 0, len(b.tags))
+ for _, m := range b.tags {
+ names = append(names, fmt.Sprintf("%s@%d", m.name, m.offset))
+ }
+ out += fmt.Sprintf("[%v u=%s a=%d q=%d %v]", b.tagType, unit, b.address, b.quantity, names)
+ }
+ return out
+}
+
+// ---------------------------------------------------------------------------
+// 7. splitBlockResponse against hand-built tags
+// ---------------------------------------------------------------------------
+
+func TestAdvSplitBitsWithZeroQuantity(t *testing.T) {
+ member := blockTag{name: "zero", tag: newTagFromWireAddress(Coil, 0, 0,
+ readWriteModel.ModbusDataType_BOOL, 1, tagConfig{}, false), offset: 0}
+ require.NotPanics(t, func() {
+ r := splitBits(member, []byte{0xFF})
+ t.Logf("quantity 0 -> code=%v value=%v", r.responseCode, r.value)
+ })
+ // offset 0, quantity 0, empty block data: lastByte = (0+0-1)/8 = 0 in Go, so the guard
+ // compares 0 >= 0 and reports an error rather than indexing.
+ require.NotPanics(t, func() {
+ r := splitBits(member, nil)
+ t.Logf("quantity 0, no data -> code=%v err=%v", r.responseCode, r.err)
+ })
+}
+
+func TestAdvSplitBitsHugeOffset(t *testing.T) {
+ member := blockTag{name: "far", tag: coilAt(0, 8), offset: 65535}
+ require.NotPanics(t, func() {
+ r := splitBits(member, make([]byte, 4))
+ assert.Equal(t, apiModel.PlcResponseCode_INTERNAL_ERROR, r.responseCode)
+ })
+}
+
+func TestAdvSplitRegistersOffsetOverflow(t *testing.T) {
+ member := blockTag{name: "far", tag: holdingRegisterAt(0, 1), offset: 65535}
+ require.NotPanics(t, func() {
+ r := splitRegisters(context.Background(), member, make([]byte, 8), BigEndianOrder)
+ assert.Equal(t, apiModel.PlcResponseCode_INTERNAL_ERROR, r.responseCode)
+ })
+}
+
+// Round trip: build a fake device image, let the optimizer merge, split the response and
+// compare every tag against what an unoptimized single read of that tag would have produced.
+func TestAdvRoundTripMatchesUnoptimizedRead(t *testing.T) {
+ image := make([]byte, 2*4096)
+ for i := range image {
+ image[i] = byte(i*7 + 3)
+ }
+ ctx := context.Background()
+ rng := rand.New(rand.NewSource(42))
+ dtypes := []readWriteModel.ModbusDataType{
+ readWriteModel.ModbusDataType_INT,
+ readWriteModel.ModbusDataType_DINT,
+ readWriteModel.ModbusDataType_REAL,
+ readWriteModel.ModbusDataType_LREAL,
+ }
+ for iteration := range 500 {
+ n := 1 + rng.Intn(8)
+ in := make([]namedTag, 0, n)
+ for i := range n {
+ dt := dtypes[rng.Intn(len(dtypes))]
+ quantity := uint16(1 + rng.Intn(4))
+ addr := uint16(rng.Intn(800))
+ in = append(in, namedTag{
+ name: fmt.Sprintf("t%d", i),
+ tag: newTagFromWireAddress(HoldingRegister, addr, quantity, dt, 1, tagConfig{}, quantity > 1),
+ })
+ }
+ blocks := optimizeReadsWithSpecLimits(in)
+ got := map[string]string{}
+ for _, b := range blocks {
+ data := image[int(b.address)*2 : int(b.address)*2+int(b.quantity)*2]
+ for _, r := range splitBlockResponse(ctx, b, apiModel.PlcResponseCode_OK, data, BigEndianOrder) {
+ require.Equal(t, apiModel.PlcResponseCode_OK, r.responseCode,
+ "iteration %d: %s failed: %v", iteration, r.name, r.err)
+ got[r.name] = fmt.Sprint(r.value)
+ }
+ }
+ for _, tag := range in {
+ words, err := tag.tag.lengthWords()
+ require.NoError(t, err)
+ raw := image[int(tag.tag.Address)*2 : int(tag.tag.Address)*2+int(words)*2]
+ want, err := ParseRegisters(ctx, raw, tag.tag.Datatype, tag.tag.Quantity, BigEndianOrder, tag.tag.StringLength)
+ require.NoError(t, err)
+ require.Equal(t, fmt.Sprint(want), got[tag.name],
+ "iteration %d: tag %s (addr=%d qty=%d dt=%v) decoded differently through the optimizer",
+ iteration, tag.name, tag.tag.Address, tag.tag.Quantity, tag.tag.Datatype)
+ }
+ require.Len(t, got, len(in), "iteration %d: a tag was dropped", iteration)
+ }
+}
+
+// The bit areas, same round trip.
+func TestAdvRoundTripCoils(t *testing.T) {
+ ctx := context.Background()
+ rng := rand.New(rand.NewSource(99))
+ bits := make([]byte, 512)
+ for i := range bits {
+ bits[i] = byte(i*31 + 17)
+ }
+ for iteration := range 500 {
+ n := 1 + rng.Intn(8)
+ in := make([]namedTag, 0, n)
+ for i := range n {
+ quantity := uint16(1 + rng.Intn(20))
+ addr := uint16(rng.Intn(2000))
+ in = append(in, namedTag{
+ name: fmt.Sprintf("c%d", i),
+ tag: newTagFromWireAddress(Coil, addr, quantity, readWriteModel.ModbusDataType_BOOL, 1, tagConfig{}, quantity > 1),
+ })
+ }
+ blocks := optimizeReadsWithSpecLimits(in)
+ got := map[string][]bool{}
+ for _, b := range blocks {
+ nbytes := (int(b.quantity) + 7) / 8
+ data := make([]byte, nbytes)
+ for i := range nbytes {
+ // pack the device's coils starting at the block's address
+ for bit := range 8 {
+ globalBit := int(b.address) + i*8 + bit
+ if bits[globalBit/8]&(1<<(globalBit%8)) != 0 {
+ data[i] |= 1 << bit
+ }
+ }
+ }
+ for _, r := range splitBlockResponse(ctx, b, apiModel.PlcResponseCode_OK, data, BigEndianOrder) {
+ require.Equal(t, apiModel.PlcResponseCode_OK, r.responseCode,
+ "iteration %d: %s failed: %v", iteration, r.name, r.err)
+ got[r.name] = flatten(r.value)
+ }
+ }
+ for _, tag := range in {
+ want := make([]bool, 0, tag.tag.Quantity)
+ for i := range int(tag.tag.Quantity) {
+ b := int(tag.tag.Address) + i
+ want = append(want, bits[b/8]&(1<<(b%8)) != 0)
+ }
+ require.Equal(t, want, got[tag.name],
+ "iteration %d: coil tag %s (addr=%d qty=%d) decoded differently through the optimizer",
+ iteration, tag.name, tag.tag.Address, tag.tag.Quantity)
+ }
+ }
+}
+
+func flatten(v apiValues.PlcValue) []bool {
+ if v == nil {
+ return nil
+ }
+ if !v.IsList() {
+ return []bool{v.GetBool()}
+ }
+ out := make([]bool, 0, len(v.GetList()))
+ for _, e := range v.GetList() {
+ out = append(out, e.GetBool())
+ }
+ return out
+}
+
+// ---------------------------------------------------------------------------
+// 8. odd byte lengths + the byte-swapping orders
+// ---------------------------------------------------------------------------
+
+// splitRegisters slices blockData to lengthInBytes, which is ODD for a scalar CHAR and for a
+// STRING of odd declared length. ParseRegisters byte-swaps in place for the two *_BYTE_SWAP
+// orders and can only swap whole pairs, so the trailing byte of an odd slice never moves - while
+// the unoptimized read, which is handed whole registers (2*lengthWords bytes), does move it.
+// Same tag, same device, different value.
+func TestAdvOddByteLengthUnderByteSwap(t *testing.T) {
+ ctx := context.Background()
+ // two registers of a device: 0x0102 0x0304
+ image := []byte{0x01, 0x02, 0x03, 0x04}
+
+ for _, order := range []ByteOrder{BigEndianOrder, LittleEndianOrder, BigEndianByteSwapOrder, LittleEndianByteSwapOrder} {
+ for _, tc := range []struct {
+ name string
+ datatype readWriteModel.ModbusDataType
+ stringLength uint16
+ }{
+ {"CHAR", readWriteModel.ModbusDataType_CHAR, 1},
+ {"STRING(3)", readWriteModel.ModbusDataType_STRING, 3},
+ } {
+ t.Run(order.String()+"/"+tc.name, func(t *testing.T) {
+ tag := newTagFromWireAddress(HoldingRegister, 0, 1, tc.datatype, tc.stringLength, tagConfig{}, false)
+ words, err := tag.lengthWords()
+ require.NoError(t, err)
+ t.Logf("lengthInBytes=%d lengthWords=%d",
+ lengthInBytes(tag.Datatype, tag.Quantity, tag.StringLength), words)
+
+ // what the unoptimized Reader does: hand ParseRegisters whole registers
+ want, err := ParseRegisters(ctx, image[:int(words)*2], tag.Datatype, tag.Quantity, order, tag.StringLength)
+ require.NoError(t, err)
+
+ // what the optimizer does: slice the block to lengthInBytes
+ got := splitRegisters(ctx, blockTag{name: "t", tag: tag, offset: 0}, image, order)
+ require.Equal(t, apiModel.PlcResponseCode_OK, got.responseCode, "%v", got.err)
+
+ assert.Equal(t, fmt.Sprintf("%v", want), fmt.Sprintf("%v", got.value),
+ "DEFECT: merging this tag into a block changes the value it decodes to")
+ })
+ }
+ }
+}
+
+// The same defect reached the way a user would: two CHAR tags a few registers apart on a
+// connection whose default byte order is BIG_ENDIAN_BYTE_SWAP.
+func TestAdvCharTagsMergedIntoABlockDecodeWrong(t *testing.T) {
+ ctx := context.Background()
+ in := named(
+ newTagFromWireAddress(HoldingRegister, 0, 1, readWriteModel.ModbusDataType_CHAR, 1, tagConfig{}, false),
+ newTagFromWireAddress(HoldingRegister, 3, 1, readWriteModel.ModbusDataType_CHAR, 1, tagConfig{}, false),
+ )
+ blocks := optimizeReadsWithSpecLimits(in)
+ require.Len(t, blocks, 1)
+ // registers 0..3 of the device
+ image := []byte{'A', 'a', 'B', 'b', 'C', 'c', 'D', 'd'}
+ for _, r := range splitBlockResponse(ctx, blocks[0], apiModel.PlcResponseCode_OK, image, BigEndianByteSwapOrder) {
+ t.Logf("%s -> %v", r.name, r.value)
+ }
+ // The unoptimized read of tag1 (register 3) byte-swaps 'D','d' to 'd','D' and reads 'd'.
+ results := splitBlockResponse(ctx, blocks[0], apiModel.PlcResponseCode_OK, image, BigEndianByteSwapOrder)
+ // Compare the decoded character, not the PlcValue's display form ("CHAR(8bit):d").
+ assert.Equal(t, "d", results[1].value.GetString(),
+ "a merged CHAR must decode the same byte the unoptimized read would")
+}
diff --git a/plc4go/internal/modbus/ReadOptimizer_test.go b/plc4go/internal/modbus/ReadOptimizer_test.go
new file mode 100644
index 0000000000..00fb74aa0b
--- /dev/null
+++ b/plc4go/internal/modbus/ReadOptimizer_test.go
@@ -0,0 +1,479 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package modbus
+
+import (
+ "context"
+ "fmt"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ apiModel "github.com/apache/plc4x/plc4go/pkg/api/model"
+ apiValues "github.com/apache/plc4x/plc4go/pkg/api/values"
+ readWriteModel "github.com/apache/plc4x/plc4go/protocols/modbus/readwrite/model"
+)
+
+// The cases here are ported from plc4j's ModbusReadOptimizerTest, including the ones that pin down
+// shipped bug fixes - the merged request losing its unit-id (GH-2686) and a coil array answering
+// with only its first element (GH-2060).
+
+// tagAt builds a tag straight from its wire address, the way plc4j's test constructs one. Going
+// through the address parser instead would drag the one-based logical addressing in and make every
+// expectation here one off from the block addresses it is about.
+func tagAt(tagType TagType, address uint16, quantity uint16, datatype readWriteModel.ModbusDataType, config tagConfig) modbusTag {
+ return newTagFromWireAddress(tagType, address, quantity, datatype, 1, config, quantity > 1)
+}
+
+func coilAt(address uint16, quantity uint16) modbusTag {
+ return tagAt(Coil, address, quantity, readWriteModel.ModbusDataType_BOOL, tagConfig{})
+}
+
+func holdingRegisterAt(address uint16, quantity uint16) modbusTag {
+ return tagAt(HoldingRegister, address, quantity, readWriteModel.ModbusDataType_INT, tagConfig{})
+}
+
+func onUnit(unitId uint8) tagConfig {
+ return tagConfig{unitId: &unitId}
+}
+
+func inByteOrder(byteOrder ByteOrder) tagConfig {
+ return tagConfig{byteOrder: &byteOrder}
+}
+
+// named numbers the tags the way plc4j's test does, so that the expectations can name them.
+func named(tags ...modbusTag) []namedTag {
+ namedTags := make([]namedTag, 0, len(tags))
+ for i, tag := range tags {
+ namedTags = append(namedTags, namedTag{name: fmt.Sprintf("tag%d", i), tag: tag})
+ }
+ return namedTags
+}
+
+// expectedBlock is what one block is checked against. The tag names say which tags ended up in it,
+// which is the half of the merging decision the addresses alone don't show.
+type expectedBlock struct {
+ tagType TagType
+ unitId *uint8
+ address uint16
+ quantity uint16
+ tags []string
+}
+
+func unitId(value uint8) *uint8 {
+ return &value
+}
+
+func assertBlocks(t *testing.T, expected []expectedBlock, actual []readBlock) {
+ t.Helper()
+ require.Len(t, actual, len(expected))
+ for i, want := range expected {
+ got := actual[i]
+ assert.Equal(t, want.tagType, got.tagType, "block %d: area", i)
+ assert.Equal(t, want.unitId, got.unitId, "block %d: unit-id", i)
+ assert.Equal(t, want.address, got.address, "block %d: address", i)
+ assert.Equal(t, want.quantity, got.quantity, "block %d: quantity", i)
+ names := make([]string, 0, len(got.tags))
+ for _, member := range got.tags {
+ names = append(names, member.name)
+ assert.Equal(t, member.tag.Address-got.address, member.offset, "block %d: offset of %q", i, member.name)
+ }
+ assert.Equal(t, want.tags, names, "block %d: members", i)
+ }
+}
+
+// A block read covers one contiguous run of coils, and the run is cut to whatever fits into
+// maxCoilQuantity addresses measured from the first tag of the block.
+func TestOptimizeReadsGroupsCoils(t *testing.T) {
+ for _, test := range []struct {
+ name string
+ tags []namedTag
+ blocks []expectedBlock
+ }{
+ {
+ name: "a single coil is read on its own",
+ tags: named(coilAt(0, 1)),
+ blocks: []expectedBlock{{tagType: Coil, address: 0, quantity: 1, tags: []string{"tag0"}}},
+ },
+ {
+ name: "two adjacent coils become one two-coil read",
+ tags: named(coilAt(0, 1), coilAt(1, 1)),
+ blocks: []expectedBlock{{tagType: Coil, address: 0, quantity: 2, tags: []string{"tag0", "tag1"}}},
+ },
+ {
+ // The window width is the gap tolerance, so a gap of 99 coils is merged straight over.
+ name: "two coils with a large gap still become one read",
+ tags: named(coilAt(0, 1), coilAt(100, 1)),
+ blocks: []expectedBlock{{tagType: Coil, address: 0, quantity: 101, tags: []string{"tag0", "tag1"}}},
+ },
+ {
+ name: "two coils further apart than a request reaches are read separately",
+ tags: named(coilAt(0, 1), coilAt(2100, 1)),
+ blocks: []expectedBlock{
+ {tagType: Coil, address: 0, quantity: 1, tags: []string{"tag0"}},
+ {tagType: Coil, address: 2100, quantity: 1, tags: []string{"tag1"}},
+ },
+ },
+ {
+ // The window is anchored at the first tag, not slid along, so a third tag is measured
+ // against the start of its block rather than against its predecessor.
+ name: "the window is measured from the first tag of a block",
+ tags: named(coilAt(0, 1), coilAt(1999, 1), coilAt(2001, 1)),
+ blocks: []expectedBlock{
+ {tagType: Coil, address: 0, quantity: 2000, tags: []string{"tag0", "tag1"}},
+ {tagType: Coil, address: 2001, quantity: 1, tags: []string{"tag2"}},
+ },
+ },
+ {
+ name: "an array occupies one coil per element",
+ tags: named(coilAt(0, 1), coilAt(6, 4)),
+ blocks: []expectedBlock{{tagType: Coil, address: 0, quantity: 10, tags: []string{"tag0", "tag1"}}},
+ },
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ assertBlocks(t, test.blocks, optimizeReadsWithSpecLimits(test.tags))
+ })
+ }
+}
+
+// The register areas are cut the same way, only counted in registers rather than coils.
+func TestOptimizeReadsGroupsRegisters(t *testing.T) {
+ for _, test := range []struct {
+ name string
+ tags []namedTag
+ blocks []expectedBlock
+ }{
+ {
+ name: "a single register is read on its own",
+ tags: named(holdingRegisterAt(0, 1)),
+ blocks: []expectedBlock{{tagType: HoldingRegister, address: 0, quantity: 1, tags: []string{"tag0"}}},
+ },
+ {
+ name: "two adjacent registers become one two-register read",
+ tags: named(holdingRegisterAt(0, 1), holdingRegisterAt(1, 1)),
+ blocks: []expectedBlock{{tagType: HoldingRegister, address: 0, quantity: 2, tags: []string{"tag0", "tag1"}}},
+ },
+ {
+ name: "two registers with a large gap still become one read",
+ tags: named(holdingRegisterAt(0, 1), holdingRegisterAt(100, 1)),
+ blocks: []expectedBlock{{tagType: HoldingRegister, address: 0, quantity: 101, tags: []string{"tag0", "tag1"}}},
+ },
+ {
+ name: "two registers further apart than a request reaches are read separately",
+ tags: named(holdingRegisterAt(0, 1), holdingRegisterAt(2100, 1)),
+ blocks: []expectedBlock{
+ {tagType: HoldingRegister, address: 0, quantity: 1, tags: []string{"tag0"}},
+ {tagType: HoldingRegister, address: 2100, quantity: 1, tags: []string{"tag1"}},
+ },
+ },
+ {
+ // A DINT is two registers wide, so a single one of them spans registers 1 and 2 -
+ // the block has to cover both or the tag cannot be decoded out of it.
+ name: "a wide data type occupies the registers it needs",
+ tags: named(tagAt(HoldingRegister, 1, 1, readWriteModel.ModbusDataType_DINT, tagConfig{})),
+ blocks: []expectedBlock{
+ {tagType: HoldingRegister, address: 1, quantity: 2, tags: []string{"tag0"}},
+ },
+ },
+ {
+ // The tags arrive unsorted; the block is still anchored at the lowest address.
+ name: "tags are sorted by address before they are cut into blocks",
+ tags: named(holdingRegisterAt(5, 1), holdingRegisterAt(1, 1)),
+ blocks: []expectedBlock{
+ {tagType: HoldingRegister, address: 1, quantity: 5, tags: []string{"tag1", "tag0"}},
+ },
+ },
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ assertBlocks(t, test.blocks, optimizeReadsWithSpecLimits(test.tags))
+ })
+ }
+}
+
+// Five areas, addressed independently and read with different function codes, so address 0 means
+// five unrelated things and a block may never span two of them.
+func TestOptimizeReadsNeverMergesAcrossAreas(t *testing.T) {
+ blocks := optimizeReadsWithSpecLimits(named(
+ coilAt(0, 1),
+ tagAt(HoldingRegister, 0, 1, readWriteModel.ModbusDataType_INT, tagConfig{}),
+ tagAt(InputRegister, 0, 1, readWriteModel.ModbusDataType_INT, tagConfig{}),
+ tagAt(ExtendedRegister, 0, 1, readWriteModel.ModbusDataType_INT, tagConfig{}),
+ tagAt(DiscreteInput, 0, 1, readWriteModel.ModbusDataType_BOOL, tagConfig{}),
+ ))
+
+ assertBlocks(t, []expectedBlock{
+ {tagType: Coil, address: 0, quantity: 1, tags: []string{"tag0"}},
+ {tagType: HoldingRegister, address: 0, quantity: 1, tags: []string{"tag1"}},
+ {tagType: InputRegister, address: 0, quantity: 1, tags: []string{"tag2"}},
+ {tagType: ExtendedRegister, address: 0, quantity: 1, tags: []string{"tag3"}},
+ {tagType: DiscreteInput, address: 0, quantity: 1, tags: []string{"tag4"}},
+ }, blocks)
+}
+
+// A block read reaches exactly one unit. Merging tags that address different units would send the
+// request to whichever one won, and dropping the unit-id from the merged request would send it to
+// the connection's default - both of which read the wrong device (plc4j GH-2686).
+func TestOptimizeReadsKeepsUnitsApart(t *testing.T) {
+ for _, test := range []struct {
+ name string
+ tags []namedTag
+ blocks []expectedBlock
+ }{
+ {
+ name: "registers on different units are not merged",
+ tags: named(
+ tagAt(HoldingRegister, 0, 1, readWriteModel.ModbusDataType_INT, onUnit(2)),
+ tagAt(HoldingRegister, 1, 1, readWriteModel.ModbusDataType_INT, onUnit(3)),
+ ),
+ blocks: []expectedBlock{
+ {tagType: HoldingRegister, unitId: unitId(2), address: 0, quantity: 1, tags: []string{"tag0"}},
+ {tagType: HoldingRegister, unitId: unitId(3), address: 1, quantity: 1, tags: []string{"tag1"}},
+ },
+ },
+ {
+ name: "coils on different units are not merged",
+ tags: named(
+ tagAt(Coil, 0, 1, readWriteModel.ModbusDataType_BOOL, onUnit(2)),
+ tagAt(Coil, 1, 1, readWriteModel.ModbusDataType_BOOL, onUnit(3)),
+ ),
+ blocks: []expectedBlock{
+ {tagType: Coil, unitId: unitId(2), address: 0, quantity: 1, tags: []string{"tag0"}},
+ {tagType: Coil, unitId: unitId(3), address: 1, quantity: 1, tags: []string{"tag1"}},
+ },
+ },
+ {
+ name: "tags on the same unit are merged and the merged block keeps the unit-id",
+ tags: named(
+ tagAt(HoldingRegister, 0, 1, readWriteModel.ModbusDataType_INT, onUnit(7)),
+ tagAt(HoldingRegister, 1, 1, readWriteModel.ModbusDataType_INT, onUnit(7)),
+ ),
+ blocks: []expectedBlock{
+ {tagType: HoldingRegister, unitId: unitId(7), address: 0, quantity: 2, tags: []string{"tag0", "tag1"}},
+ },
+ },
+ {
+ // "whatever the connection uses" is not the same statement as a number, so a tag that
+ // named no unit forms its own group even if the connection's default is that number.
+ name: "a tag without a unit-id is not merged with one that has it",
+ tags: named(
+ tagAt(HoldingRegister, 0, 1, readWriteModel.ModbusDataType_INT, tagConfig{}),
+ tagAt(HoldingRegister, 1, 1, readWriteModel.ModbusDataType_INT, onUnit(2)),
+ ),
+ blocks: []expectedBlock{
+ {tagType: HoldingRegister, unitId: nil, address: 0, quantity: 1, tags: []string{"tag0"}},
+ {tagType: HoldingRegister, unitId: unitId(2), address: 1, quantity: 1, tags: []string{"tag1"}},
+ },
+ },
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ assertBlocks(t, test.blocks, optimizeReadsWithSpecLimits(test.tags))
+ })
+ }
+}
+
+// The optimizer only ever merges requests, it never takes one apart: a tag wider than a single
+// request can carry passes through whole rather than being split into halves of a value. Such a
+// tag is rejected when its address is parsed, so it only reaches here from a caller that built it
+// by hand - but silently halving it would be worse than passing it on.
+func TestOptimizeReadsPassesAnOversizedTagThrough(t *testing.T) {
+ blocks := optimizeReadsWithSpecLimits(named(
+ tagAt(HoldingRegister, 0, 200, readWriteModel.ModbusDataType_INT, tagConfig{}),
+ tagAt(HoldingRegister, 300, 1, readWriteModel.ModbusDataType_INT, tagConfig{}),
+ ))
+
+ assertBlocks(t, []expectedBlock{
+ {tagType: HoldingRegister, address: 0, quantity: 200, tags: []string{"tag0"}},
+ {tagType: HoldingRegister, address: 300, quantity: 1, tags: []string{"tag1"}},
+ }, blocks)
+}
+
+// The ceilings are parameters, so a gateway that cannot take a full-width request can be given a
+// narrower one without the grouping changing shape.
+func TestOptimizeReadsHonoursTheGivenLimits(t *testing.T) {
+ tags := named(holdingRegisterAt(0, 1), holdingRegisterAt(10, 1))
+
+ assertBlocks(t, []expectedBlock{
+ {tagType: HoldingRegister, address: 0, quantity: 11, tags: []string{"tag0", "tag1"}},
+ }, optimizeReads(tags, maxCoilQuantity, 20))
+
+ assertBlocks(t, []expectedBlock{
+ {tagType: HoldingRegister, address: 0, quantity: 1, tags: []string{"tag0"}},
+ {tagType: HoldingRegister, address: 10, quantity: 1, tags: []string{"tag1"}},
+ }, optimizeReads(tags, maxCoilQuantity, 5))
+}
+
+func TestOptimizeReadsOnNoTags(t *testing.T) {
+ assert.Empty(t, optimizeReadsWithSpecLimits(nil))
+}
+
+////////////////////////////////////////////////////////////////////////////////////////////////////
+// Splitting the response
+////////////////////////////////////////////////////////////////////////////////////////////////////
+
+// boolsOf unpacks a list of BOOLs. PlcList.GetBoolArray only answers for a list of one, so the
+// elements are read individually, the way plc4j's test walks getIndex(i).
+func boolsOf(value apiValues.PlcValue) []bool {
+ bools := make([]bool, 0, value.GetLength())
+ for _, element := range value.GetList() {
+ bools = append(bools, element.GetBool())
+ }
+ return bools
+}
+
+// splitSingle runs one tag through the optimizer and splits the given payload for the single block
+// it is expected to produce.
+func splitSingle(t *testing.T, tag modbusTag, blockData []byte) tagReadResult {
+ t.Helper()
+ results := splitAll(t, named(tag), blockData, BigEndianOrder)
+ require.Len(t, results, 1)
+ return results[0]
+}
+
+// splitAll runs the given tags through the optimizer and splits the given payload for the single
+// block they are expected to produce.
+func splitAll(t *testing.T, tags []namedTag, blockData []byte, defaultByteOrder ByteOrder) []tagReadResult {
+ t.Helper()
+ blocks := optimizeReadsWithSpecLimits(tags)
+ require.Len(t, blocks, 1)
+ return splitBlockResponse(context.Background(), blocks[0], apiModel.PlcResponseCode_OK, blockData, defaultByteOrder)
+}
+
+// A coil array occupies n coils and the block read covers all of them, so all n values have to
+// come back - answering with the first one used to drop the rest on the floor (plc4j GH-2060).
+func TestSplitBlockResponseReturnsEveryElementOfACoilArray(t *testing.T) {
+ // Eight coils from address 0, packed least significant bit first: 1,0,0,0,1,1,0,1.
+ result := splitSingle(t, coilAt(0, 8), []byte{0b10110001})
+
+ require.Equal(t, apiModel.PlcResponseCode_OK, result.responseCode)
+ require.True(t, result.value.IsList())
+ assert.Equal(t, []bool{true, false, false, false, true, true, false, true}, boolsOf(result.value))
+}
+
+// An array sitting inside the block has to be read from the right bit and keep reading into the
+// following byte when it runs past bit 7.
+func TestSplitBlockResponseReadsACoilArrayAcrossAByteBoundary(t *testing.T) {
+ // The scalar coil at address 0 anchors the block there, so the array at address 6 really does
+ // start at bit 6 of the first byte. Bits 6,7 of byte 0 are 0,1 and bits 0,1 of byte 1 are 0,1.
+ results := splitAll(t, []namedTag{
+ {name: "scalar", tag: coilAt(0, 1)},
+ {name: "array", tag: coilAt(6, 4)},
+ }, []byte{0b10000000, 0b00000010}, BigEndianOrder)
+
+ require.Len(t, results, 2)
+ assert.Equal(t, "array", results[1].name)
+ assert.Equal(t, []bool{false, true, false, true}, boolsOf(results[1].value))
+}
+
+// A single coil keeps reading as a plain BOOL rather than as a list of one, so merging a tag into
+// a block never changes the type the caller sees.
+func TestSplitBlockResponseReturnsAScalarForASingleCoil(t *testing.T) {
+ result := splitSingle(t, coilAt(0, 1), []byte{0b00000001})
+
+ require.Equal(t, apiModel.PlcResponseCode_OK, result.responseCode)
+ assert.False(t, result.value.IsList())
+ assert.True(t, result.value.GetBool())
+}
+
+// A coil holds a single bit, so anything wider has to be reported rather than silently answered
+// with the first bit.
+func TestSplitBlockResponseReportsANonBoolCoilAsUnsupported(t *testing.T) {
+ result := splitSingle(t, tagAt(Coil, 0, 1, readWriteModel.ModbusDataType_INT, tagConfig{}), []byte{0b00000001, 0b00000000})
+
+ assert.Equal(t, apiModel.PlcResponseCode_UNSUPPORTED, result.responseCode)
+ assert.Nil(t, result.value)
+ assert.ErrorContains(t, result.err, "only BOOL is")
+}
+
+// A device answering with fewer coils than it was asked for must fail that tag, not panic.
+func TestSplitBlockResponseOnAShortCoilResponse(t *testing.T) {
+ result := splitSingle(t, coilAt(0, 16), []byte{0b00000001})
+
+ assert.Equal(t, apiModel.PlcResponseCode_INTERNAL_ERROR, result.responseCode)
+ assert.Nil(t, result.value)
+}
+
+// The registers of the block are sliced at the member's own offset, so every tag gets the value it
+// asked for and not the one at the start of the block.
+func TestSplitBlockResponseSlicesRegistersAtTheTagsOffset(t *testing.T) {
+ results := splitAll(t, []namedTag{
+ {name: "first", tag: holdingRegisterAt(0, 1)},
+ {name: "third", tag: holdingRegisterAt(2, 1)},
+ }, []byte{0x00, 0x01, 0x00, 0x02, 0x00, 0x03}, BigEndianOrder)
+
+ require.Len(t, results, 2)
+ assert.Equal(t, apiModel.PlcResponseCode_OK, results[0].responseCode)
+ assert.Equal(t, int16(1), results[0].value.GetInt16())
+ assert.Equal(t, apiModel.PlcResponseCode_OK, results[1].responseCode)
+ assert.Equal(t, int16(3), results[1].value.GetInt16())
+}
+
+// A tag wider than one register is decoded across the registers it occupies.
+func TestSplitBlockResponseDecodesAWideTagFromTheBlock(t *testing.T) {
+ results := splitAll(t, []namedTag{
+ {name: "word", tag: holdingRegisterAt(0, 1)},
+ {name: "double", tag: tagAt(HoldingRegister, 1, 1, readWriteModel.ModbusDataType_DINT, tagConfig{})},
+ }, []byte{0x00, 0x01, 0x00, 0x01, 0x00, 0x00}, BigEndianOrder)
+
+ require.Len(t, results, 2)
+ assert.Equal(t, int32(0x00010000), results[1].value.GetInt32())
+}
+
+// A block may mix byte orders: the order is a property of the tag, and only a tag that declared
+// none falls back to the connection's default.
+func TestSplitBlockResponseAppliesEachTagsOwnByteOrder(t *testing.T) {
+ results := splitAll(t, []namedTag{
+ {name: "default", tag: holdingRegisterAt(0, 1)},
+ {name: "little", tag: tagAt(HoldingRegister, 1, 1, readWriteModel.ModbusDataType_INT, inByteOrder(LittleEndianOrder))},
+ }, []byte{0x00, 0x2A, 0x2A, 0x00}, BigEndianOrder)
+
+ require.Len(t, results, 2)
+ assert.Equal(t, int16(42), results[0].value.GetInt16(), "the connection default applies to a tag without one of its own")
+ assert.Equal(t, int16(42), results[1].value.GetInt16(), "the tag's own order applies to it")
+}
+
+// A device answering with fewer registers than it was asked for must fail the tags that fall off
+// the end rather than reading past the payload.
+func TestSplitBlockResponseOnAShortRegisterResponse(t *testing.T) {
+ results := splitAll(t, []namedTag{
+ {name: "present", tag: holdingRegisterAt(0, 1)},
+ {name: "missing", tag: holdingRegisterAt(1, 1)},
+ }, []byte{0x00, 0x01}, BigEndianOrder)
+
+ require.Len(t, results, 2)
+ assert.Equal(t, apiModel.PlcResponseCode_OK, results[0].responseCode)
+ assert.Equal(t, apiModel.PlcResponseCode_INTERNAL_ERROR, results[1].responseCode)
+ assert.Nil(t, results[1].value)
+}
+
+// Merging reads means a device that rejects one address takes the whole block down with it, so
+// every member is answered with the block's own code.
+func TestSplitBlockResponseFailsEveryTagOfAFailedBlock(t *testing.T) {
+ blocks := optimizeReadsWithSpecLimits(named(holdingRegisterAt(0, 1), holdingRegisterAt(1, 1)))
+ require.Len(t, blocks, 1)
+
+ results := splitBlockResponse(context.Background(), blocks[0], apiModel.PlcResponseCode_REMOTE_ERROR, nil, BigEndianOrder)
+
+ require.Len(t, results, 2)
+ for _, result := range results {
+ assert.Equal(t, apiModel.PlcResponseCode_REMOTE_ERROR, result.responseCode, result.name)
+ assert.Nil(t, result.value, result.name)
+ }
+}