fix(inputs.modbus): Improve duplicate field checks (#11912)
This commit is contained in:
parent
14c8b9b135
commit
4b18183da2
|
|
@ -110,7 +110,7 @@ func (c *ConfigurationPerRequest) Check() error {
|
|||
def.Fields[fidx] = f
|
||||
|
||||
// Check for duplicate field definitions
|
||||
id, err := c.fieldID(seed, def.SlaveID, def.RegisterType, def.Measurement, f.Name)
|
||||
id, err := c.fieldID(seed, def, f.Name)
|
||||
if err != nil {
|
||||
return fmt.Errorf("cannot determine field id for %q: %v", f.Name, err)
|
||||
}
|
||||
|
|
@ -253,23 +253,23 @@ func (c *ConfigurationPerRequest) newFieldFromDefinition(def requestFieldDefinit
|
|||
return f, nil
|
||||
}
|
||||
|
||||
func (c *ConfigurationPerRequest) fieldID(seed maphash.Seed, slave byte, register, measurement, name string) (uint64, error) {
|
||||
func (c *ConfigurationPerRequest) fieldID(seed maphash.Seed, def requestDefinition, name string) (uint64, error) {
|
||||
var mh maphash.Hash
|
||||
mh.SetSeed(seed)
|
||||
|
||||
if err := mh.WriteByte(slave); err != nil {
|
||||
if err := mh.WriteByte(def.SlaveID); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if err := mh.WriteByte(0); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if _, err := mh.WriteString(register); err != nil {
|
||||
if _, err := mh.WriteString(def.RegisterType); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if err := mh.WriteByte(0); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if _, err := mh.WriteString(measurement); err != nil {
|
||||
if _, err := mh.WriteString(def.Measurement); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if err := mh.WriteByte(0); err != nil {
|
||||
|
|
@ -282,6 +282,25 @@ func (c *ConfigurationPerRequest) fieldID(seed maphash.Seed, slave byte, registe
|
|||
return 0, err
|
||||
}
|
||||
|
||||
// Tags
|
||||
for k, v := range def.Tags {
|
||||
if _, err := mh.WriteString(k); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if err := mh.WriteByte('='); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if _, err := mh.WriteString(v); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
if err := mh.WriteByte(':'); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
}
|
||||
if err := mh.WriteByte(0); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
|
||||
return mh.Sum64(), nil
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,16 +1,23 @@
|
|||
package modbus
|
||||
|
||||
import (
|
||||
"encoding/binary"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
mb "github.com/grid-x/modbus"
|
||||
"github.com/stretchr/testify/require"
|
||||
"github.com/tbrandon/mbserver"
|
||||
|
||||
"github.com/influxdata/telegraf"
|
||||
"github.com/influxdata/telegraf/config"
|
||||
"github.com/influxdata/telegraf/plugins/inputs"
|
||||
"github.com/influxdata/telegraf/plugins/parsers/influx"
|
||||
"github.com/influxdata/telegraf/testutil"
|
||||
)
|
||||
|
||||
|
|
@ -2044,3 +2051,123 @@ 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")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Prepare the influx parser for expectations
|
||||
parser := &influx.Parser{}
|
||||
require.NoError(t, parser.Init())
|
||||
|
||||
// Compare options
|
||||
options := []cmp.Option{
|
||||
testutil.IgnoreTime(),
|
||||
testutil.SortMetrics(),
|
||||
}
|
||||
|
||||
// Register the plugin
|
||||
inputs.Add("modbus", func() telegraf.Input { return &Modbus{} })
|
||||
|
||||
// Define a function to return the register value as data
|
||||
readFunc := func(s *mbserver.Server, frame mbserver.Framer) ([]byte, *mbserver.Exception) {
|
||||
data := frame.GetData()
|
||||
register := binary.BigEndian.Uint16(data[0:2])
|
||||
numRegs := binary.BigEndian.Uint16(data[2:4])
|
||||
|
||||
// Add the length in bytes and the register to the returned data
|
||||
buf := make([]byte, 2*numRegs+1)
|
||||
buf[0] = byte(2 * numRegs)
|
||||
switch numRegs {
|
||||
case 1: // 16-bit
|
||||
binary.BigEndian.PutUint16(buf[1:], register)
|
||||
case 2: // 32-bit
|
||||
binary.BigEndian.PutUint32(buf[1:], uint32(register))
|
||||
case 4: // 64-bit
|
||||
binary.BigEndian.PutUint64(buf[1:], uint64(register))
|
||||
}
|
||||
fmt.Println(buf)
|
||||
return buf, &mbserver.Success
|
||||
}
|
||||
|
||||
// Setup a Modbus server to test against
|
||||
serv := mbserver.NewServer()
|
||||
serv.RegisterFunctionHandler(mb.FuncCodeReadInputRegisters, readFunc)
|
||||
serv.RegisterFunctionHandler(mb.FuncCodeReadHoldingRegisters, readFunc)
|
||||
require.NoError(t, serv.ListenTCP("localhost:1502"))
|
||||
defer serv.Close()
|
||||
|
||||
// Run the test cases
|
||||
for _, f := range folders {
|
||||
// Only handle folders
|
||||
if !f.IsDir() {
|
||||
continue
|
||||
}
|
||||
testcasePath := filepath.Join("testcases", f.Name())
|
||||
configFilename := filepath.Join(testcasePath, "telegraf.conf")
|
||||
expectedOutputFilename := filepath.Join(testcasePath, "expected.out")
|
||||
expectedErrorFilename := filepath.Join(testcasePath, "expected.err")
|
||||
initErrorFilename := filepath.Join(testcasePath, "init.err")
|
||||
|
||||
t.Run(f.Name(), func(t *testing.T) {
|
||||
// Read the expected error for the init call if any
|
||||
var expectedInitError string
|
||||
if _, err := os.Stat(initErrorFilename); err == nil {
|
||||
e, err := testutil.ParseLinesFromFile(initErrorFilename)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, e, 1)
|
||||
expectedInitError = e[0]
|
||||
}
|
||||
|
||||
// Read the expected output if any
|
||||
var expected []telegraf.Metric
|
||||
if _, err := os.Stat(expectedOutputFilename); err == nil {
|
||||
var err error
|
||||
expected, err = testutil.ParseMetricsFromFile(expectedOutputFilename, parser)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
// Read the expected error if any
|
||||
var expectedErrors []string
|
||||
if _, err := os.Stat(expectedErrorFilename); err == nil {
|
||||
e, err := testutil.ParseLinesFromFile(expectedErrorFilename)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, e)
|
||||
expectedErrors = e
|
||||
}
|
||||
|
||||
// Configure the plugin
|
||||
cfg := config.NewConfig()
|
||||
require.NoError(t, cfg.LoadConfig(configFilename))
|
||||
require.Len(t, cfg.Inputs, 1)
|
||||
|
||||
// Extract the plugin and make sure it connects to our dummy
|
||||
// server
|
||||
plugin := cfg.Inputs[0].Input.(*Modbus)
|
||||
plugin.Controller = "tcp://localhost:1502"
|
||||
|
||||
// Init the plugin.
|
||||
err := plugin.Init()
|
||||
if expectedInitError != "" {
|
||||
require.ErrorContains(t, err, expectedInitError)
|
||||
return
|
||||
}
|
||||
require.NoError(t, err)
|
||||
|
||||
// Gather data
|
||||
var acc testutil.Accumulator
|
||||
require.NoError(t, plugin.Gather(&acc))
|
||||
if len(acc.Errors) > 0 {
|
||||
var actualErrorMsgs []string
|
||||
for _, err := range acc.Errors {
|
||||
actualErrorMsgs = append(actualErrorMsgs, err.Error())
|
||||
}
|
||||
require.ElementsMatch(t, actualErrorMsgs, expectedErrors)
|
||||
}
|
||||
|
||||
// Check the metric nevertheless as we might get some metrics despite errors.
|
||||
actual := acc.GetTelegrafMetrics()
|
||||
testutil.RequireMetricsEqual(t, expected, actual, options...)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,2 @@
|
|||
V,slave_id=1,element=EleMeter\ 1,name=Device,type=input_register Voltage=200i
|
||||
V,slave_id=2,element=EleMeter\ 2,name=Device,type=input_register Voltage=200i
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
[[inputs.modbus]]
|
||||
name = "Device"
|
||||
timeout = "1s"
|
||||
controller = "tcp://localhost:502"
|
||||
|
||||
configuration_type = "request"
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 1
|
||||
byte_order = "ABCD"
|
||||
register = "input"
|
||||
fields = [
|
||||
{address=200, name="Voltage", type="INT32", measurement="V", omit=false},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
element = "EleMeter 1"
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 2
|
||||
byte_order = "ABCD"
|
||||
register = "input"
|
||||
fields = [
|
||||
{address=200, name="Voltage", type="INT32", measurement="V", omit=false},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
element = "EleMeter 2"
|
||||
|
|
@ -0,0 +1,3 @@
|
|||
modbus,slave_id=1,name=Device,type=holding_register,location=Zone\ 1 humidity=1,temperature=4,active=7
|
||||
modbus,slave_id=1,name=Device,type=holding_register,location=Zone\ 2 humidity=2,temperature=5,active=8
|
||||
modbus,slave_id=1,name=Device,type=holding_register,location=Zone\ 3 humidity=3,temperature=6,active=9
|
||||
|
|
@ -0,0 +1,40 @@
|
|||
[[inputs.modbus]]
|
||||
name = "Device"
|
||||
controller = "tcp://localhost:502"
|
||||
configuration_type = "request"
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 1
|
||||
register = "holding"
|
||||
fields = [
|
||||
{ name = "humidity", type = "INT16", scale=1.0, address = 1},
|
||||
{ name = "temperature", type = "INT16", scale=1.0, address = 4},
|
||||
{ name = "active", type = "INT16", scale=1.0, address = 7},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
location = 'Zone 1'
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 1
|
||||
register = "holding"
|
||||
fields = [
|
||||
{ name = "humidity", type = "INT16", scale=1.0, address = 2},
|
||||
{ name = "temperature", type = "INT16", scale=1.0, address = 5},
|
||||
{ name = "active", type = "INT16", scale=1.0, address = 8},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
location = 'Zone 2'
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 1
|
||||
register = "holding"
|
||||
fields = [
|
||||
{ name = "humidity", type = "INT16", scale=1.0, address = 3},
|
||||
{ name = "temperature", type = "INT16", scale=1.0, address = 6},
|
||||
{ name = "active", type = "INT16", scale=1.0, address = 9},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
location = 'Zone 3'
|
||||
|
|
@ -0,0 +1 @@
|
|||
configuraton invalid: field "Voltage" duplicated in measurement "V"
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
[[inputs.modbus]]
|
||||
name = "Device"
|
||||
timeout = "1s"
|
||||
controller = "tcp://localhost:1502"
|
||||
|
||||
configuration_type = "request"
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 1
|
||||
byte_order = "ABCD"
|
||||
register = "input"
|
||||
fields = [
|
||||
{address=200, name="Voltage", type="FLOAT32", measurement="V", omit=false},
|
||||
{address=200, name="Voltage", type="FLOAT32", measurement="V", omit=false},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
element = "EleMeter 1"
|
||||
|
|
@ -0,0 +1 @@
|
|||
duplicated in measurement "modbus"
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
[[inputs.modbus]]
|
||||
name = "Device"
|
||||
controller = "tcp://localhost:502"
|
||||
configuration_type = "request"
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 1
|
||||
register = "holding"
|
||||
fields = [
|
||||
{ name = "humidity", type = "INT16", scale=1.0, address = 1},
|
||||
{ name = "temperature", type = "INT16", scale=1.0, address = 4},
|
||||
{ name = "active", type = "INT16", scale=1.0, address = 7},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
location = 'Zone 1'
|
||||
|
||||
[[inputs.modbus.request]]
|
||||
slave_id = 1
|
||||
register = "holding"
|
||||
fields = [
|
||||
{ name = "humidity", type = "INT16", scale=1.0, address = 2},
|
||||
{ name = "temperature", type = "INT16", scale=1.0, address = 5},
|
||||
{ name = "active", type = "INT16", scale=1.0, address = 8},
|
||||
]
|
||||
|
||||
[inputs.modbus.request.tags]
|
||||
location = 'Zone 1'
|
||||
Loading…
Reference in New Issue