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/07 14:58:34 UTC
(plc4x-extras) branch develop updated (8dd8a01e -> 64c05ab1)
This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a change to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x-extras.git
from 8dd8a01e test: Pin plc4x's Modbus exception-response defect
new a6c53cae build: Bump plc4go past the discriminator fix, and flip the Modbus test
new 64c05ab1 docs: Correct the count of exception responses in the public capture
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
plc4go/go.mod | 4 +-
plc4go/go.sum | 4 +-
.../internal/analyzer/protocols_test.go | 60 +++++++++-------------
.../internal/pcapfixture/vectors.go | 25 ++++-----
4 files changed, 42 insertions(+), 51 deletions(-)
(plc4x-extras) 01/02: build: Bump plc4go past the discriminator fix, and flip the Modbus test
Posted by sr...@apache.org.
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-extras.git
commit a6c53cae10dcf118678650e6704a025615b06b92
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Mon Sep 7 16:46:19 2026 +0200
build: Bump plc4go past the discriminator fix, and flip the Modbus test
plc4go moves to v0.0.0-20260907131220-8ed193b9438b, which is the first version
carrying the code generator fix for a discriminator value that no sub type
pins. Modbus exception responses now keep the function code that failed, so
0x83 and 0x88 round-trip instead of both collapsing to 0x80.
TestAModbusExceptionResponseIsReportedAsADefect asserted the analyzer reported
those two packets, and it said in its own comment that the day they came back
clean was the day to delete it. Rather than delete the exchange along with the
assertion, it becomes TestModbusExceptionResponsesRoundTrip: the payloads are
real exception responses captured in the wild, nothing else in the suite covers
them, and asserting that they now survive is what stops the defect returning
unnoticed on a later bump.
go mod tidy also promotes github.com/pkg/errors from indirect to direct, which
it has been since the analyzer's finding, codec and report packages were added.
---
plc4go/go.mod | 4 +-
plc4go/go.sum | 4 +-
.../internal/analyzer/protocols_test.go | 60 +++++++++-------------
.../internal/pcapfixture/vectors.go | 23 +++++----
4 files changed, 41 insertions(+), 50 deletions(-)
diff --git a/plc4go/go.mod b/plc4go/go.mod
index 90046c0e..8297f47e 100644
--- a/plc4go/go.mod
+++ b/plc4go/go.mod
@@ -25,10 +25,11 @@ require (
charm.land/bubbles/v2 v2.2.1
charm.land/bubbletea/v2 v2.0.9
charm.land/lipgloss/v2 v2.0.6
- github.com/apache/plc4x/plc4go v0.0.0-20260902093211-931674134ff2
+ github.com/apache/plc4x/plc4go v0.0.0-20260907131220-8ed193b9438b
github.com/charmbracelet/fang v1.0.0
github.com/charmbracelet/x/exp/teatest/v2 v2.0.0-20260902165432-6f6ad8b37b0a
github.com/gopacket/gopacket v1.7.1
+ github.com/pkg/errors v0.9.1
github.com/rs/zerolog v1.35.1
github.com/spf13/cobra v1.10.2
github.com/spf13/viper v1.21.0
@@ -65,7 +66,6 @@ require (
github.com/muesli/roff v0.1.0 // indirect
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
github.com/pelletier/go-toml/v2 v2.2.4 // indirect
- github.com/pkg/errors v0.9.1 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/rogpeppe/go-internal v1.11.0 // indirect
github.com/sagikazarmark/locafero v0.12.0 // indirect
diff --git a/plc4go/go.sum b/plc4go/go.sum
index 82f7d7c7..77f52995 100644
--- a/plc4go/go.sum
+++ b/plc4go/go.sum
@@ -6,8 +6,8 @@ charm.land/lipgloss/v2 v2.0.6 h1:EaGKeuA8FvF+v2BT5VmZd2LoYLaMZJXA5n34th8nCIQ=
charm.land/lipgloss/v2 v2.0.6/go.mod h1:ipDDJNSGa1hlwDtSfW1s2/xR8Vdhbut4PXh2zEKZd0Q=
github.com/ajankovic/xdiff v0.0.1 h1:V1cj8t5xwYzm6ZGPqPOlAc9AIajXuTEn41D/1MJBWMM=
github.com/ajankovic/xdiff v0.0.1/go.mod h1:SUmEZ67uB97I0zkiuQ+lb+LOms9ipn8X+p+2RdJV710=
-github.com/apache/plc4x/plc4go v0.0.0-20260902093211-931674134ff2 h1:AehpEo6hGU1fFryP9ZQtkwhiQom9jqbgB1Lhp1CBfjs=
-github.com/apache/plc4x/plc4go v0.0.0-20260902093211-931674134ff2/go.mod h1:eXckuqFuXTmT05hN7BrZUIthBY96xSoXMXJNt//fHXo=
+github.com/apache/plc4x/plc4go v0.0.0-20260907131220-8ed193b9438b h1:wDfhQwN2ElvP9lsEdisfdChMduL2ilh7J+aDLgQJjIE=
+github.com/apache/plc4x/plc4go v0.0.0-20260907131220-8ed193b9438b/go.mod h1:eXckuqFuXTmT05hN7BrZUIthBY96xSoXMXJNt//fHXo=
github.com/atotto/clipboard v0.1.4 h1:EH0zSVneZPSuFR11BlR9YppQTVDbh5+16AmcJi4g1z4=
github.com/atotto/clipboard v0.1.4/go.mod h1:ZY9tmq7sm5xIbd9bOK4onWV4S6X0u6GY7Vn0Yu86PYI=
github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ12Gv5o=
diff --git a/plc4go/tools/plc4xpcapanalyzer/internal/analyzer/protocols_test.go b/plc4go/tools/plc4xpcapanalyzer/internal/analyzer/protocols_test.go
index 7ee02b46..1858ecf4 100644
--- a/plc4go/tools/plc4xpcapanalyzer/internal/analyzer/protocols_test.go
+++ b/plc4go/tools/plc4xpcapanalyzer/internal/analyzer/protocols_test.go
@@ -269,51 +269,41 @@ func analyseWithStderr(t *testing.T, capture, protocolName, client string, stder
return findings, counters
}
-// TestAModbusExceptionResponseIsReportedAsADefect records a defect in plc4x, deliberately.
+// TestModbusExceptionResponsesRoundTrip pins a plc4x fix that this test used to record as a defect.
//
-// Modbus flags an error by setting the top bit of the original function code: 0x03 becomes 0x83.
-// plc4x's ModbusPDUError keeps only the flag, so it writes 0x80 back whatever went in -- its own
-// parser reads the function correctly and its own serializer then discards it. The defect is in
-// the shared protocol definition, not the Go binding: the generated Java has a hard-coded
-// "getFunctionFlag() { return 0; }".
+// Modbus flags an error by setting the top bit of the original function code, so 0x03 becomes 0x83
+// and 0x08 becomes 0x88. plc4x used to write 0x80 back whatever went in: the typeSwitch case for
+// ModbusPDUError matches on the error bit alone, which left the code generator with no constant for
+// the function code, so it emitted a zero. Its own parser read the function correctly and its own
+// serializer then discarded it. Fixed upstream in the generator, which now retains a discriminator
+// value that no sub type pins.
//
-// This test asserts the analyzer NOTICES, which is the tool's whole purpose and is worth pinning
-// in both directions:
-//
-// - if it stops reporting these, the tool has regressed and would report a corrupted
-// re-serialization as a clean round trip
-// - if it starts reporting them as clean, plc4x has been fixed upstream. That is good news,
-// and this test is where it will be noticed. Delete it then, and say so in the message.
-func TestAModbusExceptionResponseIsReportedAsADefect(t *testing.T) {
+// The exchange stays here as a regression vector rather than being deleted with the defect, because
+// these are real exception responses and nothing else in the suite covers them. A failure here means
+// either the fix was lost in a plc4go bump or the analyzer has started reporting clean traffic.
+func TestModbusExceptionResponsesRoundTrip(t *testing.T) {
packets := pcapfixture.ModbusExceptionSession()
capture := filepath.Join(t.TempDir(), "exceptions.pcap")
require.NoError(t, pcapfixture.Write(capture, pcapfixture.ModbusWire, packets))
findings, counters := analyseFixture(t, capture, protocol.ModbusTcp.Name)
require.Len(t, findings, len(packets))
+ assert.Equal(t, 0, counters.Issues())
- // The requests round-trip; the exception responses do not.
- var defects []finding.Finding
+ exceptions := 0
for _, found := range findings {
- if found.Verdict.IsIssue() {
- defects = append(defects, found)
+ assert.Equal(t, finding.VerdictOK, found.Verdict,
+ "packet %d: %s", found.Number, found.Reason)
+ require.Equal(t, found.Original, found.Reserialized, "packet %d", found.Number)
+
+ // The two responses are the point: the error bit and the function code both survive,
+ // where the function code used to be replaced by a zero.
+ if len(found.Original) > 7 && found.Original[7]&0x80 != 0 {
+ exceptions++
+ assert.NotZero(t, found.Original[7]&0x7f, "the vector has to carry a function code")
+ assert.Equal(t, found.Original[7], found.Reserialized[7],
+ "the function code that failed has to survive re-serialization")
}
}
- require.Len(t, defects, 2, "both exception responses have to be reported")
- assert.Equal(t, 2, counters.Issues())
-
- for _, defect := range defects {
- assert.Equal(t, finding.VerdictBytesDiffer, defect.Verdict,
- "plc4x parses these, so the defect is in what it writes back, not in the reading")
- require.Equal(t, 9, len(defect.Original), "the exchange is a nine-byte exception response")
- require.Equal(t, len(defect.Original), len(defect.Reserialized))
-
- // The function byte, and the exact nature of the loss: the error bit survives and the
- // function code does not.
- assert.Equal(t, 7, defect.DiffOffset, "the function code is at offset 7")
- assert.Equal(t, byte(0x80), defect.Reserialized[7],
- "plc4x writes the error bit alone, having dropped which function failed")
- assert.Equal(t, byte(0x80), defect.Original[7]&0x80, "the original has the error bit too")
- assert.NotZero(t, defect.Original[7]&0x7f, "and a function code, which is what is lost")
- }
+ require.Equal(t, 2, exceptions, "the fixture has to still contain both exception responses")
}
diff --git a/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go b/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go
index 5ffcc789..f09b1280 100644
--- a/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go
+++ b/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go
@@ -222,24 +222,25 @@ func SlmpSession() []Packet {
// ModbusExceptionSession is a Modbus exchange whose responses are exceptions.
//
-// These do NOT round-trip, and that is the point of having them: plc4x loses the function code
-// of an exception response. Modbus flags an error by setting the top bit of the original
-// function code -- 0x03 becomes 0x83, 0x08 becomes 0x88 -- and plc4x's ModbusPDUError captures
-// only the flag, not which function failed, so it writes back 0x80 whatever went in. Its own
-// parser reads the function correctly and its own serializer then discards it.
+// This used to be the one fixture that deliberately did not round-trip. plc4x lost the function
+// code of an exception response: Modbus flags an error by setting the top bit of the original
+// function code -- 0x03 becomes 0x83, 0x08 becomes 0x88 -- and ModbusPDUError captured only the
+// flag, so it wrote back 0x80 whatever went in, its own parser having read the function correctly
+// and its own serializer having then discarded it.
//
-// The defect is in the shared protocol definition rather than in the Go binding: the type switch
-// in modbus.mspec matches ModbusPDUError on the error bit alone, and the generated Java has a
-// hard-coded "getFunctionFlag() { return 0; }". So every plc4x language binding does this.
+// The cause was in the code generator rather than in the Go binding, which is why every language
+// binding did it: the type switch in modbus.mspec matches ModbusPDUError on the error bit alone,
+// leaving the generator with no constant for the function code. It now retains a discriminator
+// value that no sub type pins, and these round-trip exactly.
//
// The payloads are real. The 0x88 exchange is from a public capture of 2004 shipped in
// gopacket's own testdata, where thirteen packets show it; the 0x83 one came from a live
-// capture. plc4x's own Modbus test suites contain no exception response at all -- the error flag
-// is false in every case across TCP, RTU and ASCII -- which is why nobody had noticed.
+// capture. plc4x's own Modbus test suites contained no exception response at all -- the error
+// flag was false in every case across TCP, RTU and ASCII -- which is why nobody had noticed.
func ModbusExceptionSession() []Packet {
return []Packet{
{Payload: mustHex("00000000000600010000000a"), Direction: FromClient, Comment: "read holding registers request"},
- {Payload: mustHex("00000000000a880b"[:0] + "0000000000030a880b"), Direction: FromServer, Comment: "exception response to function 0x08 (public capture)"},
+ {Payload: mustHex("0000000000030a880b"), Direction: FromServer, Comment: "exception response to function 0x08 (public capture)"},
{Payload: mustHex("000100000006010300000002"), Direction: FromClient, Comment: "read holding registers request"},
{Payload: mustHex("00010000000301830b"), Direction: FromServer, Comment: "exception response to function 0x03"},
}
(plc4x-extras) 02/02: docs: Correct the count of exception responses in the public capture
Posted by sr...@apache.org.
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-extras.git
commit 64c05ab1fe48c1afa642dda763143722e0365ee4
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Mon Sep 7 16:56:20 2026 +0200
docs: Correct the count of exception responses in the public capture
The comment claimed thirteen packets in gopacket's 2004 capture showed this
defect. Re-measuring either side of the fix shows nine did: four that were
nothing but this bug and are now clean, and five that also carry a trailing
byte the frame length counts but the exception PDU does not, so they still
differ for a reason this fix does not address. The remaining four findings in
that capture were never this bug at all.
---
plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go b/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go
index f09b1280..cf859a1a 100644
--- a/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go
+++ b/plc4go/tools/plc4xpcapanalyzer/internal/pcapfixture/vectors.go
@@ -234,7 +234,7 @@ func SlmpSession() []Packet {
// value that no sub type pins, and these round-trip exactly.
//
// The payloads are real. The 0x88 exchange is from a public capture of 2004 shipped in
-// gopacket's own testdata, where thirteen packets show it; the 0x83 one came from a live
+// gopacket's own testdata, where nine packets show it; the 0x83 one came from a live
// capture. plc4x's own Modbus test suites contained no exception response at all -- the error
// flag was false in every case across TCP, RTU and ASCII -- which is why nobody had noticed.
func ModbusExceptionSession() []Packet {