From 9747d80b0d532620a5350c79557872112113eb0d Mon Sep 17 00:00:00 2001 From: toimtoimtoim Date: Wed, 19 Mar 2025 23:36:33 +0200 Subject: [PATCH] Added `poller.Config.OnClientDoErrorFunc` callback to improve error handling and statistics. Breaking change - packet.ErrUnknown etc type changed from uint8 to packet.ErrCode + same with related functions --- README.md | 2 +- cmd/modbus-poller/main.go | 2 +- field.go | 39 ++++++++++++++++++ field_test.go | 76 ++++++++++++++++++++++++++++++++++ packet/error.go | 56 ++++++++++++++++--------- packet/error_test.go | 10 +++++ poller/poller.go | 56 +++++++++++++++++++++---- poller/poller_test.go | 87 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 300 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index a820858..9c12561 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,7 @@ func main() { defer cancel() b := modbus.NewRequestBuilderWithConfig(modbus.BuilderDefaults{ - ServerAddress: "tcp://127.0.0.1:5022", + ServerAddress: "tcp://127.0.0.1:5022?invalid_addr=1000,12000-12100&read_timeout=2s", FunctionCode: packet.FunctionReadHoldingRegisters, // fc3 UnitID: 1, Protocol: modbus.ProtocolTCP, diff --git a/cmd/modbus-poller/main.go b/cmd/modbus-poller/main.go index cea6679..a5d2c82 100644 --- a/cmd/modbus-poller/main.go +++ b/cmd/modbus-poller/main.go @@ -18,7 +18,7 @@ Example `config.json` content to poll "Victron Energy Meter VM-3P75CT" over UDP { "defaults": { - "server_address": "udp://192.168.0.200:502", + "server_address": "udp://192.168.0.200:502?invalid_addr=1000,12000-12100&read_timeout=1s", "function_code": 3, "unit_id": 1, "protocol": "tcp", diff --git a/field.go b/field.go index 5241c5a..4563481 100644 --- a/field.go +++ b/field.go @@ -135,8 +135,18 @@ type Field struct { // Only relevant to register function fields Bit uint8 `json:"bit" mapstructure:"bit"` + // FromHighByte is for single byte data types stored in single register (e.g. byte,uint8,int8) + // + // In Modbus (which uses big-endian format), the most significant byte is + // sent first and is therefore considered the 0th byte. The least significant byte + // is sent second and is considered the 1st byte. + // + // Modbus register with value `0x1234`. + // - 0x12 is High Byte, 0th byte + // - 0x34 is Low byte, is the 1st byte FromHighByte bool `json:"from_high_byte" mapstructure:"from_high_byte"` + // Length is length of string and raw bytes data types. Length uint8 `json:"length" mapstructure:"length"` @@ -144,6 +154,19 @@ type Field struct { // Invalid that represents not existent value in modbus. Given value (presented in hex) when encountered is converted to ErrInvalidValue error. // for example your energy meter ac power is uint32 value of which `0xffffffff` should be treated as error/invalid value. + // + // Usually invalid value is largest unsigned or smallest signed value per data type. Example: + // - uint8 = 0xff (255) + // - int8 = 0x80 (-127) + // - uint16 = 0xff, 0xff (65535) + // - int16 = 0x80, 0x00 (-32768) + // - uint32 = 0xff, 0xff, 0xff, 0xff (4294967295) + // - int32 = 0x80, 0x0, 0x0, 0x0 (-2147483648) + // - uint64 = 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff (18446744073709551615) + // - int64 = 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 (-9223372036854775808) + // - float32 is same as uint32 + // - float64 is same as uint64 + // - bit, boolean - can not have invalid values Invalid Invalid `json:"invalid,omitempty" mapstructure:"invalid"` } @@ -328,6 +351,22 @@ func (f *Field) CheckInvalid(registers *packet.Registers) error { if f.Invalid == nil { return nil } + + if f.Type == FieldTypeByte || f.Type == FieldTypeUint8 || f.Type == FieldTypeInt8 { + regData, err := registers.Register(f.Address) + if err != nil { + return err + } + b := regData[1] + if f.FromHighByte { + b = regData[0] + } + if b == f.Invalid[0] { + return ErrInvalidValue + } + return nil + } + ok, err := registers.IsEqualBytes(f.Address, uint8(f.registerSize()*2), f.Invalid) if err != nil { return err diff --git a/field_test.go b/field_test.go index 2d06141..76d8717 100644 --- a/field_test.go +++ b/field_test.go @@ -510,6 +510,21 @@ func TestParseFieldType(t *testing.T) { } func TestField_CheckInvalid(t *testing.T) { + //exampleData := []byte{ + // 0b00010001, 0x0, // bit // 0 + // 0x3, 0x4, // byte // 1 + // 0xFF, 0x80, // uint8, int8 // 2 + // 0xff, 0xff, // uint16 // 3 + // 0x80, 0x00, // int16 // 4 + // 0xff, 0xff, 0xff, 0xff, // uint32 // 5,6 + // 0x80, 0x0, 0x0, 0x0, // int32 // 7,8 + // 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // uint64 // 9,10,11,12 + // 0x80, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // int64 // 13,14,15,16 + // 0xff, 0xff, 0xff, 0xff, // float32 // 17,18 + // 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // float64 // 19,20,21,22 + // 0x56, 0x53, 0x83, 0x43, // string(3) // 56=V 53=S 83=ƒ 43=C + //} + var testCases = []struct { name string when Field @@ -517,6 +532,67 @@ func TestField_CheckInvalid(t *testing.T) { givenStartAddress uint16 expectErr string }{ + { + name: "is invalid byte, low byte", + when: Field{Address: 1, Type: FieldTypeByte, Invalid: []byte{0xff}}, + givenData: []byte{0x1, 0x2, 0x3, 0xff, 0x5, 0x6}, + expectErr: "invalid value", + }, + { + name: "not invalid byte, low byte", + when: Field{Address: 1, Type: FieldTypeByte, Invalid: []byte{0xff}}, + givenData: []byte{0x1, 0x2, 0x3, 0x4, 0x5, 0x6}, + expectErr: "", + }, + { + name: "is invalid byte, high byte", + when: Field{Address: 1, Type: FieldTypeByte, Invalid: []byte{0xff}, FromHighByte: true}, + givenData: []byte{0x1, 0x2, 0xff, 0x4, 0x5, 0x6}, + expectErr: "invalid value", + }, + { + name: "not invalid byte, high byte", + when: Field{Address: 1, Type: FieldTypeByte, Invalid: []byte{0xff}, FromHighByte: true}, + givenData: []byte{0x1, 0x2, 0x3, 0x4, 0x5, 0x6}, + expectErr: "", + }, + { + name: "is invalid uint8, low byte", + when: Field{Address: 1, Type: FieldTypeUint8, Invalid: []byte{0xff}}, + givenData: []byte{0x1, 0x2, 0x3, 0xff, 0x5, 0x6}, + expectErr: "invalid value", + }, + { + name: "is invalid uint8, high byte", + when: Field{Address: 1, Type: FieldTypeUint8, Invalid: []byte{0xff}, FromHighByte: true}, + givenData: []byte{0x1, 0x2, 0xff, 0x4, 0x5, 0x6}, + expectErr: "invalid value", + }, + { + name: "is invalid uint8, low byte", + when: Field{Address: 1, Type: FieldTypeInt8, Invalid: []byte{0x80}}, + givenData: []byte{0x1, 0x2, 0x3, 0x80, 0x5, 0x6}, + expectErr: "invalid value", + }, + { + name: "is invalid uint8, high byte", + when: Field{Address: 1, Type: FieldTypeInt8, Invalid: []byte{0x80}, FromHighByte: true}, + givenData: []byte{0x1, 0x2, 0x80, 0x4, 0x5, 0x6}, + expectErr: "invalid value", + }, + { + name: "is invalid uint16", + when: Field{Address: 1, Type: FieldTypeUint16, Invalid: []byte{0xff, 0xff}}, + givenData: []byte{0x1, 0x2, 0xff, 0xff, 0x5, 0x6}, + expectErr: "invalid value", + }, + { + name: "is invalid int16", + when: Field{Address: 1, Type: FieldTypeInt16, Invalid: []byte{0x80, 0x00}}, + givenData: []byte{0x1, 0x2, 0x80, 0x0, 0x5, 0x6}, + expectErr: "invalid value", + }, + { name: "ok", when: Field{Address: 3, Type: FieldTypeInt16, Invalid: []byte{0xff, 0xff}}, diff --git a/packet/error.go b/packet/error.go index 0d801a9..de8ccc1 100644 --- a/packet/error.go +++ b/packet/error.go @@ -10,13 +10,13 @@ type ErrCode uint8 const ( // ErrUnknown is catchall error code - ErrUnknown = 0 + ErrUnknown ErrCode = 0 // ErrIllegalFunction is The function code received in the query is not an allowable action for the server. // This may be because the function code is only applicable to newer devices, and was not implemented in the // unit selected. It could also indicate that the server is in the wrong state to process a request of this // type, for example because it is not configured and is being asked to return register values. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 48 - ErrIllegalFunction = 1 + ErrIllegalFunction ErrCode = 1 // ErrIllegalDataAddress is The data address received in the query is not an allowable address for the server. // More specifically, the combination of reference number and transfer length is invalid. For a controller with 100 // registers, the PDU addresses the first register as 0, and the last one as 99. If a request is submitted with a @@ -26,45 +26,45 @@ const ( // Code 0x02 “Illegal Data Address” since it attempts to operate on registers 96, 97, 98, 99 and 100, and // there is no register with address 100. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 48 - ErrIllegalDataAddress = 2 + ErrIllegalDataAddress ErrCode = 2 // ErrIllegalDataValue is A value contained in the query data field is not an allowable value for server. // This indicates a fault in the structure of the remainder of a complex request, such as that the implied length // is incorrect. It specifically does NOT mean that a data item submitted for storage in a register has a value // outside the expectation of the application program, since the MODBUS protocol is unaware of the significance of // any particular value of any particular register. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 48 - ErrIllegalDataValue = 3 + ErrIllegalDataValue ErrCode = 3 // ErrServerFailure is An unrecoverable error occurred while the server was attempting to perform the requested action. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 48 - ErrServerFailure = 4 + ErrServerFailure ErrCode = 4 // ErrAcknowledge is Specialized use in conjunction with programming commands. The server has accepted the request // and is processing it, but a long duration of time will be required to do so. This response is returned to prevent // a timeout error from occurring in the client. The client can next issue a Poll Program Complete message to // determine if processing is completed. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 48 - ErrAcknowledge = 5 + ErrAcknowledge ErrCode = 5 // ErrServerBusy is Specialized use in conjunction with programming commands. The server is engaged in processing a // long duration program command. The client should retransmit the message later when the server is free. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 48 - ErrServerBusy = 6 + ErrServerBusy ErrCode = 6 // ErrMemoryParityError is Specialized use in conjunction with function codes 20 and 21 and reference type 6, to // indicate that the extended file area failed to pass a consistency check. // The server attempted to read record file, but detected a parity error in the memory. The client can retry // the request, but service may be required on the server device. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 48 - ErrMemoryParityError = 8 + ErrMemoryParityError ErrCode = 8 // ErrGatewayPathUnavailable is Specialized use in conjunction with gateways, indicates that the gateway was unable // to allocate an internal communication path from the input port to the output port for processing the request. // Usually means that the gateway is misconfigured or overloaded. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 49 - ErrGatewayPathUnavailable = 10 + ErrGatewayPathUnavailable ErrCode = 10 // ErrGatewayTargetedDeviceResponse is Specialized use in conjunction with gateways, indicates that no response was // obtained from the target device. Usually means that the device is not present on the network. // Quote from: `MODBUS Application Protocol Specification V1.1b3`, page 49 - ErrGatewayTargetedDeviceResponse = 11 + ErrGatewayTargetedDeviceResponse ErrCode = 11 ) -func errorText(code uint8) string { +func errorText(code ErrCode) string { switch code { case ErrIllegalFunction: return "Illegal function" @@ -91,8 +91,16 @@ func errorText(code uint8) string { } } +// ModbusError allows distinguishing Modbus error responses (response with error code) +// from other (i.e. network related or parsing the response) errors when requesting data +// from modbus server. +type ModbusError interface { + Error() string + ErrorCode() ErrCode +} + // NewErrorParseTCP creates new instance of parsing error that can be sent to the client -func NewErrorParseTCP(code uint8, message string) *ErrorParseTCP { +func NewErrorParseTCP(code ErrCode, message string) *ErrorParseTCP { return &ErrorParseTCP{ Message: message, Packet: ErrorResponseTCP{ @@ -125,7 +133,7 @@ type ErrorResponseTCP struct { TransactionID uint16 UnitID uint8 Function uint8 - Code uint8 + Code ErrCode } // Error translates error code to error message. @@ -142,7 +150,7 @@ func (re ErrorResponseTCP) Bytes() []byte { binary.BigEndian.PutUint16(result[4:6], 3) result[6] = re.UnitID result[7] = re.Function + functionCodeErrorBitmask - result[8] = re.Code + result[8] = uint8(re.Code) return result } @@ -152,8 +160,13 @@ func (re ErrorResponseTCP) FunctionCode() uint8 { return re.Function } +// ErrorCode returns error code returned by modbus server +func (re ErrorResponseTCP) ErrorCode() ErrCode { + return re.Code +} + // NewErrorParseRTU creates new instance of parsing error that can be sent to the client -func NewErrorParseRTU(code uint8, message string) *ErrorParseRTU { +func NewErrorParseRTU(code ErrCode, message string) *ErrorParseRTU { return &ErrorParseRTU{ Message: message, Packet: ErrorResponseRTU{ @@ -184,7 +197,7 @@ func (e ErrorParseRTU) Bytes() []byte { type ErrorResponseRTU struct { UnitID uint8 Function uint8 - Code uint8 + Code ErrCode } // Error translates error code to error message. @@ -198,7 +211,7 @@ func (re ErrorResponseRTU) Bytes() []byte { result[0] = re.UnitID result[1] = re.Function + functionCodeErrorBitmask - result[2] = re.Code + result[2] = uint8(re.Code) crc := CRC16(result[0:3]) result[3] = uint8(crc) result[4] = uint8(crc >> 8) @@ -211,6 +224,11 @@ func (re ErrorResponseRTU) FunctionCode() uint8 { return re.Function } +// ErrorCode returns error code returned by modbus server +func (re ErrorResponseRTU) ErrorCode() ErrCode { + return re.Code +} + // AsTCPErrorPacket converts raw packet bytes to Modbus TCP error response if possible // // Example packet: 0xda 0x87 0x00 0x00 0x00 0x03 0x01 0x81 0x03 @@ -233,7 +251,7 @@ func AsTCPErrorPacket(data []byte) error { TransactionID: binary.BigEndian.Uint16(data[0:2]), UnitID: data[6], Function: data[7] - functionCodeErrorBitmask, - Code: data[8], + Code: ErrCode(data[8]), } } return nil // probably start of valid packet @@ -258,7 +276,7 @@ func AsRTUErrorPacket(data []byte) error { return &ErrorResponseRTU{ UnitID: data[0], Function: data[1] - functionCodeErrorBitmask, - Code: data[2], + Code: ErrCode(data[2]), } } return nil // probably start of valid packet diff --git a/packet/error_test.go b/packet/error_test.go index 8d8adcf..f927175 100644 --- a/packet/error_test.go +++ b/packet/error_test.go @@ -123,6 +123,16 @@ func TestErrorResponseRTU_FunctionCode(t *testing.T) { assert.Equal(t, uint8(1), given.FunctionCode()) } +func TestErrorResponseTCP_ModbusErrorCode(t *testing.T) { + given := ErrorResponseTCP{Function: 1, Code: ErrIllegalDataAddress} + assert.Equal(t, ErrIllegalDataAddress, given.ErrorCode()) +} + +func TestErrorResponseRTU_ModbusErrorCode(t *testing.T) { + given := ErrorResponseRTU{Function: 1, Code: ErrIllegalDataAddress} + assert.Equal(t, ErrIllegalDataAddress, given.ErrorCode()) +} + func TestErrorResponseTCP_Bytes(t *testing.T) { var testCases = []struct { name string diff --git a/poller/poller.go b/poller/poller.go index bb7c175..7706897 100644 --- a/poller/poller.go +++ b/poller/poller.go @@ -46,6 +46,15 @@ type Config struct { // Defaults to DefaultConnectClient ConnectFunc func(ctx context.Context, batchProtocol modbus.ProtocolType, address string) (Client, error) + // OnClientDoErrorFunc is called when Client.Do returns with an error. + // User can decide do suppress certain errors by not returning from this function. In that + // case these errors will not be included in statistics. + // + // Use-case for this callback: + // Some engine controllers will return packet.ErrIllegalDataValue error code when the engine + // is not turned on, and you might not want to pollute logs with modbus errors like that. + OnClientDoErrorFunc func(err error, batchIndex int) error + // TimeNow allows mocking Result.Time value in tests // Defaults to time.Now TimeNow func() time.Time @@ -72,9 +81,10 @@ func NewPollerWithConfig(batches []modbus.BuilderRequest, conf Config) *Poller { } for i, batch := range batches { p.jobs[i] = job{ - timeNow: timeNow, - logger: p.logger, - connectFunc: p.connectFunc, + timeNow: timeNow, + logger: p.logger, + connectFunc: p.connectFunc, + onClientDoErrorFunc: conf.OnClientDoErrorFunc, stats: jobBatchStatistics{ lock: sync.RWMutex{}, @@ -134,9 +144,10 @@ func (p *Poller) Poll(ctx context.Context) error { } type job struct { - timeNow func() time.Time - logger *slog.Logger - connectFunc func(ctx context.Context, batchProtocol modbus.ProtocolType, address string) (Client, error) + timeNow func() time.Time + logger *slog.Logger + connectFunc func(ctx context.Context, batchProtocol modbus.ProtocolType, address string) (Client, error) + onClientDoErrorFunc func(err error, batchIndex int) error batchIndex int batch modbus.BuilderRequest @@ -215,10 +226,27 @@ func (j *job) poll(ctx context.Context) error { start := j.timeNow() resp, err := client.Do(ctx, batch.Request) reqDuration := j.timeNow().Sub(start) + + if err != nil && j.onClientDoErrorFunc != nil { + // user can decide do suppress certain errors + // for example some vessel engine controllers will return packet.ErrIllegalDataValue + // error code when engine is not turned on, and you might not want to pollute + // logs with modbus errors like that + err = j.onClientDoErrorFunc(err, j.batchIndex) + if err == nil { + continue + } + } + if err != nil { countDoErr++ j.stats.IncRequestErrCount() + var mbErr packet.ModbusError + if errors.As(err, &mbErr) { + j.stats.IncRequestModbusErrCount() + } + j.logger.Error("request failed", "err", err, "req_duration", reqDuration, @@ -341,12 +369,20 @@ type BatchStatistics struct { // IsPolling shows if that batch job currently in polling or waiting for retry IsPolling bool + // StartCount is count how many times the poll job has (re)started StartCount uint64 + // RequestOKCount is count how many modbus request have succeeded for that job RequestOKCount uint64 - // RequestErrCount is count how many modbus request have failed for that job + + // RequestErrCount is total count how many request have failed for that job + // this count does not distinguish modbus errors from network errors RequestErrCount uint64 + + // RequestModbusErrCount is count how many request have failed with modbus error code for that job + RequestModbusErrCount uint64 + // SendSkipCount is count how many ResultChan sends were skipped due blocked Result channel SendSkipCount uint64 } @@ -380,6 +416,12 @@ func (j *jobBatchStatistics) IncRequestErrCount() { j.stats.RequestErrCount++ } +func (j *jobBatchStatistics) IncRequestModbusErrCount() { + j.lock.Lock() + defer j.lock.Unlock() + j.stats.RequestModbusErrCount++ +} + func (j *jobBatchStatistics) IncSendSkipCount() { j.lock.Lock() defer j.lock.Unlock() diff --git a/poller/poller_test.go b/poller/poller_test.go index c6a1cf9..001986f 100644 --- a/poller/poller_test.go +++ b/poller/poller_test.go @@ -133,3 +133,90 @@ func TestNewPollerWithConfig(t *testing.T) { assert.Equal(t, expect, result) } + +func TestPoller_PollWithError(t *testing.T) { + f1 := modbus.Field{ + Name: "f1", + Address: 10, + Type: modbus.FieldTypeInt16, + ServerAddress: "server", + FunctionCode: packet.FunctionReadHoldingRegisters, + UnitID: 1, + Protocol: modbus.ProtocolTCP, + RequestInterval: modbus.Duration(50 * time.Millisecond), + } + b := modbus.NewRequestBuilder("x", 1) + b.AddField(f1) + batches, err := b.Split() + if !assert.NoError(t, err) { + return + } + + var testCases = []struct { + name string + whenResponse packet.Response + whenResponseErr error + expectStats []BatchStatistics + }{ + { + name: "ok", + whenResponseErr: &packet.ErrorResponseTCP{ + TransactionID: 1245, + UnitID: 1, + Function: 2, + Code: packet.ErrIllegalDataAddress, + }, + expectStats: []BatchStatistics{ + { + BatchIndex: 0, + FunctionCode: 0x3, + Protocol: 0x1, + ServerAddress: "server", + IsPolling: false, + StartCount: 0x1, + RequestOKCount: 0x0, + RequestErrCount: 0x2, + RequestModbusErrCount: 0x1, + SendSkipCount: 0x0, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + client := &mockClient{ + onDo: func(doCount int, req packet.Request) (packet.Response, error) { + if doCount > 1 { + cancel() // second request will end the test + return nil, errors.New("end") + } + return tc.whenResponse, tc.whenResponseErr + }, + } + + logger := slog.New(slog.NewJSONHandler(os.Stdout, nil)) + testTime := time.Unix(1615662935, 0).In(time.UTC) // 2021-03-13T19:15:35+00:00 + conf := Config{ + Logger: logger, + ConnectFunc: func(ctx context.Context, batchProtocol modbus.ProtocolType, address string) (Client, error) { + return client, nil + }, + OnClientDoErrorFunc: func(err error, batchIndex int) error { + return err + }, + TimeNow: func() time.Time { return testTime }, + } + p := NewPollerWithConfig(batches, conf) + assert.Len(t, p.jobs, 1) + + err = p.Poll(ctx) + assert.NoError(t, err) + + actual := p.BatchStatistics() + assert.Equal(t, tc.expectStats, actual) + }) + } +}