You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2026/09/11 15:10:16 UTC
(plc4x) 04/05: fix(plc4go/modbus): report an unknown exception code as a remote error
This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit 40e0e8afbe1ae2f82e9c58fb00304b6f40d5e24b
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Fri Sep 11 16:57:22 2026 +0200
fix(plc4go/modbus): report an unknown exception code as a remote error
A Modbus exception code outside the mapped set hit a default branch that only
logged, leaving the tag with no response code at all. An absent tag is not an
error to the caller: DefaultPlcWriteResponse reports NOT_FOUND for it, so a
device that refused the write was indistinguishable from a tag the request never
carried -- and downstream that reads as "unknown tag" rather than "the device
said no".
Any unmapped exception code is now REMOTE_ERROR, matching what plc4j does in
getErrorCode.
---
plc4go/internal/modbus/Writer.go | 63 +++++++++++++++++++++++------------
plc4go/internal/modbus/Writer_test.go | 42 +++++++++++++++++++++++
2 files changed, 83 insertions(+), 22 deletions(-)
diff --git a/plc4go/internal/modbus/Writer.go b/plc4go/internal/modbus/Writer.go
index c1ef7fba62..92dbeda629 100644
--- a/plc4go/internal/modbus/Writer.go
+++ b/plc4go/internal/modbus/Writer.go
@@ -255,6 +255,40 @@ func (m *Writer) Write(ctx context.Context, writeRequest apiModel.PlcWriteReques
return result
}
+// responseCodeOf maps a modbus exception onto the response code the caller sees, exactly as plc4j's
+// ModbusTcpConnection.getErrorCode does. The second value is false for a code the specification
+// doesn't define, which is left to the caller to report - a read and a write have different things
+// to say about one.
+func responseCodeOf(exceptionCode readWriteModel.ModbusErrorCode) (apiModel.PlcResponseCode, bool) {
+ switch exceptionCode {
+ case readWriteModel.ModbusErrorCode_ILLEGAL_FUNCTION:
+ return apiModel.PlcResponseCode_UNSUPPORTED, true
+ case readWriteModel.ModbusErrorCode_ILLEGAL_DATA_ADDRESS:
+ return apiModel.PlcResponseCode_INVALID_ADDRESS, true
+ case readWriteModel.ModbusErrorCode_ILLEGAL_DATA_VALUE:
+ return apiModel.PlcResponseCode_INVALID_DATA, true
+ case readWriteModel.ModbusErrorCode_SLAVE_DEVICE_FAILURE:
+ return apiModel.PlcResponseCode_REMOTE_ERROR, true
+ case readWriteModel.ModbusErrorCode_ACKNOWLEDGE:
+ // The device took the request and will take its time over it. For a write that is as good
+ // as done; a read has nothing to show for it, which is why extractResponseData overrides
+ // this one.
+ return apiModel.PlcResponseCode_OK, true
+ case readWriteModel.ModbusErrorCode_SLAVE_DEVICE_BUSY:
+ return apiModel.PlcResponseCode_REMOTE_BUSY, true
+ case readWriteModel.ModbusErrorCode_NEGATIVE_ACKNOWLEDGE:
+ return apiModel.PlcResponseCode_REMOTE_ERROR, true
+ case readWriteModel.ModbusErrorCode_MEMORY_PARITY_ERROR:
+ return apiModel.PlcResponseCode_INTERNAL_ERROR, true
+ case readWriteModel.ModbusErrorCode_GATEWAY_PATH_UNAVAILABLE:
+ return apiModel.PlcResponseCode_INTERNAL_ERROR, true
+ case readWriteModel.ModbusErrorCode_GATEWAY_TARGET_DEVICE_FAILED_TO_RESPOND:
+ return apiModel.PlcResponseCode_REMOTE_ERROR, true
+ default:
+ return apiModel.PlcResponseCode_INTERNAL_ERROR, false
+ }
+}
+
// ToPlc4xWriteResponse turns the ADU a device answered with into a PLC4X response. It takes the
// ADUs through the little bit of them this needs, so that it serves every flavor - the generated
// ModbusADU parent type carries no PDU accessor of its own.
@@ -317,30 +351,15 @@ func (m *Writer) toPlc4xWriteResponse(requestPdu readWriteModel.ModbusPDU, respo
}
responseCodes[tagName] = apiModel.PlcResponseCode_OK
case readWriteModel.ModbusPDUError:
- switch resp.GetExceptionCode() {
- case readWriteModel.ModbusErrorCode_ILLEGAL_FUNCTION:
- responseCodes[tagName] = apiModel.PlcResponseCode_UNSUPPORTED
- case readWriteModel.ModbusErrorCode_ILLEGAL_DATA_ADDRESS:
- responseCodes[tagName] = apiModel.PlcResponseCode_INVALID_ADDRESS
- case readWriteModel.ModbusErrorCode_ILLEGAL_DATA_VALUE:
- responseCodes[tagName] = apiModel.PlcResponseCode_INVALID_DATA
- case readWriteModel.ModbusErrorCode_SLAVE_DEVICE_FAILURE:
- responseCodes[tagName] = apiModel.PlcResponseCode_REMOTE_ERROR
- case readWriteModel.ModbusErrorCode_ACKNOWLEDGE:
- responseCodes[tagName] = apiModel.PlcResponseCode_OK
- case readWriteModel.ModbusErrorCode_SLAVE_DEVICE_BUSY:
- responseCodes[tagName] = apiModel.PlcResponseCode_REMOTE_BUSY
- case readWriteModel.ModbusErrorCode_NEGATIVE_ACKNOWLEDGE:
- responseCodes[tagName] = apiModel.PlcResponseCode_REMOTE_ERROR
- case readWriteModel.ModbusErrorCode_MEMORY_PARITY_ERROR:
- responseCodes[tagName] = apiModel.PlcResponseCode_INTERNAL_ERROR
- case readWriteModel.ModbusErrorCode_GATEWAY_PATH_UNAVAILABLE:
- responseCodes[tagName] = apiModel.PlcResponseCode_INTERNAL_ERROR
- case readWriteModel.ModbusErrorCode_GATEWAY_TARGET_DEVICE_FAILED_TO_RESPOND:
- responseCodes[tagName] = apiModel.PlcResponseCode_REMOTE_ERROR
- default:
+ responseCode, mapped := responseCodeOf(resp.GetExceptionCode())
+ if !mapped {
+ // An exception code the specification doesn't define. The device refused; which way it
+ // spelled that is all that is unknown. Leaving the tag out of the map instead would have
+ // the caller read it back as NOT_FOUND, which says the tag was never written at all.
m.log.Debug().Interface("exceptionCode", resp.GetExceptionCode()).Msg("Unmapped exception code")
+ responseCode = apiModel.PlcResponseCode_REMOTE_ERROR
}
+ responseCodes[tagName] = responseCode
default:
return nil, errors.Errorf("unsupported response type %T", resp)
}
diff --git a/plc4go/internal/modbus/Writer_test.go b/plc4go/internal/modbus/Writer_test.go
index 337d305629..c98c4f5d3e 100644
--- a/plc4go/internal/modbus/Writer_test.go
+++ b/plc4go/internal/modbus/Writer_test.go
@@ -274,6 +274,48 @@ func TestWriter_singleWriteEchoValidation(t *testing.T) {
}
}
+// Every exception a device answers with has to reach the caller as a response code. An exception
+// code the specification doesn't define is still the device refusing the write, so it becomes
+// REMOTE_ERROR - leaving the tag out of the response map instead has GetResponseCode report it as
+// NOT_FOUND, which reads as "no such tag" rather than "the device said no".
+func TestWriter_exceptionResponseCodes(t *testing.T) {
+ writer := NewWriter(DefaultConfiguration(), newCaptureCodec(nil))
+ tests := []struct {
+ name string
+ exceptionCode readWriteModel.ModbusErrorCode
+ expected apiModel.PlcResponseCode
+ }{
+ {
+ "illegal data address",
+ readWriteModel.ModbusErrorCode_ILLEGAL_DATA_ADDRESS,
+ apiModel.PlcResponseCode_INVALID_ADDRESS,
+ },
+ {
+ "slave device busy",
+ readWriteModel.ModbusErrorCode_SLAVE_DEVICE_BUSY,
+ apiModel.PlcResponseCode_REMOTE_BUSY,
+ },
+ {
+ // 0x09 is one of the gaps the specification leaves between the codes it defines.
+ "a code outside the known set",
+ readWriteModel.ModbusErrorCode(9),
+ apiModel.PlcResponseCode_REMOTE_ERROR,
+ },
+ }
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ request := writeRequestFor(t, writer, NewTag(HoldingRegister, 3, 1, readWriteModel.ModbusDataType_UINT), spiValues.NewPlcUINT(42))
+ response, err := writer.ToPlc4xWriteResponse(
+ readWriteModel.NewModbusTcpADU(1, 1, readWriteModel.NewModbusPDUWriteSingleRegisterRequest(2, 42)),
+ readWriteModel.NewModbusTcpADU(1, 1, readWriteModel.NewModbusPDUError(test.exceptionCode)),
+ request,
+ )
+ require.NoError(t, err)
+ assert.Equal(t, test.expected, response.GetResponseCode("tag"))
+ })
+ }
+}
+
// A response PDU that doesn't belong to the request we sent must be an error rather than a panic
// on an unchecked type assertion.
func TestWriter_mismatchedRequestPduIsAnError(t *testing.T) {