From 0b359e8d84734869b663e22bfc204813519553bb Mon Sep 17 00:00:00 2001 From: Timur Date: Tue, 17 May 2022 08:01:36 +0200 Subject: [PATCH] fix(inputs/modbus) #11105 no empty grouped requests Simple modification to * allow starting the field list with an omitted field (useful if many requests needed) * avoids requests where all fields are omitted As a result, the total number of request is generally lower, also improving performance improve readme readme linting use empty instaed of len = 0 wip wip wip --- plugins/inputs/modbus/README.md | 18 +++++++ .../inputs/modbus/configuration_register.go | 2 +- .../inputs/modbus/configuration_request.go | 21 ++++---- plugins/inputs/modbus/modbus_test.go | 43 +++++++++++++++ plugins/inputs/modbus/request.go | 54 ++++++++++++++++--- plugins/inputs/modbus/sample_request.conf | 20 +++++++ 6 files changed, 141 insertions(+), 17 deletions(-) diff --git a/plugins/inputs/modbus/README.md b/plugins/inputs/modbus/README.md index 872823dc52f24..f40344e9dff65 100644 --- a/plugins/inputs/modbus/README.md +++ b/plugins/inputs/modbus/README.md @@ -53,6 +53,7 @@ Registers via Modbus TCP or Modbus RTU/ASCII. configuration_type = "register" ## --- "register" configuration style --- + ## Prefer the "request" configuarion style (See below, that offers more options and improvements) ## Measurements ## @@ -322,6 +323,7 @@ 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. +For _coil_ and _discrete input_ registers this setting specifies the **bit** containing the value of the field. ##### name @@ -331,6 +333,7 @@ and can be omitted in this case. __Please note:__ There cannot be multiple fields with the same `name` in one metric identified by `measurement`, `slave_id` and `register`. +**Please note:** There cannot be multiple fields with the same `name` in one metric identified by `measurement`, `slave_id` and `register`. ##### register datatype @@ -357,6 +360,7 @@ cases. __Please note:__ The resulting field-type will be set to `FLOAT64` if no output format is specified. +**Please note:** The resulting field-type will be set to `FLOAT64` if no output format is specified. ##### output datatype @@ -391,6 +395,8 @@ requests. This way, you can fill "holes" in the addresses to construct consecutive address ranges resulting in a single request. Using a single modbus request can be beneficial as the values are all collected at the same point in time. +When specifying `omit=true`, the corresponding field will be ignored when collecting the metric but is taken into account when constructing the modbus requests. This way, you can fill "holes" in the addresses to construct consecutive address ranges resulting in a single request. Using a single modbus request can be beneficial as the values are all collected at the same point in time. +If your device can produce many fields, you can list all of them as `omit=false` (except of course the ones you are actually interested in) and let telegraf find the optimal number of requests needed. #### Tags definitions @@ -398,6 +404,18 @@ Each `request` can be accompanied by tags valid for this request. __Please note:__ These tags take precedence over predefined tags such as `name`, `type` or `slave_id`. +**Please note:** These tags take precedence over predefined tags such as `name`, `type` or `slave_id`. + +#### Optimising the number of requests + +In the case where your modbus device can produce several hundreds of signals but you +are interested in logging only a few of them, you can be interested in optimising the +number of requests sent to your device. In order to do so, list all the fields available +in your device in your configuration, set all of them to `omit=true` except for the fields +you want to log and set the `optimise_requests` to `true`. + +**Nota Bene** you should not use the `optimise_requests` options if your device requires +the first address to always be queried. --- diff --git a/plugins/inputs/modbus/configuration_register.go b/plugins/inputs/modbus/configuration_register.go index 87275675a0f0c..7ea28b7f2121d 100644 --- a/plugins/inputs/modbus/configuration_register.go +++ b/plugins/inputs/modbus/configuration_register.go @@ -82,7 +82,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, false), 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 e9ab1f57bdfab..026ebf9dcd1ae 100644 --- a/plugins/inputs/modbus/configuration_request.go +++ b/plugins/inputs/modbus/configuration_request.go @@ -21,12 +21,13 @@ type requestFieldDefinition struct { } type requestDefinition struct { - SlaveID byte `toml:"slave_id"` - ByteOrder string `toml:"byte_order"` - RegisterType string `toml:"register"` - Measurement string `toml:"measurement"` - Fields []requestFieldDefinition `toml:"fields"` - Tags map[string]string `toml:"tags"` + SlaveID byte `toml:"slave_id"` + ByteOrder string `toml:"byte_order"` + RegisterType string `toml:"register"` + Measurement string `toml:"measurement"` + Fields []requestFieldDefinition `toml:"fields"` + Tags map[string]string `toml:"tags"` + OptimiseRequests bool `toml:"optimise_requests"` } type ConfigurationPerRequest struct { @@ -147,16 +148,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.OptimiseRequests) set.coil = append(set.coil, requests...) case "discrete": - requests := groupFieldsToRequests(fields, def.Tags, maxQuantityDiscreteInput) + requests := groupFieldsToRequests(fields, def.Tags, maxQuantityDiscreteInput, def.OptimiseRequests) set.discrete = append(set.discrete, requests...) case "holding": - requests := groupFieldsToRequests(fields, def.Tags, maxQuantityHoldingRegisters) + requests := groupFieldsToRequests(fields, def.Tags, maxQuantityHoldingRegisters, def.OptimiseRequests) set.holding = append(set.holding, requests...) case "input": - requests := groupFieldsToRequests(fields, def.Tags, maxQuantityInputRegisters) + requests := groupFieldsToRequests(fields, def.Tags, maxQuantityInputRegisters, def.OptimiseRequests) set.input = append(set.input, requests...) default: return nil, fmt.Errorf("unknown register type %q", def.RegisterType) diff --git a/plugins/inputs/modbus/modbus_test.go b/plugins/inputs/modbus/modbus_test.go index 5a7a7570d7b7d..65d84dcc64a04 100644 --- a/plugins/inputs/modbus/modbus_test.go +++ b/plugins/inputs/modbus/modbus_test.go @@ -1923,4 +1923,47 @@ func TestRequestsStartingWithOmits(t *testing.T) { require.NoError(t, modbus.Gather(&acc)) acc.Wait(len(expected)) testutil.RequireMetricsEqual(t, expected, acc.GetTelegrafMetrics(), testutil.IgnoreTime()) + require.Len(t, modbus.requests[1].holding, 1) + require.Equal(t, uint16(0), modbus.requests[1].holding[0].address) +} + +func TestOptimisingRequestNumber(t *testing.T) { + modbus := Modbus{ + Name: "Test", + Controller: "tcp://localhost:1502", + ConfigurationType: "request", + Log: testutil.Logger{}, + } + modbus.Requests = []requestDefinition{ + {SlaveID: 1, + ByteOrder: "ABCD", + RegisterType: "holding", + OptimiseRequests: true, + Fields: []requestFieldDefinition{ + { + Name: "holding-0", + Address: uint16(0), + InputType: "INT16", + Omit: true, + }, + { + Name: "holding-1", + Address: uint16(1), + InputType: "UINT16", + Omit: true, + }, + { + Name: "holding-2", + Address: uint16(2), + InputType: "INT64", + Omit: false, + }, + }, + }, + } + require.NoError(t, modbus.Init()) + require.NotEmpty(t, modbus.requests) + require.NotNil(t, modbus.requests[1]) + require.Len(t, modbus.requests[1].holding, 1) + require.Equal(t, uint16(2), modbus.requests[1].holding[0].address) } diff --git a/plugins/inputs/modbus/request.go b/plugins/inputs/modbus/request.go index b3c5b62dc76ed..55abec711aaf7 100644 --- a/plugins/inputs/modbus/request.go +++ b/plugins/inputs/modbus/request.go @@ -13,12 +13,9 @@ func newRequest(f field, tags map[string]string) request { r := request{ address: f.address, length: f.length, - fields: []field{}, + fields: []field{f}, tags: map[string]string{}, } - if !f.omit { - r.fields = append(r.fields, f) - } // Copy the tags for k, v := range tags { r.tags[k] = v @@ -26,7 +23,7 @@ func newRequest(f field, tags map[string]string) request { return r } -func groupFieldsToRequests(fields []field, tags map[string]string, maxBatchSize uint16) []request { +func groupFieldsToRequests(fields []field, tags map[string]string, maxBatchSize uint16, optimise bool) []request { if len(fields) == 0 { return nil } @@ -37,7 +34,9 @@ func groupFieldsToRequests(fields []field, tags map[string]string, maxBatchSize addrJ := fields[j].address return addrI < addrJ || (addrI == addrJ && fields[i].length > fields[j].length) }) - + if optimise { // if it is OK to remove the leading omitted fields, try to optimise the number of requests + return groupFieldsToRequestsOptimised(fields, tags, maxBatchSize) + } // Construct the consecutive register chunks for the addresses and construct Modbus requests. // 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 @@ -65,5 +64,48 @@ func groupFieldsToRequests(fields []field, tags map[string]string, maxBatchSize current = newRequest(f, tags) } requests = append(requests, current) + // Filter out completely empty requests + nonEmptyRequests := make([]request, 0, len(requests)) + for _, request := range requests { + if len(request.fields) != 0 { + nonEmptyRequests = append(nonEmptyRequests, request) + } + } + requests = nonEmptyRequests return requests } + +func isNonEmptyRequest(r request) bool { + return !((len(r.fields) == 0) || ((len(r.fields) == 1) && (r.fields[0].address == r.address))) +} + +func groupFieldsToRequestsOptimised(fields []field, tags map[string]string, maxBatchSize uint16) []request { + var requests []request + current := newRequest(fields[0], tags) + for _, f := range fields[1:] { + // 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 { + // 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. + if isNonEmptyRequest(current) { + current.fields = append(current.fields, f) + } else { + current = newRequest(f, tags) + } + } + } + if isNonEmptyRequest(current) { + requests = append(requests, current) + } + current = newRequest(f, tags) + } + if isNonEmptyRequest(current) { + requests = append(requests, current) + } + return requests + +} diff --git a/plugins/inputs/modbus/sample_request.conf b/plugins/inputs/modbus/sample_request.conf index 717b04e2de22e..c2309fb2dab45 100644 --- a/plugins/inputs/modbus/sample_request.conf +++ b/plugins/inputs/modbus/sample_request.conf @@ -74,6 +74,26 @@ machine = "impresser" location = "main building" + [[inputs.modbus.request]] + ## Holding example with request optimisation + ## The first field will not be added to the request. + slave_id = 1 + byte_order = "DCBA" + register = "holding" + optimise_requests = "true" + fields = [ + { address=0, name="voltage", type="INT16", omit=true }, + { address=1, name="current", type="INT32", scale=0.001 }, + { address=3, name="power", type="UINT32", omit=true }, + { address=5, name="energy", type="FLOAT32", scale=0.001, measurement="W" }, + { address=7, name="frequency", type="UINT32", scale=0.1 }, + { address=8, name="power_factor", type="INT64", scale=0.01 }, + ] + + [[inputs.modbus.request.tags]] + machine = "impresser" + location = "main building" + [[inputs.modbus.request]] ## Input example with type conversions slave_id = 1