fix: reading multiple holding registers in modbus input plugin (#8628)

plugins/inputs/modbus/modbus.go

@@ -217,23 +217,33 @@ func (m *Modbus) InitRegister(fields []fieldContainer, name string) error {
 	sort.Slice(addrs, func(i, j int) bool { return addrs[i] < addrs[j] })
 
 	ii := 0
+	maxQuantity := 1
 	var registersRange []registerRange
+	if name == cDiscreteInputs || name == cCoils {
+		maxQuantity = 2000
+	} else if name == cInputRegisters || name == cHoldingRegisters {
+		maxQuantity = 125
+	}
 
 	// Get range of consecutive integers
 	// [1, 2, 3, 5, 6, 10, 11, 12, 14]
 	// (1, 3) , (5, 2) , (10, 3), (14 , 1)
 	for range addrs {
-		if ii < len(addrs) {
+		if ii >= len(addrs) {
-			start := addrs[ii]
+			break
-			end := start
-
-			for ii < len(addrs)-1 && addrs[ii+1]-addrs[ii] == 1 {
-				end = addrs[ii+1]
-				ii++
-			}
-			ii++
-			registersRange = append(registersRange, registerRange{start, end - start + 1})
 		}
+		quantity := 1
+		start := addrs[ii]
+		end := start
+
+		for ii < len(addrs)-1 && addrs[ii+1]-addrs[ii] == 1 && quantity < maxQuantity {
+			end = addrs[ii+1]
+			ii++
+			quantity++
+		}
+		ii++
+
+		registersRange = append(registersRange, registerRange{start, end - start + 1})
 	}
 
 	m.registers = append(m.registers, register{name, registersRange, fields})
@@ -434,7 +444,7 @@ func (m *Modbus) getFields() error {
 					for bitPosition := 0; bitPosition < 8; bitPosition++ {
 						bitRawValues[address] = getBitValue(readValue, bitPosition)
 						address = address + 1
-						if address+1 > rr.length {
+						if address > rr.address+rr.length {
 							break
 						}
 					}

plugins/inputs/modbus/modbus_test.go

@@ -1,6 +1,7 @@
 package modbus
 
 import (
+	"fmt"
 	"testing"
 
 	m "github.com/goburrow/modbus"
@@ -657,6 +658,102 @@ func TestHoldingRegisters(t *testing.T) {
 	}
 }
 
+func TestReadMultipleCoilLimit(t *testing.T) {
+	serv := mbserver.NewServer()
+	err := serv.ListenTCP("localhost:1502")
+	assert.NoError(t, err)
+	defer serv.Close()
+
+	handler := m.NewTCPClientHandler("localhost:1502")
+	err = handler.Connect()
+	assert.NoError(t, err)
+	defer handler.Close()
+	client := m.NewClient(handler)
+
+	fcs := []fieldContainer{}
+	writeValue := uint16(0)
+	for i := 0; i <= 4000; i++ {
+		fc := fieldContainer{}
+		fc.Name = fmt.Sprintf("coil-%v", i)
+		fc.Address = []uint16{uint16(i)}
+		fcs = append(fcs, fc)
+
+		t.Run(fc.Name, func(t *testing.T) {
+			_, err = client.WriteSingleCoil(fc.Address[0], writeValue)
+			assert.NoError(t, err)
+		})
+
+		writeValue = 65280 - writeValue
+	}
+
+	modbus := Modbus{
+		Name:       "TestReadCoils",
+		Controller: "tcp://localhost:1502",
+		SlaveID:    1,
+		Coils:      fcs,
+	}
+
+	err = modbus.Init()
+	assert.NoError(t, err)
+	var acc testutil.Accumulator
+	err = modbus.Gather(&acc)
+	assert.NoError(t, err)
+
+	writeValue = 0
+	for i := 0; i <= 4000; i++ {
+		t.Run(modbus.registers[0].Fields[i].Name, func(t *testing.T) {
+			assert.Equal(t, writeValue, modbus.registers[0].Fields[i].value)
+			writeValue = 1 - writeValue
+		})
+	}
+}
+
+func TestReadMultipleHoldingRegisterLimit(t *testing.T) {
+	serv := mbserver.NewServer()
+	err := serv.ListenTCP("localhost:1502")
+	assert.NoError(t, err)
+	defer serv.Close()
+
+	handler := m.NewTCPClientHandler("localhost:1502")
+	err = handler.Connect()
+	assert.NoError(t, err)
+	defer handler.Close()
+	client := m.NewClient(handler)
+
+	fcs := []fieldContainer{}
+	for i := 0; i <= 400; i++ {
+		fc := fieldContainer{}
+		fc.Name = fmt.Sprintf("HoldingRegister-%v", i)
+		fc.ByteOrder = "AB"
+		fc.DataType = "INT16"
+		fc.Scale = 1.0
+		fc.Address = []uint16{uint16(i)}
+		fcs = append(fcs, fc)
+
+		t.Run(fc.Name, func(t *testing.T) {
+			_, err = client.WriteSingleRegister(fc.Address[0], uint16(i))
+			assert.NoError(t, err)
+		})
+	}
+
+	modbus := Modbus{
+		Name:             "TestHoldingRegister",
+		Controller:       "tcp://localhost:1502",
+		SlaveID:          1,
+		HoldingRegisters: fcs,
+	}
+
+	err = modbus.Init()
+	assert.NoError(t, err)
+	var acc testutil.Accumulator
+	err = modbus.Gather(&acc)
+	assert.NoError(t, err)
+
+	for i := 0; i <= 400; i++ {
+		assert.Equal(t, int16(i), modbus.registers[0].Fields[i].value)
+	}
+}
+
 func TestRetrySuccessful(t *testing.T) {
 	retries := 0
 	maxretries := 2