From e097676f71f8653046e1bdf9227afedb64669e0f Mon Sep 17 00:00:00 2001 From: Sven Rebhan <36194019+srebhan@users.noreply.github.com> Date: Wed, 26 Oct 2022 21:58:27 +0200 Subject: [PATCH] fix(inputs.modbus): Handle field-measurement definitions correctly on duplicate field check (#12109) --- .../inputs/modbus/configuration_request.go | 8 +-- plugins/inputs/modbus/modbus_test.go | 1 - .../duplicate_fields_issue_12091/expected.out | 7 ++ .../telegraf.conf | 71 +++++++++++++++++++ 4 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/expected.out create mode 100644 plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/telegraf.conf diff --git a/plugins/inputs/modbus/configuration_request.go b/plugins/inputs/modbus/configuration_request.go index 60569cf36..895e20d22 100644 --- a/plugins/inputs/modbus/configuration_request.go +++ b/plugins/inputs/modbus/configuration_request.go @@ -110,7 +110,7 @@ func (c *ConfigurationPerRequest) Check() error { def.Fields[fidx] = f // Check for duplicate field definitions - id, err := c.fieldID(seed, def, f.Name) + id, err := c.fieldID(seed, def, f) if err != nil { return fmt.Errorf("cannot determine field id for %q: %v", f.Name, err) } @@ -253,7 +253,7 @@ func (c *ConfigurationPerRequest) newFieldFromDefinition(def requestFieldDefinit return f, nil } -func (c *ConfigurationPerRequest) fieldID(seed maphash.Seed, def requestDefinition, name string) (uint64, error) { +func (c *ConfigurationPerRequest) fieldID(seed maphash.Seed, def requestDefinition, field requestFieldDefinition) (uint64, error) { var mh maphash.Hash mh.SetSeed(seed) @@ -269,13 +269,13 @@ func (c *ConfigurationPerRequest) fieldID(seed maphash.Seed, def requestDefiniti if err := mh.WriteByte(0); err != nil { return 0, err } - if _, err := mh.WriteString(def.Measurement); err != nil { + if _, err := mh.WriteString(field.Measurement); err != nil { return 0, err } if err := mh.WriteByte(0); err != nil { return 0, err } - if _, err := mh.WriteString(name); err != nil { + if _, err := mh.WriteString(field.Name); err != nil { return 0, err } if err := mh.WriteByte(0); err != nil { diff --git a/plugins/inputs/modbus/modbus_test.go b/plugins/inputs/modbus/modbus_test.go index 49bb41ec1..09ba1e5e5 100644 --- a/plugins/inputs/modbus/modbus_test.go +++ b/plugins/inputs/modbus/modbus_test.go @@ -2086,7 +2086,6 @@ func TestCases(t *testing.T) { case 4: // 64-bit binary.BigEndian.PutUint64(buf[1:], uint64(register)) } - fmt.Println(buf) return buf, &mbserver.Success } diff --git a/plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/expected.out b/plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/expected.out new file mode 100644 index 000000000..b56797e31 --- /dev/null +++ b/plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/expected.out @@ -0,0 +1,7 @@ +Bitfield,name=EAP,resource=30KTL,slave_id=9,type=holding_register Alarm1=0u,Alarm2=0u,Alarm3=0u,State1=32000u,State2=0u,State3=0u +Current,name=EAP,resource=30KTL,slave_id=9,type=holding_register PV1=0,PV2=0,PV3=0,PV4=0 +Power,name=EAP,resource=30KTL,slave_id=9,type=holding_register AC=32080,DC=32064,Efficiency=0 +Resistance,name=EAP,resource=30KTL,slave_id=9,type=holding_register Insulation=0 +Status,name=EAP,resource=30KTL,slave_id=9,type=holding_register Device=32086u +Temp,name=EAP,resource=30KTL,slave_id=9,type=holding_register Internal=0 +Voltage,name=EAP,resource=30KTL,slave_id=9,type=holding_register PV1=0,PV2=0,PV3=0,PV4=0 \ No newline at end of file diff --git a/plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/telegraf.conf b/plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/telegraf.conf new file mode 100644 index 000000000..2da6ff752 --- /dev/null +++ b/plugins/inputs/modbus/testcases/duplicate_fields_issue_12091/telegraf.conf @@ -0,0 +1,71 @@ +# Retrieve data from MODBUS slave devices +[[inputs.modbus]] + + # Hiermit wird auch _time auf 60s (also ganze Minuten) gerundet + # Sollten (nach der Timestamp Rundung) mehrere Abfragen auf denselben Timestamp (_time) + # kommen wird kein Fehler geworfen, sondern der bestehende Wert wird einfach upgedated! + precision = "60s" + + + ## Connection Configuration + ## + ## The plugin supports connections to PLCs via MODBUS/TCP, RTU over TCP, ASCII over TCP or + ## via serial line communication in binary (RTU) or readable (ASCII) encoding + ## + ## Device name + name = "EAP" + + ## Timeout for each request + timeout = "500ms" + + ## Maximum number of retries and the time to wait between retries + ## when a slave-device is busy. + busy_retries = 3 + busy_retries_wait = "200ms" + + # TCP - connect via Modbus/TCP + controller = "tcp://192.168.254.223:502" + + ## Trace the connection to the modbus device as debug messages + ## Note: You have to enable telegraf's debug mode to see those messages! + debug_connection = true + + ## Define the configuration schema + ## |---register -- define fields per register type in the original style (only supports one slave ID) + ## |---request -- define fields on a requests base + configuration_type = "request" + + ## --- "request" configuration style --- + + ## Per request definition + ## + + ## Define a request sent to the device + [[inputs.modbus.request]] + slave_id = 9 + byte_order = "ABCD" + register = "holding" + fields = [ + { address=32000, measurement="Bitfield", name="State1", type="UINT16" }, + { address=32002, measurement="Bitfield", name="State2", type="UINT16" }, + { address=32003, measurement="Bitfield", name="State3", type="UINT32" }, + { address=32008, measurement="Bitfield", name="Alarm1", type="UINT16" }, + { address=32009, measurement="Bitfield", name="Alarm2", type="UINT16" }, + { address=32010, measurement="Bitfield", name="Alarm3", type="UINT16" }, + { address=32016, measurement="Voltage", name="PV1", type="INT16", scale=0.1, output="FLOAT64" }, + { address=32017, measurement="Current", name="PV1", type="INT16", scale=0.01, output="FLOAT64" }, + { address=32018, measurement="Voltage", name="PV2", type="INT16", scale=0.1, output="FLOAT64" }, + { address=32019, measurement="Current", name="PV2", type="INT16", scale=0.01, output="FLOAT64" }, + { address=32020, measurement="Voltage", name="PV3", type="INT16", scale=0.1, output="FLOAT64" }, + { address=32021, measurement="Current", name="PV3", type="INT16", scale=0.01, output="FLOAT64" }, + { address=32022, measurement="Voltage", name="PV4", type="INT16", scale=0.1, output="FLOAT64" }, + { address=32023, measurement="Current", name="PV4", type="INT16", scale=0.01, output="FLOAT64" }, + { address=32064, measurement="Power", name="DC", type="INT32", output="FLOAT64" }, + { address=32080, measurement="Power", name="AC", type="INT32", output="FLOAT64" }, + { address=32086, measurement="Power", name="Efficiency", type="UINT16", scale=0.01, output="FLOAT64" }, + { address=32087, measurement="Temp", name="Internal", type="INT16", scale=0.1, output="FLOAT64" }, + { address=32088, measurement="Resistance", name="Insulation", type="UINT16", scale=0.001, output="FLOAT64" }, + { address=32089, measurement="Status", name="Device", type="UINT16" }, + ] + [inputs.modbus.request.tags] + resource = "30KTL" \ No newline at end of file