diff --git a/plugins/inputs/modbus/README.md b/plugins/inputs/modbus/README.md index ad5a7cf21717c..9983d84e24afc 100644 --- a/plugins/inputs/modbus/README.md +++ b/plugins/inputs/modbus/README.md @@ -357,6 +357,16 @@ See the [CONFIGURATION.md][CONFIGURATION.md] for more details. ## coil registers. This is necessary for some devices see ## https://github.com/influxdata/telegraf/issues/8905 # read_coils_starting_at_zero = false + + ## String byte-location in registers AFTER byte-order conversion + ## Some device (e.g. EM340) place the string byte in only the upper or + ## lower byte location of a register see + ## https://github.com/influxdata/telegraf/issues/14748 + ## Available settings: + ## lower -- use only lower byte of the register i.e. 00XX 00XX 00XX 00XX + ## upper -- use only upper byte of the register i.e. XX00 XX00 XX00 XX00 + ## By default both bytes of the register are used i.e. XXXX XXXX. + # string_register_location = "" ``` ## Notes diff --git a/plugins/inputs/modbus/configuration_metric.go b/plugins/inputs/modbus/configuration_metric.go index f25771ac98a54..d06e2fbd22830 100644 --- a/plugins/inputs/modbus/configuration_metric.go +++ b/plugins/inputs/modbus/configuration_metric.go @@ -43,6 +43,13 @@ func (c *ConfigurationPerMetric) SampleConfigPart() string { } func (c *ConfigurationPerMetric) Check() error { + switch c.workarounds.StringRegisterLocation { + case "", "both", "lower", "upper": + // Do nothing as those are valid + default: + return fmt.Errorf("invalid 'string_register_location' %q", c.workarounds.StringRegisterLocation) + } + seed := maphash.MakeSeed() seenFields := make(map[uint64]bool) @@ -302,7 +309,7 @@ func (c *ConfigurationPerMetric) newField(def metricFieldDefinition, mdef metric return field{}, err } - f.converter, err = determineConverter(inType, order, outType, def.Scale) + f.converter, err = determineConverter(inType, order, outType, def.Scale, c.workarounds.StringRegisterLocation) if err != nil { return field{}, err } diff --git a/plugins/inputs/modbus/configuration_register.go b/plugins/inputs/modbus/configuration_register.go index 8266e59b4b997..28762223adde0 100644 --- a/plugins/inputs/modbus/configuration_register.go +++ b/plugins/inputs/modbus/configuration_register.go @@ -35,6 +35,13 @@ func (c *ConfigurationOriginal) SampleConfigPart() string { } func (c *ConfigurationOriginal) Check() error { + switch c.workarounds.StringRegisterLocation { + case "", "both", "lower", "upper": + // Do nothing as those are valid + default: + return fmt.Errorf("invalid 'string_register_location' %q", c.workarounds.StringRegisterLocation) + } + if err := c.validateFieldDefinitions(c.DiscreteInputs, cDiscreteInputs); err != nil { return err } @@ -165,7 +172,7 @@ func (c *ConfigurationOriginal) newFieldFromDefinition(def fieldDefinition, type return f, err } - f.converter, err = determineConverter(inType, byteOrder, outType, def.Scale) + f.converter, err = determineConverter(inType, byteOrder, outType, def.Scale, c.workarounds.StringRegisterLocation) if err != nil { return f, err } diff --git a/plugins/inputs/modbus/configuration_request.go b/plugins/inputs/modbus/configuration_request.go index 85ab88a4b164d..68feb2ffb35c2 100644 --- a/plugins/inputs/modbus/configuration_request.go +++ b/plugins/inputs/modbus/configuration_request.go @@ -46,6 +46,13 @@ func (c *ConfigurationPerRequest) SampleConfigPart() string { } func (c *ConfigurationPerRequest) Check() error { + switch c.workarounds.StringRegisterLocation { + case "", "both", "lower", "upper": + // Do nothing as those are valid + default: + return fmt.Errorf("invalid 'string_register_location' %q", c.workarounds.StringRegisterLocation) + } + seed := maphash.MakeSeed() seenFields := make(map[uint64]bool) @@ -349,7 +356,7 @@ func (c *ConfigurationPerRequest) newFieldFromDefinition(def requestFieldDefinit return field{}, err } - f.converter, err = determineConverter(inType, order, outType, def.Scale) + f.converter, err = determineConverter(inType, order, outType, def.Scale, c.workarounds.StringRegisterLocation) if err != nil { return field{}, err } diff --git a/plugins/inputs/modbus/modbus.go b/plugins/inputs/modbus/modbus.go index 463020672ec9d..e47128a14b262 100644 --- a/plugins/inputs/modbus/modbus.go +++ b/plugins/inputs/modbus/modbus.go @@ -31,6 +31,7 @@ type ModbusWorkarounds struct { CloseAfterGather bool `toml:"close_connection_after_gather"` OnRequestPerField bool `toml:"one_request_per_field"` ReadCoilsStartingAtZero bool `toml:"read_coils_starting_at_zero"` + StringRegisterLocation string `toml:"string_register_location"` } // According to github.com/grid-x/serial diff --git a/plugins/inputs/modbus/modbus_test.go b/plugins/inputs/modbus/modbus_test.go index 419ba6795674a..c339274d87da1 100644 --- a/plugins/inputs/modbus/modbus_test.go +++ b/plugins/inputs/modbus/modbus_test.go @@ -16,6 +16,7 @@ import ( "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/config" + "github.com/influxdata/telegraf/metric" "github.com/influxdata/telegraf/plugins/inputs" "github.com/influxdata/telegraf/plugins/parsers/influx" "github.com/influxdata/telegraf/testutil" @@ -571,3 +572,179 @@ func TestRequestsWorkaroundsReadCoilsStartingAtZeroRegister(t *testing.T) { require.Equal(t, maxQuantityCoils, plugin.requests[1].coil[1].address) require.Equal(t, uint16(1), plugin.requests[1].coil[1].length) } + +func TestWorkaroundsStringRegisterLocation(t *testing.T) { + tests := []struct { + name string + location string + order string + content []byte + expected string + }{ + { + name: "default big-endian", + order: "ABCD", + content: []byte{ + 0x4d, 0x6f, 0x64, 0x62, 0x75, 0x73, 0x20, 0x53, + 0x74, 0x72, 0x69, 0x6e, 0x67, 0x00, + }, + expected: "Modbus String", + }, + { + name: "default little-endian", + order: "DCBA", + content: []byte{ + 0x6f, 0x4d, 0x62, 0x64, 0x73, 0x75, 0x53, 0x20, + 0x72, 0x74, 0x6e, 0x69, 0x00, 0x67, + }, + expected: "Modbus String", + }, + { + name: "both big-endian", + location: "both", + order: "ABCD", + content: []byte{ + 0x4d, 0x6f, 0x64, 0x62, 0x75, 0x73, 0x20, 0x53, + 0x74, 0x72, 0x69, 0x6e, 0x67, 0x00, + }, + expected: "Modbus String", + }, + { + name: "both little-endian", + location: "both", + order: "DCBA", + content: []byte{ + 0x6f, 0x4d, 0x62, 0x64, 0x73, 0x75, 0x53, 0x20, + 0x72, 0x74, 0x6e, 0x69, 0x00, 0x67, + }, + expected: "Modbus String", + }, + { + name: "lower big-endian", + location: "lower", + order: "ABCD", + content: []byte{ + 0x00, 0x4d, 0x00, 0x6f, 0x00, 0x64, 0x00, 0x62, + 0x00, 0x75, 0x00, 0x73, 0x00, 0x20, 0x00, 0x53, + 0x00, 0x74, 0x00, 0x72, 0x00, 0x69, 0x00, 0x6e, + 0x00, 0x67, 0x00, 0x00, + }, + expected: "Modbus String", + }, + { + name: "lower little-endian", + location: "lower", + order: "DCBA", + content: []byte{ + 0x4d, 0x00, 0x6f, 0x00, 0x64, 0x00, 0x62, 0x00, + 0x75, 0x00, 0x73, 0x00, 0x20, 0x00, 0x53, 0x00, + 0x74, 0x00, 0x72, 0x00, 0x69, 0x00, 0x6e, 0x00, + 0x67, 0x00, 0x00, 0x00, + }, + expected: "Modbus String", + }, + { + name: "upper big-endian", + location: "upper", + order: "ABCD", + content: []byte{ + 0x4d, 0x00, 0x6f, 0x00, 0x64, 0x00, 0x62, 0x00, + 0x75, 0x00, 0x73, 0x00, 0x20, 0x00, 0x53, 0x00, + 0x74, 0x00, 0x72, 0x00, 0x69, 0x00, 0x6e, 0x00, + 0x67, 0x00, 0x00, 0x00, + }, + expected: "Modbus String", + }, + { + name: "upper little-endian", + location: "upper", + order: "DCBA", + content: []byte{ + 0x00, 0x4d, 0x00, 0x6f, 0x00, 0x64, 0x00, 0x62, + 0x00, 0x75, 0x00, 0x73, 0x00, 0x20, 0x00, 0x53, + 0x00, 0x74, 0x00, 0x72, 0x00, 0x69, 0x00, 0x6e, + 0x00, 0x67, 0x00, 0x00, + }, + expected: "Modbus String", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + const addr = uint16(8) + length := uint16(len(tt.content) / 2) + + expected := []telegraf.Metric{ + metric.New( + "modbus", + map[string]string{ + "name": "Test", + "slave_id": "1", + "type": "holding_register", + }, + map[string]interface{}{ + "value": tt.expected, + }, + time.Unix(0, 0), + ), + } + + plugin := &Modbus{ + Name: "Test", + Controller: "tcp://localhost:1502", + ConfigurationType: "request", + Log: testutil.Logger{}, + Workarounds: ModbusWorkarounds{StringRegisterLocation: tt.location}, + ConfigurationPerRequest: ConfigurationPerRequest{ + Requests: []requestDefinition{ + { + SlaveID: 1, + ByteOrder: tt.order, + RegisterType: "holding", + Fields: []requestFieldDefinition{ + { + Address: addr, + Name: "value", + InputType: "STRING", + Length: length, + }, + }, + }, + }, + }, + } + require.NoError(t, plugin.Init()) + + // Create a mock server and fill in the data + serv := mbserver.NewServer() + require.NoError(t, serv.ListenTCP("localhost:1502")) + defer serv.Close() + + handler := mb.NewTCPClientHandler("localhost:1502") + require.NoError(t, handler.Connect()) + defer handler.Close() + client := mb.NewClient(handler) + _, err := client.WriteMultipleRegisters(addr, length, tt.content) + require.NoError(t, err) + + // Gather the data + var acc testutil.Accumulator + require.NoError(t, plugin.Gather(&acc)) + + // Compare + actual := acc.GetTelegrafMetrics() + testutil.RequireMetricsEqual(t, expected, actual, testutil.IgnoreTime()) + }) + } +} + +func TestWorkaroundsStringRegisterLocationInvalid(t *testing.T) { + plugin := &Modbus{ + Name: "Test", + Controller: "tcp://localhost:1502", + ConfigurationType: "request", + Log: testutil.Logger{}, + Workarounds: ModbusWorkarounds{StringRegisterLocation: "foo"}, + } + require.ErrorContains(t, plugin.Init(), `invalid 'string_register_location'`) +} diff --git a/plugins/inputs/modbus/sample_general_end.conf b/plugins/inputs/modbus/sample_general_end.conf index 0ab5c924ee610..aec19a7e500eb 100644 --- a/plugins/inputs/modbus/sample_general_end.conf +++ b/plugins/inputs/modbus/sample_general_end.conf @@ -38,4 +38,14 @@ ## Enforce the starting address to be zero for the first request on ## coil registers. This is necessary for some devices see ## https://github.com/influxdata/telegraf/issues/8905 - # read_coils_starting_at_zero = false \ No newline at end of file + # read_coils_starting_at_zero = false + + ## String byte-location in registers AFTER byte-order conversion + ## Some device (e.g. EM340) place the string byte in only the upper or + ## lower byte location of a register see + ## https://github.com/influxdata/telegraf/issues/14748 + ## Available settings: + ## lower -- use only lower byte of the register i.e. 00XX 00XX 00XX 00XX + ## upper -- use only upper byte of the register i.e. XX00 XX00 XX00 XX00 + ## By default both bytes of the register are used i.e. XXXX XXXX. + # string_register_location = "" diff --git a/plugins/inputs/modbus/type_conversions.go b/plugins/inputs/modbus/type_conversions.go index f8e9f99ce8009..5730284e78511 100644 --- a/plugins/inputs/modbus/type_conversions.go +++ b/plugins/inputs/modbus/type_conversions.go @@ -18,8 +18,19 @@ func determineUntypedConverter(outType string) (fieldConverterFunc, error) { return nil, fmt.Errorf("invalid output data-type: %s", outType) } -func determineConverter(inType, byteOrder, outType string, scale float64) (fieldConverterFunc, error) { - if scale != 0.0 && inType != "STRING" { +func determineConverter(inType, byteOrder, outType string, scale float64, strloc string) (fieldConverterFunc, error) { + if inType == "STRING" { + switch strloc { + case "", "both": + return determineConverterString(byteOrder) + case "lower": + return determineConverterStringLow(byteOrder) + case "upper": + return determineConverterStringHigh(byteOrder) + } + } + + if scale != 0.0 { return determineConverterScale(inType, byteOrder, outType, scale) } return determineConverterNoScale(inType, byteOrder, outType) @@ -85,8 +96,6 @@ func determineConverterNoScale(inType, byteOrder, outType string) (fieldConverte return determineConverterF32(outType, byteOrder) case "FLOAT64": return determineConverterF64(outType, byteOrder) - case "STRING": - return determineConverterString(byteOrder) } return nil, fmt.Errorf("invalid input data-type: %s", inType) } diff --git a/plugins/inputs/modbus/type_conversions_string.go b/plugins/inputs/modbus/type_conversions_string.go index 26d57fa2557ef..c8cb9d4364d40 100644 --- a/plugins/inputs/modbus/type_conversions_string.go +++ b/plugins/inputs/modbus/type_conversions_string.go @@ -23,3 +23,41 @@ func determineConverterString(byteOrder string) (fieldConverterFunc, error) { return string(s) }, nil } + +func determineConverterStringLow(byteOrder string) (fieldConverterFunc, error) { + tohost, err := endiannessConverter16(byteOrder) + if err != nil { + return nil, err + } + + return func(b []byte) interface{} { + // Swap the bytes according to endianness + var buf bytes.Buffer + for i := 0; i < len(b); i += 2 { + v := tohost(b[i : i+2]) + buf.WriteByte(byte(v & 0xFF)) + } + // Remove everything after null-termination + s, _ := bytes.CutSuffix(buf.Bytes(), []byte{0x00}) + return string(s) + }, nil +} + +func determineConverterStringHigh(byteOrder string) (fieldConverterFunc, error) { + tohost, err := endiannessConverter16(byteOrder) + if err != nil { + return nil, err + } + + return func(b []byte) interface{} { + // Swap the bytes according to endianness + var buf bytes.Buffer + for i := 0; i < len(b); i += 2 { + v := tohost(b[i : i+2]) + buf.WriteByte(byte(v >> 8)) + } + // Remove everything after null-termination + s, _ := bytes.CutSuffix(buf.Bytes(), []byte{0x00}) + return string(s) + }, nil +}