diff --git a/plugins/inputs/modbus/README.md b/plugins/inputs/modbus/README.md index daef7fe92..1180d462a 100644 --- a/plugins/inputs/modbus/README.md +++ b/plugins/inputs/modbus/README.md @@ -138,6 +138,19 @@ See the [CONFIGURATION.md][CONFIGURATION.md] for more details. ## Can be overriden by the individual field definitions. Defaults to "modbus" # measurement = "modbus" + ## Request optimization algorithm. + ## |---none -- Do not perform any optimization and use the given layout(default) + ## |---shrink -- Shrink requests to actually requested fields + ## | by stripping leading and trailing omits + ## |---rearrange -- Rearrange request boundaries within consecutive address ranges + ## | to reduce the number of requested registers by keeping + ## | the number of requests. + ## |---aggressive -- Rearrange request boundaries similar to "rearrange" but + ## allow to request registers not specified by the user to + ## fill gaps. This usually reduces the number of requests at the + ## cost of more requested registers. + # optimization = "none" + ## Field definitions ## Analog Variables, Input Registers and Holding Registers ## address - address of the register to query. For coil and discrete inputs this is the bit address. @@ -250,7 +263,7 @@ configuration for a single slave-device. The field `data_type` defines the representation of the data value on input from the modbus registers. The input values are then converted from the given -`data_type` to a type that is apropriate when sending the value to the output +`data_type` to a type that is appropriate when sending the value to the output plugin. These output types are usually one of string, integer or floating-point-number. The size of the output type is assumed to be large enough for all supported input types. The mapping from the input type to the output @@ -289,7 +302,7 @@ conversion from unsigned values). ### `request` configuration style -This sytle can be used to specify the modbus requests directly. It enables +This style can be used to specify the modbus requests directly. It enables specifying multiple `[[inputs.modbus.request]]` sections including multiple slave-devices. This way, _modbus_ gateway devices can be queried. Please note that _requests_ might be split for non-consecutive addresses. If you want to @@ -321,6 +334,49 @@ using the `measurement` setting. If the setting is omitted `modbus` is used. Furthermore, the measurement value can be overridden by each field individually. +#### Optimization setting + +__Please only use request optimization if you do understand the implications!__ +The `optimization` setting can be used to optimize the actual requests sent to +the device. The following algorithms are available + +##### `none` (_default_) + +Do not perform any optimization. Please note that the requests are still obeying +the maximum request sizes. Furthermore, completely empty requests, i.e. all +fields specify `omit=true`, are removed. Otherwise, the requests are sent as +specified by the user including request of omitted fields. This setting should +be used if you want full control over the requests e.g. to accommodate for +device constraints. + +##### `shrink` + +This optimization allows to remove leading and trailing fields from requests if +those fields are omitted. This can shrink the request number and sizes in cases +where you specify large amounts of omitted fields, e.g. for documentation +purposes. + +##### `rearrange` + +Requests are processed similar to `shrink` but the request boundaries are +rearranged such that usually less registers are being read while keeping the +number of requests. This optimization algorithm only works on consecutive +address ranges and respects user-defined gaps in the field addresses. + +__Please note:__ This optimization might take long in case of many +non-consecutive, non-omitted fields! + +##### `aggressive` + +Requests are processed similar to `rearrange` but user-defined gaps in the field +addresses are filled automatically. This usually reduces the number of requests, +but will increase the number of registers read due to larger requests. +This algorithm might be useful if you only want to specify the fields you are +interested in but want to minimize the number of requests sent to the device. + +__Please note:__ This optimization might take long in case of many +non-consecutive, non-omitted fields! + #### Field definitions Each `request` can contain a list of fields to collect from the modbus device. @@ -328,8 +384,8 @@ Each `request` can contain a list of fields to collect from the modbus device. ##### address A field is identified by an `address` that reflects the modbus register -address. You can usually find the address values for the different datapoints in -the datasheet of your modbus device. This is a mandatory setting. +address. You can usually find the address values for the different data-points +in the datasheet of your modbus device. This is a mandatory setting. For _coil_ and _discrete input_ registers this setting specifies the __bit__ containing the value of the field. @@ -474,5 +530,5 @@ an issue or submit a pull-request. ```sh $ ./telegraf -config telegraf.conf -input-filter modbus -test -modbus.InputRegisters,host=orangepizero Current=0,Energy=0,Frecuency=60,Power=0,PowerFactor=0,Voltage=123.9000015258789 1554079521000000000 +modbus.InputRegisters,host=orangepizero Current=0,Energy=0,Frequency=60,Power=0,PowerFactor=0,Voltage=123.9000015258789 1554079521000000000 ``` diff --git a/plugins/inputs/modbus/configuration_register.go b/plugins/inputs/modbus/configuration_register.go index 34e672145..187e322d2 100644 --- a/plugins/inputs/modbus/configuration_register.go +++ b/plugins/inputs/modbus/configuration_register.go @@ -81,7 +81,7 @@ func (c *ConfigurationOriginal) initRequests(fieldDefs []fieldDefinition, maxQua if err != nil { return nil, err } - return groupFieldsToRequests(fields, nil, maxQuantity), nil + return groupFieldsToRequests(fields, nil, maxQuantity, "none"), nil } func (c *ConfigurationOriginal) initFields(fieldDefs []fieldDefinition) ([]field, error) { diff --git a/plugins/inputs/modbus/configuration_request.go b/plugins/inputs/modbus/configuration_request.go index 895e20d22..8e3a468c7 100644 --- a/plugins/inputs/modbus/configuration_request.go +++ b/plugins/inputs/modbus/configuration_request.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "hash/maphash" + + "github.com/influxdata/telegraf/internal/choice" ) //go:embed sample_request.conf @@ -25,6 +27,7 @@ type requestDefinition struct { ByteOrder string `toml:"byte_order"` RegisterType string `toml:"register"` Measurement string `toml:"measurement"` + Optimization string `toml:"optimization"` Fields []requestFieldDefinition `toml:"fields"` Tags map[string]string `toml:"tags"` } @@ -42,6 +45,12 @@ func (c *ConfigurationPerRequest) Check() error { seenFields := make(map[uint64]bool) for _, def := range c.Requests { + // Check for valid optimization + validOptimizations := []string{"", "none", "shrink", "rearrange", "aggressive"} + if !choice.Contains(def.Optimization, validOptimizations) { + return fmt.Errorf("unknown optimization %q", def.Optimization) + } + // Check byte order of the data switch def.ByteOrder { case "": @@ -153,16 +162,16 @@ func (c *ConfigurationPerRequest) Process() (map[byte]requestSet, error) { switch def.RegisterType { case "coil": - requests := groupFieldsToRequests(fields, def.Tags, maxQuantityCoils) + requests := groupFieldsToRequests(fields, def.Tags, maxQuantityCoils, def.Optimization) set.coil = append(set.coil, requests...) case "discrete": - requests := groupFieldsToRequests(fields, def.Tags, maxQuantityDiscreteInput) + requests := groupFieldsToRequests(fields, def.Tags, maxQuantityDiscreteInput, def.Optimization) set.discrete = append(set.discrete, requests...) case "holding": - requests := groupFieldsToRequests(fields, def.Tags, maxQuantityHoldingRegisters) + requests := groupFieldsToRequests(fields, def.Tags, maxQuantityHoldingRegisters, def.Optimization) set.holding = append(set.holding, requests...) case "input": - requests := groupFieldsToRequests(fields, def.Tags, maxQuantityInputRegisters) + requests := groupFieldsToRequests(fields, def.Tags, maxQuantityInputRegisters, def.Optimization) set.input = append(set.input, requests...) default: return nil, fmt.Errorf("unknown register type %q", def.RegisterType) diff --git a/plugins/inputs/modbus/modbus.go b/plugins/inputs/modbus/modbus.go index ecbb58657..ac45aab6f 100644 --- a/plugins/inputs/modbus/modbus.go +++ b/plugins/inputs/modbus/modbus.go @@ -122,12 +122,12 @@ func (m *Modbus) Init() error { // Check and process the configuration if err := cfg.Check(); err != nil { - return fmt.Errorf("configuraton invalid: %v", err) + return fmt.Errorf("configuration invalid: %v", err) } r, err := cfg.Process() if err != nil { - return fmt.Errorf("cannot process configuraton: %v", err) + return fmt.Errorf("cannot process configuration: %v", err) } m.requests = r diff --git a/plugins/inputs/modbus/modbus_test.go b/plugins/inputs/modbus/modbus_test.go index 09ba1e5e5..f67b0fb35 100644 --- a/plugins/inputs/modbus/modbus_test.go +++ b/plugins/inputs/modbus/modbus_test.go @@ -1494,7 +1494,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: empty field name in request for slave 1", + errormsg: "configuration invalid: empty field name in request for slave 1", }, { name: "invalid byte-order (coil)", @@ -1506,7 +1506,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { Fields: []requestFieldDefinition{}, }, }, - errormsg: "configuraton invalid: unknown byte-order \"AB\"", + errormsg: "configuration invalid: unknown byte-order \"AB\"", }, { name: "duplicate fields (coil)", @@ -1527,7 +1527,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"coil-0\" duplicated in measurement \"modbus\" (slave 1/\"coil\")", + errormsg: "configuration invalid: field \"coil-0\" duplicated in measurement \"modbus\" (slave 1/\"coil\")", }, { name: "duplicate fields multiple requests (coil)", @@ -1557,7 +1557,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"coil-0\" duplicated in measurement \"foo\" (slave 1/\"coil\")", + errormsg: "configuration invalid: field \"coil-0\" duplicated in measurement \"foo\" (slave 1/\"coil\")", }, { name: "invalid byte-order (discrete)", @@ -1569,7 +1569,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { Fields: []requestFieldDefinition{}, }, }, - errormsg: "configuraton invalid: unknown byte-order \"AB\"", + errormsg: "configuration invalid: unknown byte-order \"AB\"", }, { name: "duplicate fields (discrete)", @@ -1590,7 +1590,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"discrete-0\" duplicated in measurement \"modbus\" (slave 1/\"discrete\")", + errormsg: "configuration invalid: field \"discrete-0\" duplicated in measurement \"modbus\" (slave 1/\"discrete\")", }, { name: "duplicate fields multiple requests (discrete)", @@ -1620,7 +1620,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"discrete-0\" duplicated in measurement \"foo\" (slave 1/\"discrete\")", + errormsg: "configuration invalid: field \"discrete-0\" duplicated in measurement \"foo\" (slave 1/\"discrete\")", }, { name: "invalid byte-order (holding)", @@ -1632,7 +1632,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { Fields: []requestFieldDefinition{}, }, }, - errormsg: "configuraton invalid: unknown byte-order \"AB\"", + errormsg: "configuration invalid: unknown byte-order \"AB\"", }, { name: "invalid field name (holding)", @@ -1647,7 +1647,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: empty field name in request for slave 1", + errormsg: "configuration invalid: empty field name in request for slave 1", }, { name: "invalid field input type (holding)", @@ -1663,7 +1663,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "cannot process configuraton: initializing field \"holding-0\" failed: invalid input datatype \"\" for determining field length", + errormsg: "cannot process configuration: initializing field \"holding-0\" failed: invalid input datatype \"\" for determining field length", }, { name: "invalid field output type (holding)", @@ -1681,7 +1681,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "cannot process configuraton: initializing field \"holding-0\" failed: unknown output type \"UINT8\"", + errormsg: "cannot process configuration: initializing field \"holding-0\" failed: unknown output type \"UINT8\"", }, { name: "duplicate fields (holding)", @@ -1702,7 +1702,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"holding-0\" duplicated in measurement \"modbus\" (slave 1/\"holding\")", + errormsg: "configuration invalid: field \"holding-0\" duplicated in measurement \"modbus\" (slave 1/\"holding\")", }, { name: "duplicate fields multiple requests (holding)", @@ -1732,7 +1732,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"holding-0\" duplicated in measurement \"foo\" (slave 1/\"holding\")", + errormsg: "configuration invalid: field \"holding-0\" duplicated in measurement \"foo\" (slave 1/\"holding\")", }, { name: "invalid byte-order (input)", @@ -1744,7 +1744,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { Fields: []requestFieldDefinition{}, }, }, - errormsg: "configuraton invalid: unknown byte-order \"AB\"", + errormsg: "configuration invalid: unknown byte-order \"AB\"", }, { name: "invalid field name (input)", @@ -1759,7 +1759,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: empty field name in request for slave 1", + errormsg: "configuration invalid: empty field name in request for slave 1", }, { name: "invalid field input type (input)", @@ -1775,7 +1775,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "cannot process configuraton: initializing field \"input-0\" failed: invalid input datatype \"\" for determining field length", + errormsg: "cannot process configuration: initializing field \"input-0\" failed: invalid input datatype \"\" for determining field length", }, { name: "invalid field output type (input)", @@ -1793,7 +1793,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "cannot process configuraton: initializing field \"input-0\" failed: unknown output type \"UINT8\"", + errormsg: "cannot process configuration: initializing field \"input-0\" failed: unknown output type \"UINT8\"", }, { name: "duplicate fields (input)", @@ -1814,7 +1814,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"input-0\" duplicated in measurement \"modbus\" (slave 1/\"input\")", + errormsg: "configuration invalid: field \"input-0\" duplicated in measurement \"modbus\" (slave 1/\"input\")", }, { name: "duplicate fields multiple requests (input)", @@ -1844,7 +1844,7 @@ func TestConfigurationPerRequestFail(t *testing.T) { }, }, }, - errormsg: "configuraton invalid: field \"input-0\" duplicated in measurement \"foo\" (slave 1/\"input\")", + errormsg: "configuration invalid: field \"input-0\" duplicated in measurement \"foo\" (slave 1/\"input\")", }, } @@ -1949,7 +1949,7 @@ func TestRequestsEmptyFields(t *testing.T) { }, } err := modbus.Init() - require.EqualError(t, err, `configuraton invalid: found request section without fields`) + require.EqualError(t, err, `configuration invalid: found request section without fields`) } func TestMultipleSlavesOneFail(t *testing.T) { @@ -2051,6 +2051,7 @@ func TestMultipleSlavesOneFail(t *testing.T) { require.Len(t, acc.Errors, 1) require.EqualError(t, acc.FirstError(), "slave 2: modbus: exception '11' (gateway target device failed to respond), function '131'") } + func TestCases(t *testing.T) { // Get all directories in testdata folders, err := os.ReadDir("testcases") @@ -2170,3 +2171,697 @@ func TestCases(t *testing.T) { }) } } + +type rangeDefinition struct { + start uint16 + count uint16 + increment uint16 + length uint16 + dtype string + omit bool +} + +type requestExpectation struct { + fields []rangeDefinition + req request +} + +func generateRequestDefinitions(ranges []rangeDefinition) []requestFieldDefinition { + var fields []requestFieldDefinition + + id := 0 + for _, r := range ranges { + if r.increment == 0 { + r.increment = r.length + } + for i := uint16(0); i < r.count; i++ { + f := requestFieldDefinition{ + Name: fmt.Sprintf("holding-%d", id), + Address: r.start + i*r.increment, + InputType: r.dtype, + Omit: r.omit, + } + fields = append(fields, f) + id++ + } + } + return fields +} + +func generateExpectation(defs []requestExpectation) []request { + var requests []request + + for _, def := range defs { + r := def.req + r.fields = make([]field, 0) + for _, d := range def.fields { + if d.increment == 0 { + d.increment = d.length + } + for i := uint16(0); i < d.count; i++ { + f := field{ + address: d.start + i*d.increment, + length: d.length, + } + r.fields = append(r.fields, f) + } + } + requests = append(requests, r) + } + return requests +} + +func requireEqualRequests(t *testing.T, expected, actual []request) { + require.Equal(t, len(expected), len(actual), "request size mismatch") + + for i, e := range expected { + a := actual[i] + require.Equalf(t, e.address, a.address, "address mismatch in request %d", i) + require.Equalf(t, e.length, a.length, "length mismatch in request %d", i) + require.Equalf(t, len(e.fields), len(a.fields), "no. fields mismatch in request %d", i) + for j, ef := range e.fields { + af := a.fields[j] + require.Equalf(t, ef.address, af.address, "address mismatch in field %d of request %d", j, i) + require.Equalf(t, ef.length, af.length, "length mismatch in field %d of request %d", j, i) + } + } +} + +func TestRequestOptimizationShrink(t *testing.T) { + maxsize := maxQuantityHoldingRegisters + tests := []struct { + name string + inputs []rangeDefinition + expected []requestExpectation + }{ + { + name: "no omit", + inputs: []rangeDefinition{ + {0, 2 * maxQuantityHoldingRegisters, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 0, count: maxsize, length: 1}}, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{{start: maxsize, count: maxsize, length: 1}}, + req: request{address: maxsize, length: maxsize}, + }, + }, + }, + { + name: "borders", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, maxsize - 2, 1, 1, "INT16", true}, + {maxsize - 1, 2, 1, 1, "INT16", false}, + {maxsize + 1, maxsize - 2, 1, 1, "INT16", true}, + {2*maxsize - 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: maxsize - 1, count: 1, length: 1}, + }, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{ + {start: maxsize, count: 1, length: 1}, + {start: 2*maxsize - 1, count: 1, length: 1}, + }, + req: request{address: maxsize, length: maxsize}, + }, + }, + }, + { + name: "borders with gap", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, maxsize - 2, 1, 1, "INT16", true}, + {maxsize - 1, 2, 1, 1, "INT16", false}, + {maxsize + 1, 4, 1, 1, "INT16", true}, + {2*maxsize - 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: maxsize - 1, count: 1, length: 1}, + }, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{{start: maxsize, count: 1, length: 1}}, + req: request{address: maxsize, length: 1}, + }, + { + fields: []rangeDefinition{{start: 2*maxsize - 1, count: 1, length: 1}}, + req: request{address: 2*maxsize - 1, length: 1}, + }, + }, + }, + { + name: "large gaps", + inputs: []rangeDefinition{ + {18, 3, 1, 1, "INT16", false}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 18, count: 3, length: 1}}, + req: request{address: 18, length: 3}, + }, + { + fields: []rangeDefinition{{start: maxsize - 2, count: 5, length: 1}}, + req: request{address: maxsize - 2, length: 5}, + }, + { + fields: []rangeDefinition{{start: maxsize + 42, count: 2, length: 1}}, + req: request{address: maxsize + 42, length: 2}, + }, + }, + }, + { + name: "large gaps filled", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, 17, 1, 1, "INT16", true}, + {18, 3, 1, 1, "INT16", false}, + {21, maxsize - 23, 1, 1, "INT16", true}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 3, 39, 1, 1, "INT16", true}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: 18, count: 3, length: 1}, + {start: maxsize - 2, count: 2, length: 1}, + }, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{ + {start: maxsize, count: 3, length: 1}, + {start: maxsize + 42, count: 2, length: 1}, + }, + req: request{address: maxsize, length: 44}, + }, + }, + }, + { + name: "large gaps filled with offset", + inputs: []rangeDefinition{ + {18, 3, 1, 1, "INT16", false}, + {21, maxsize - 23, 1, 1, "INT16", true}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 3, 39, 1, 1, "INT16", true}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 18, count: 3, length: 1}, + {start: maxsize - 2, count: 5, length: 1}, + }, + req: request{address: 18, length: 110}, + }, + { + fields: []rangeDefinition{{start: maxsize + 42, count: 2, length: 1}}, + req: request{address: maxsize + 42, length: 2}, + }, + }, + }, + { + name: "worst case", + inputs: []rangeDefinition{ + {0, maxsize, 2, 1, "INT16", false}, + {1, maxsize, 2, 1, "INT16", true}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 0, count: maxsize/2 + 1, increment: 2, length: 1}}, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{{start: maxsize + 1, count: maxsize / 2, increment: 2, length: 1}}, + req: request{address: maxsize + 1, length: maxsize - 2}, + }, + }, + }, + { + name: "from PR #11106", + inputs: []rangeDefinition{ + {0, 2, 1, 1, "INT16", true}, + {2, 1, 1, 1, "INT16", false}, + {3, 2*maxsize + 1, 1, 1, "INT16", true}, + {2*maxsize + 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 2, count: 1, length: 1}}, + req: request{address: 2, length: 1}, + }, + { + fields: []rangeDefinition{{start: 2*maxsize + 1, count: 1, length: 1}}, + req: request{address: 2*maxsize + 1, length: 1}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Generate the input structure and the expectation + requestFields := generateRequestDefinitions(tt.inputs) + expected := generateExpectation(tt.expected) + + // Setup the plugin + slaveID := byte(1) + plugin := Modbus{ + Name: "Test", + Controller: "tcp://localhost:1502", + ConfigurationType: "request", + Log: testutil.Logger{}, + } + plugin.Requests = []requestDefinition{ + { + SlaveID: slaveID, + ByteOrder: "ABCD", + RegisterType: "holding", + Optimization: "shrink", + Fields: requestFields, + }, + } + require.NoError(t, plugin.Init()) + require.NotEmpty(t, plugin.requests) + require.Contains(t, plugin.requests, slaveID) + requireEqualRequests(t, expected, plugin.requests[slaveID].holding) + }) + } +} + +func TestRequestOptimizationRearrange(t *testing.T) { + maxsize := maxQuantityHoldingRegisters + tests := []struct { + name string + inputs []rangeDefinition + expected []requestExpectation + }{ + { + name: "no omit", + inputs: []rangeDefinition{ + {0, 2 * maxQuantityHoldingRegisters, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 0, count: maxsize, length: 1}}, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{{start: maxsize, count: maxsize, length: 1}}, + req: request{address: maxsize, length: maxsize}, + }, + }, + }, + { + name: "borders", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, maxsize - 2, 1, 1, "INT16", true}, + {maxsize - 1, 2, 1, 1, "INT16", false}, + {maxsize + 1, maxsize - 2, 1, 1, "INT16", true}, + {2*maxsize - 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: maxsize - 1, count: 1, length: 1}, + }, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{ + {start: maxsize, count: 1, length: 1}, + {start: 2*maxsize - 1, count: 1, length: 1}, + }, + req: request{address: maxsize, length: maxsize}, + }, + }, + }, + { + name: "borders with gap", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, maxsize - 2, 1, 1, "INT16", true}, + {maxsize - 1, 2, 1, 1, "INT16", false}, + {maxsize + 1, 4, 1, 1, "INT16", true}, + {2*maxsize - 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 0, count: 1, length: 1}}, + req: request{address: 0, length: 1}, + }, + { + fields: []rangeDefinition{ + {start: maxsize - 1, count: 1, length: 1}, + {start: maxsize, count: 1, length: 1}, + }, + req: request{address: maxsize - 1, length: 2}, + }, + { + fields: []rangeDefinition{{start: 2*maxsize - 1, count: 1, length: 1}}, + req: request{address: 2*maxsize - 1, length: 1}, + }, + }, + }, + { + name: "large gaps", + inputs: []rangeDefinition{ + {18, 3, 1, 1, "INT16", false}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 18, count: 3, length: 1}}, + req: request{address: 18, length: 3}, + }, + { + fields: []rangeDefinition{{start: maxsize - 2, count: 5, length: 1}}, + req: request{address: maxsize - 2, length: 5}, + }, + { + fields: []rangeDefinition{{start: maxsize + 42, count: 2, length: 1}}, + req: request{address: maxsize + 42, length: 2}, + }, + }, + }, + { + name: "large gaps filled", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, 17, 1, 1, "INT16", true}, + {18, 3, 1, 1, "INT16", false}, + {21, maxsize - 23, 1, 1, "INT16", true}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 3, 39, 1, 1, "INT16", true}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: 18, count: 3, length: 1}, + }, + req: request{address: 0, length: 21}, + }, + { + fields: []rangeDefinition{ + {start: maxsize - 2, count: 5, length: 1}, + {start: maxsize + 42, count: 2, length: 1}, + }, + req: request{address: maxsize - 2, length: 46}, + }, + }, + }, + { + name: "large gaps filled with offset", + inputs: []rangeDefinition{ + {18, 3, 1, 1, "INT16", false}, + {21, maxsize - 23, 1, 1, "INT16", true}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 3, 39, 1, 1, "INT16", true}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 18, count: 3, length: 1}}, + req: request{address: 18, length: 3}, + }, + { + fields: []rangeDefinition{ + {start: maxsize - 2, count: 5, length: 1}, + {start: maxsize + 42, count: 2, length: 1}, + }, + req: request{address: maxsize - 2, length: 46}, + }, + }, + }, + { + name: "from PR #11106", + inputs: []rangeDefinition{ + {0, 2, 1, 1, "INT16", true}, + {2, 1, 1, 1, "INT16", false}, + {3, 2*maxsize + 1, 1, 1, "INT16", true}, + {2*maxsize + 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 2, count: 1, length: 1}}, + req: request{address: 2, length: 1}, + }, + { + fields: []rangeDefinition{{start: 2*maxsize + 1, count: 1, length: 1}}, + req: request{address: 2*maxsize + 1, length: 1}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Generate the input structure and the expectation + requestFields := generateRequestDefinitions(tt.inputs) + expected := generateExpectation(tt.expected) + + // Setup the plugin + slaveID := byte(1) + plugin := Modbus{ + Name: "Test", + Controller: "tcp://localhost:1502", + ConfigurationType: "request", + Log: testutil.Logger{}, + } + plugin.Requests = []requestDefinition{ + { + SlaveID: slaveID, + ByteOrder: "ABCD", + RegisterType: "holding", + Optimization: "rearrange", + Fields: requestFields, + }, + } + require.NoError(t, plugin.Init()) + require.NotEmpty(t, plugin.requests) + require.Contains(t, plugin.requests, slaveID) + requireEqualRequests(t, expected, plugin.requests[slaveID].holding) + }) + } +} + +func TestRequestOptimizationAggressive(t *testing.T) { + maxsize := maxQuantityHoldingRegisters + tests := []struct { + name string + inputs []rangeDefinition + expected []requestExpectation + }{ + { + name: "no omit", + inputs: []rangeDefinition{ + {0, 2 * maxQuantityHoldingRegisters, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 0, count: maxsize, length: 1}}, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{{start: maxsize, count: maxsize, length: 1}}, + req: request{address: maxsize, length: maxsize}, + }, + }, + }, + { + name: "borders", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, maxsize - 2, 1, 1, "INT16", true}, + {maxsize - 1, 2, 1, 1, "INT16", false}, + {maxsize + 1, maxsize - 2, 1, 1, "INT16", true}, + {2*maxsize - 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: maxsize - 1, count: 1, length: 1}, + }, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{ + {start: maxsize, count: 1, length: 1}, + {start: 2*maxsize - 1, count: 1, length: 1}, + }, + req: request{address: maxsize, length: maxsize}, + }, + }, + }, + { + name: "borders with gap", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, maxsize - 2, 1, 1, "INT16", true}, + {maxsize - 1, 2, 1, 1, "INT16", false}, + {maxsize + 1, 4, 1, 1, "INT16", true}, + {2*maxsize - 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: maxsize - 1, count: 1, length: 1}, + }, + req: request{address: 0, length: maxsize}, + }, + { + fields: []rangeDefinition{ + {start: maxsize, count: 1, length: 1}, + {start: 2*maxsize - 1, count: 1, length: 1}, + }, + req: request{address: maxsize, length: maxsize}, + }, + }, + }, + { + name: "large gaps", + inputs: []rangeDefinition{ + {18, 3, 1, 1, "INT16", false}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 18, count: 3, length: 1}}, + req: request{address: 18, length: 3}, + }, + { + fields: []rangeDefinition{ + {start: maxsize - 2, count: 5, length: 1}, + {start: maxsize + 42, count: 2, length: 1}, + }, + req: request{address: maxsize - 2, length: 46}, + }, + }, + }, + { + name: "large gaps filled", + inputs: []rangeDefinition{ + {0, 1, 1, 1, "INT16", false}, + {1, 17, 1, 1, "INT16", true}, + {18, 3, 1, 1, "INT16", false}, + {21, maxsize - 23, 1, 1, "INT16", true}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 3, 39, 1, 1, "INT16", true}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{ + {start: 0, count: 1, length: 1}, + {start: 18, count: 3, length: 1}, + }, + req: request{address: 0, length: 21}, + }, + { + fields: []rangeDefinition{ + {start: maxsize - 2, count: 5, length: 1}, + {start: maxsize + 42, count: 2, length: 1}, + }, + req: request{address: maxsize - 2, length: 46}, + }, + }, + }, + { + name: "large gaps filled with offset", + inputs: []rangeDefinition{ + {18, 3, 1, 1, "INT16", false}, + {21, maxsize - 23, 1, 1, "INT16", true}, + {maxsize - 2, 5, 1, 1, "INT16", false}, + {maxsize + 3, 39, 1, 1, "INT16", true}, + {maxsize + 42, 2, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 18, count: 3, length: 1}}, + req: request{address: 18, length: 3}, + }, + { + fields: []rangeDefinition{ + {start: maxsize - 2, count: 5, length: 1}, + {start: maxsize + 42, count: 2, length: 1}, + }, + req: request{address: maxsize - 2, length: 46}, + }, + }, + }, + { + name: "from PR #11106", + inputs: []rangeDefinition{ + {0, 2, 1, 1, "INT16", true}, + {2, 1, 1, 1, "INT16", false}, + {3, 2*maxsize + 1, 1, 1, "INT16", true}, + {2*maxsize + 1, 1, 1, 1, "INT16", false}, + }, + expected: []requestExpectation{ + { + fields: []rangeDefinition{{start: 2, count: 1, length: 1}}, + req: request{address: 2, length: 1}, + }, + { + fields: []rangeDefinition{{start: 2*maxsize + 1, count: 1, length: 1}}, + req: request{address: 2*maxsize + 1, length: 1}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Generate the input structure and the expectation + requestFields := generateRequestDefinitions(tt.inputs) + expected := generateExpectation(tt.expected) + + // Setup the plugin + slaveID := byte(1) + plugin := Modbus{ + Name: "Test", + Controller: "tcp://localhost:1502", + ConfigurationType: "request", + Log: testutil.Logger{}, + } + plugin.Requests = []requestDefinition{ + { + SlaveID: slaveID, + ByteOrder: "ABCD", + RegisterType: "holding", + Optimization: "aggressive", + Fields: requestFields, + }, + } + require.NoError(t, plugin.Init()) + require.NotEmpty(t, plugin.requests) + require.Contains(t, plugin.requests, slaveID) + requireEqualRequests(t, expected, plugin.requests[slaveID].holding) + }) + } +} diff --git a/plugins/inputs/modbus/request.go b/plugins/inputs/modbus/request.go index b3c5b62dc..d8cdc22c5 100644 --- a/plugins/inputs/modbus/request.go +++ b/plugins/inputs/modbus/request.go @@ -9,24 +9,121 @@ type request struct { tags map[string]string } -func newRequest(f field, tags map[string]string) request { - r := request{ - address: f.address, - length: f.length, - fields: []field{}, - tags: map[string]string{}, +func countRegisters(requests []request) uint64 { + var l uint64 + for _, r := range requests { + l += uint64(r.length) } - if !f.omit { - r.fields = append(r.fields, f) - } - // Copy the tags - for k, v := range tags { - r.tags[k] = v - } - return r + return l } -func groupFieldsToRequests(fields []field, tags map[string]string, maxBatchSize uint16) []request { +// Only split too-large groups, but ignore all optimization potential +func splitMaxBatchSize(g request, maxBatchSize uint16) []request { + var requests []request + + idx := 0 + for start := g.address; start < g.address+g.length; { + current := request{ + fields: []field{}, + address: start, + } + for _, f := range g.fields[idx:] { + // End of field still fits into the batch + if f.address+f.length <= start+maxBatchSize { + current.fields = append(current.fields, f) + idx++ + } + } + + end := start + maxBatchSize + if end > g.address+g.length { + end = g.address + g.length + } + if idx >= len(g.fields) || g.fields[idx].address >= end { + current.length = end - start + } else { + current.length = g.fields[idx].address - start + } + start = end + + if len(current.fields) > 0 { + requests = append(requests, current) + } + } + + return requests +} + +func shrinkGroup(g request, maxBatchSize uint16) []request { + var requests []request + var current request + + for _, f := range g.fields { + // Just add the field and update length if we are still + // within the maximum batch-size + if current.length > 0 && f.address+f.length <= current.address+maxBatchSize { + current.fields = append(current.fields, f) + current.length = f.address - current.address + f.length + continue + } + + // Ignore completely empty requests + if len(current.fields) > 0 { + requests = append(requests, current) + } + + // Create a new request + current = request{ + fields: []field{f}, + address: f.address, + length: f.length, + } + } + if len(current.fields) > 0 { + requests = append(requests, current) + } + + return requests +} + +func optimizeGroup(g request, maxBatchSize uint16) []request { + if len(g.fields) == 0 { + return nil + } + + requests := shrinkGroup(g, maxBatchSize) + length := countRegisters(requests) + + for i := 1; i < len(g.fields)-1; i++ { + // Always keep consecutive fields as they are known to be optimal + if g.fields[i-1].address+g.fields[i-1].length == g.fields[i].address { + continue + } + + // Perform the split and check if it is better + // Note: This involves recursive optimization of the right side of the split. + current := shrinkGroup(request{fields: g.fields[:i]}, maxBatchSize) + current = append(current, optimizeGroup(request{fields: g.fields[i:]}, maxBatchSize)...) + currentLength := countRegisters(current) + + // Do not allow for more requests + if len(current) > len(requests) { + continue + } + // Try to reduce the number of registers we are trying to access + if currentLength >= length { + continue + } + + // We found a better solution + requests = current + length = currentLength + } + + return requests +} + +func groupFieldsToRequests(fields []field, tags map[string]string, maxBatchSize uint16, optimization string) []request { if len(fields) == 0 { return nil } @@ -42,28 +139,78 @@ func groupFieldsToRequests(fields []field, tags map[string]string, maxBatchSize // For field addresses like [1, 2, 3, 5, 6, 10, 11, 12, 14] we should construct the following // requests (1, 3) , (5, 2) , (10, 3), (14 , 1). Furthermore, we should respect field boundaries // and the given maximum chunk sizes. - var requests []request - - current := newRequest(fields[0], tags) - for _, f := range fields[1:] { + var groups []request + var current request + for _, f := range fields { // Check if we need to interrupt the current chunk and require a new one - needInterrupt := f.address != current.address+current.length // not consecutive - needInterrupt = needInterrupt || f.length+current.length > maxBatchSize // too large - - if !needInterrupt { + if current.length > 0 && f.address == current.address+current.length { // Still safe to add the field to the current request current.length += f.length if !f.omit { - // Omit adding the field but use it for constructing the request. current.fields = append(current.fields, f) } continue } // Finish the current request, add it to the list and construct a new one - requests = append(requests, current) - current = newRequest(f, tags) + if current.length > 0 { + groups = append(groups, current) + } + current = request{ + fields: []field{}, + address: f.address, + length: f.length, + } + if !f.omit { + current.fields = append(current.fields, f) + } + } + if current.length > 0 { + groups = append(groups, current) + } + + var requests []request + switch optimization { + case "shrink": + // Shrink request by striping leading and trailing fields with an omit flag set + for _, g := range groups { + if len(g.fields) > 0 { + requests = append(requests, shrinkGroup(g, maxBatchSize)...) + } + } + case "rearrange": + // Allow rearranging fields between request in order to reduce the number of touched + // registers while keeping the number of requests + for _, g := range groups { + if len(g.fields) > 0 { + requests = append(requests, optimizeGroup(g, maxBatchSize)...) + } + } + case "aggressive": + // Allow rearranging fields similar to "rearrange" but allow mixing of groups + // This might reduce the number of requests at the cost of more registers being touched. + var total request + for _, g := range groups { + if len(g.fields) > 0 { + total.fields = append(total.fields, g.fields...) + } + } + requests = optimizeGroup(total, maxBatchSize) + default: + // no optimization + for _, g := range groups { + if len(g.fields) > 0 { + requests = append(requests, splitMaxBatchSize(g, maxBatchSize)...) + } + } + } + + // Copy the tags + for i := range requests { + requests[i].tags = make(map[string]string) + for k, v := range tags { + requests[i].tags[k] = v + } } - requests = append(requests, current) return requests } diff --git a/plugins/inputs/modbus/sample_request.conf b/plugins/inputs/modbus/sample_request.conf index 29ea42e08..876ff8d13 100644 --- a/plugins/inputs/modbus/sample_request.conf +++ b/plugins/inputs/modbus/sample_request.conf @@ -25,6 +25,19 @@ ## Can be overriden by the individual field definitions. Defaults to "modbus" # measurement = "modbus" + ## Request optimization algorithm. + ## |---none -- Do not perform any optimization and use the given layout(default) + ## |---shrink -- Shrink requests to actually requested fields + ## | by stripping leading and trailing omits + ## |---rearrange -- Rearrange request boundaries within consecutive address ranges + ## | to reduce the number of requested registers by keeping + ## | the number of requests. + ## |---aggressive -- Rearrange request boundaries similar to "rearrange" but + ## allow to request registers not specified by the user to + ## fill gaps. This usually reduces the number of requests at the + ## cost of more requested registers. + # optimization = "none" + ## Field definitions ## Analog Variables, Input Registers and Holding Registers ## address - address of the register to query. For coil and discrete inputs this is the bit address. diff --git a/plugins/inputs/modbus/testcases/duplicate_fields_same_slave_and_request/init.err b/plugins/inputs/modbus/testcases/duplicate_fields_same_slave_and_request/init.err index 6e8a022bd..137f9c3a4 100644 --- a/plugins/inputs/modbus/testcases/duplicate_fields_same_slave_and_request/init.err +++ b/plugins/inputs/modbus/testcases/duplicate_fields_same_slave_and_request/init.err @@ -1 +1 @@ -configuraton invalid: field "Voltage" duplicated in measurement "V" \ No newline at end of file +configuration invalid: field "Voltage" duplicated in measurement "V" \ No newline at end of file