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 13:12:47 UTC
(plc4x) 01/02: fix(plc4go): Retain a discriminator value that no sub type pins
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 726bd57ece002db08c4db827a5fd184388b7e08b
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Mon Sep 7 14:58:14 2026 +0200
fix(plc4go): Retain a discriminator value that no sub type pins
A sub type's discriminator getter is generated as a constant, because the
typeSwitch case normally pins the value: ModbusPDUReadCoilsRequest knows its
functionFlag is 0x01. A case that leaves a discriminator open, by not
mentioning it or by matching it with a wildcard, left the generator with
nothing to return, so it emitted a zero. The parsed value was then lost twice
over: the getter reported zero, and serializing the parent wrote zero.
Modbus exception responses show it plainly. ModbusPDUError matches on the
error flag alone, so the function code that failed was dropped, and both 0x83
and 0x88 came back as 0x80. BACnet network layer messages had it worse: their
cases match with a wildcard, so even a caller that never re-serialized read
messageType as 0, making a vendor proprietary message (messageType >= 128)
indistinguishable from WhoIsRouterToNetwork.
The parent now keeps the value it parsed and the sub type reads it back from
there. Only the cases that leave a discriminator open assign it, so a sub type
that pins its own discriminator is unaffected and still compares equal whether
it was parsed or constructed.
Also adds parser-serializer coverage for both, where there was none: Modbus
had no exception response case in any of its three suites, and BACnet had no
network layer message case at all. Both fail without this change.
Constructing such a sub type still cannot set the value, as the discriminator
is not a property field; that is left for a follow-up. This covers the Go
generator only, and the Java generator has the same defect.
---
.../templates/go/complex-type-template.go.ftlh | 63 ++++++++++++++++
.../protocols/bacnet/ParserSerializerTestsuite.xml | 88 ++++++++++++++++++++++
.../modbus/tcp/ParserSerializerTestsuite.xml | 35 +++++++++
.../ads/readwrite/model/AdsErrorResponse.go | 4 +-
plc4go/protocols/ads/readwrite/model/AmsPacket.go | 12 +++
plc4go/protocols/bacnetip/readwrite/model/APDU.go | 6 ++
.../bacnetip/readwrite/model/APDUUnknown.go | 2 +-
.../model/BACnetConfirmedServiceRequest.go | 6 ++
.../model/BACnetConfirmedServiceRequestUnknown.go | 2 +-
.../model/BACnetUnconfirmedServiceRequest.go | 6 ++
.../BACnetUnconfirmedServiceRequestUnknown.go | 2 +-
plc4go/protocols/bacnetip/readwrite/model/NLM.go | 8 ++
.../bacnetip/readwrite/model/NLMReserved.go | 2 +-
.../readwrite/model/NLMVendorProprietaryMessage.go | 2 +-
.../knxnetip/readwrite/model/LDataFrame.go | 6 ++
.../knxnetip/readwrite/model/LDataFrameACK.go | 2 +-
.../protocols/modbus/readwrite/model/ModbusPDU.go | 6 ++
.../modbus/readwrite/model/ModbusPDUError.go | 2 +-
plc4go/protocols/umas/readwrite/model/ModbusPDU.go | 6 ++
.../umas/readwrite/model/ModbusPDUError.go | 2 +-
.../protocols/bacnet/ParserSerializerTestsuite.xml | 88 ++++++++++++++++++++++
.../modbus/tcp/ParserSerializerTestsuite.xml | 35 +++++++++
22 files changed, 375 insertions(+), 10 deletions(-)
diff --git a/code-generation/language/go/src/main/resources/templates/go/complex-type-template.go.ftlh b/code-generation/language/go/src/main/resources/templates/go/complex-type-template.go.ftlh
index 5a56b91568..dbd3738c4f 100644
--- a/code-generation/language/go/src/main/resources/templates/go/complex-type-template.go.ftlh
+++ b/code-generation/language/go/src/main/resources/templates/go/complex-type-template.go.ftlh
@@ -65,6 +65,53 @@ import (
<#macro emitImport import>${helper.emitRequiredImport(import)}</#macro>
<#macro emitImportWithAlias alias import>${helper.emitRequiredImport(alias, import)}</#macro>
+<#--
+ The discriminators a parent has to remember after parsing.
+
+ A sub type's discriminator getter is normally a constant, because the typeSwitch case pins
+ the value: ModbusPDUReadCoilsRequest knows its functionFlag is 0x01. A case that leaves a
+ discriminator open, by not mentioning it or by matching it with a wildcard, gives the sub
+ type nothing to return. The parent therefore keeps the value it parsed and the sub type
+ reads it back from there; otherwise the value is silently replaced by a zero, both when
+ asking the sub type for it and when serializing the parent again.
+
+ Only discriminators that are fields of the parent qualify. Parser arguments are supplied by
+ the caller instead of being read from the wire and are never serialized, and an implicit
+ field is recomputed on serialization, so neither needs to be retained.
+-->
+<#function retainedDiscriminators typeDefinition>
+ <#local retained = []>
+ <#if typeDefinition.isDiscriminatedParentTypeDefinition() && typeDefinition.switchField.isPresent()>
+ <#list typeDefinition.switchField.orElseThrow().cases as discriminatedCase>
+ <#list discriminatedCase.getDiscriminatorMap() as discriminatorName, discriminatorTerm>
+ <#if !(discriminatorTerm?? && !helper.isWildcard(discriminatorTerm))
+ && !retained?seq_contains(discriminatorName)
+ && typeDefinition.getNamedFieldByName(discriminatorName).isPresent()
+ && typeDefinition.getNamedFieldByName(discriminatorName).orElseThrow().isDiscriminatorField()>
+ <#local retained = retained + [discriminatorName]>
+ </#if>
+ </#list>
+ </#list>
+ </#if>
+ <#return retained>
+</#function>
+
+<#--
+ The retained discriminators one particular typeSwitch case leaves open. Assigning only these,
+ inside the case that needs them, keeps the parent free of state a sub type never reads: a sub
+ type that pins its discriminator compares equal whether it was parsed or constructed.
+-->
+<#function retainedDiscriminatorsForCase typeDefinition discriminatedCase>
+ <#local retained = []>
+ <#local caseDiscriminators = discriminatedCase.getDiscriminatorMap()>
+ <#list retainedDiscriminators(typeDefinition) as discriminatorName>
+ <#if !(caseDiscriminators[discriminatorName]?? && !helper.isWildcard(caseDiscriminators[discriminatorName]))>
+ <#local retained = retained + [discriminatorName]>
+ </#if>
+ </#list>
+ <#return retained>
+</#function>
+
<@importSectionWithContentBelow><@emitImport import="github.com/apache/plc4x/plc4go/spi/utils" />
// Code generated by code-generation. DO NOT EDIT.
@@ -195,6 +242,12 @@ type _${type.name} struct {
${type.name}Contract
${type.name}Requirements
}
+ <#list retainedDiscriminators(type) as retainedName>
+ <#assign retainedTypeRef=helper.getDiscriminatorTypes(type)[retainedName]>
+ // ${retainedName} as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ ${helper.getGoSafeName(retainedName)} <#if retainedTypeRef.isNonSimpleTypeReference() && !retainedTypeRef.isEnumTypeReference()>I</#if>${helper.getLanguageTypeNameForTypeReference(retainedTypeRef)}
+ </#list>
</#if>
<#-- Property fields are fields that require a property in the pojo -->
<#if type.propertyFields?has_content>
@@ -535,6 +588,9 @@ func (m *_${type.name}) Get${discriminatorName?cap_first}() <#if typeRef.isNonS
<#else>
${helper.toParseExpression(null, typeRef, discriminatorTerm, parserArguments)}
</#if>
+ <#elseif retainedDiscriminators(discriminatedChildType.parentType.orElseThrow())?seq_contains(discriminatorName)>
+ <#assign parentTypeName=discriminatedChildType.parentType.orElseThrow().name>
+ m.${parentTypeName}Contract.(*_${parentTypeName}).${helper.getGoSafeName(discriminatorName)}
<#else>
${helper.getNullValueForTypeReference(typeRef)}
</#if>
@@ -1311,6 +1367,10 @@ func (m *_${type.name}) parse(ctx context.Context, readBuffer utils.ReadBuffer<#
</#if>: // ${case.name}
</...@compress>
+ <#list retainedDiscriminatorsForCase(type, case) as retainedName>
+ // This case does not pin ${retainedName}, so ${case.name} reads the parsed value back from here.
+ m.${helper.getGoSafeName(retainedName)} = ${helper.getGoSafeName(retainedName)}
+ </#list>
if _child, err = new(_${case.name}).parse(ctx, readBuffer, m<#if case.allParserArguments.isPresent()>, <#list case.allParserArguments.orElseThrow() as parserArgument><#if <#--TODO: here DF1ResponseMessage throws a payloadLength not present: meaning that some feature is not rightfully implemented as the result should be a error not a missing pointer-->type.getPropertyFieldFromThisOrParentByName(parserArgument.name).isPresent() && type.getPropertyFieldFromThisOrParentByName(parserArgumen [...]
Go has no implicit numeric conversion, so an argument whose declared width differs from the
value being passed (e.g. a uint8 field bound to a uint16 parser argument) will not compile.
@@ -1811,6 +1871,9 @@ func (m *_${type.name}) deepCopy() *_${type.name} {
</#if>
<#if type.isDiscriminatedParentTypeDefinition()>
nil, // will be set by child
+ <#list retainedDiscriminators(type) as retainedName>
+ m.${helper.getGoSafeName(retainedName)},
+ </#list>
</#if>
<#-- Property fields are fields that require a property in the pojo -->
<#if type.propertyFields?has_content>
diff --git a/plc4go/assets/testing/protocols/bacnet/ParserSerializerTestsuite.xml b/plc4go/assets/testing/protocols/bacnet/ParserSerializerTestsuite.xml
index 3c2caddaa8..50994e45ed 100644
--- a/plc4go/assets/testing/protocols/bacnet/ParserSerializerTestsuite.xml
+++ b/plc4go/assets/testing/protocols/bacnet/ParserSerializerTestsuite.xml
@@ -10037,4 +10037,92 @@
</xml>
</testcase>
+ <testcase>
+ <!-- A network layer message whose messageType falls in the reserved range. The typeSwitch
+ case matches it with a wildcard, so NLMReserved has no constant to return and the
+ parsed messageType has to survive in order to be serialized again. -->
+ <name>Network Layer Message: Reserved Message Type</name>
+ <raw>810a0007018042</raw>
+ <root-type>BVLC</root-type>
+ <xml>
+ <BVLC>
+ <bacnetType dataType="uint" bitLength="8">129</bacnetType>
+ <bvlcFunction dataType="uint" bitLength="8">10</bvlcFunction>
+ <bvlcLength dataType="uint" bitLength="16">7</bvlcLength>
+ <BVLCOriginalUnicastNPDU>
+ <npdu>
+ <NPDU>
+ <protocolVersionNumber dataType="uint" bitLength="8">1</protocolVersionNumber>
+ <control>
+ <NPDUControl>
+ <messageTypeFieldPresent dataType="bit" bitLength="1">true</messageTypeFieldPresent>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <destinationSpecified dataType="bit" bitLength="1">false</destinationSpecified>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <sourceSpecified dataType="bit" bitLength="1">false</sourceSpecified>
+ <expectingReply dataType="bit" bitLength="1">false</expectingReply>
+ <networkPriority>
+ <NPDUNetworkPriority dataType="uint" bitLength="2" stringRepresentation="NORMAL_MESSAGE">0</NPDUNetworkPriority>
+ </networkPriority>
+ </NPDUControl>
+ </control>
+ <nlm>
+ <NLM>
+ <messageType dataType="uint" bitLength="8">66</messageType>
+ <NLMReserved></NLMReserved>
+ </NLM>
+ </nlm>
+ <apdu></apdu>
+ </NPDU>
+ </npdu>
+ </BVLCOriginalUnicastNPDU>
+ </BVLC>
+ </xml>
+ </testcase>
+
+ <testcase>
+ <!-- Same situation for a vendor proprietary message, which is any messageType >= 128. -->
+ <name>Network Layer Message: Vendor Proprietary Message</name>
+ <raw>810a00090180800007</raw>
+ <root-type>BVLC</root-type>
+ <xml>
+ <BVLC>
+ <bacnetType dataType="uint" bitLength="8">129</bacnetType>
+ <bvlcFunction dataType="uint" bitLength="8">10</bvlcFunction>
+ <bvlcLength dataType="uint" bitLength="16">9</bvlcLength>
+ <BVLCOriginalUnicastNPDU>
+ <npdu>
+ <NPDU>
+ <protocolVersionNumber dataType="uint" bitLength="8">1</protocolVersionNumber>
+ <control>
+ <NPDUControl>
+ <messageTypeFieldPresent dataType="bit" bitLength="1">true</messageTypeFieldPresent>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <destinationSpecified dataType="bit" bitLength="1">false</destinationSpecified>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <sourceSpecified dataType="bit" bitLength="1">false</sourceSpecified>
+ <expectingReply dataType="bit" bitLength="1">false</expectingReply>
+ <networkPriority>
+ <NPDUNetworkPriority dataType="uint" bitLength="2" stringRepresentation="NORMAL_MESSAGE">0</NPDUNetworkPriority>
+ </networkPriority>
+ </NPDUControl>
+ </control>
+ <nlm>
+ <NLM>
+ <messageType dataType="uint" bitLength="8">128</messageType>
+ <NLMVendorProprietaryMessage>
+ <vendorId>
+ <BACnetVendorId dataType="uint" bitLength="16" stringRepresentation="SIEMENS_SCHWEIZAG">7</BACnetVendorId>
+ </vendorId>
+ </NLMVendorProprietaryMessage>
+ </NLM>
+ </nlm>
+ <apdu></apdu>
+ </NPDU>
+ </npdu>
+ </BVLCOriginalUnicastNPDU>
+ </BVLC>
+ </xml>
+ </testcase>
+
</test:testsuite>
\ No newline at end of file
diff --git a/plc4go/assets/testing/protocols/modbus/tcp/ParserSerializerTestsuite.xml b/plc4go/assets/testing/protocols/modbus/tcp/ParserSerializerTestsuite.xml
index 76ff08103e..a85a351ead 100644
--- a/plc4go/assets/testing/protocols/modbus/tcp/ParserSerializerTestsuite.xml
+++ b/plc4go/assets/testing/protocols/modbus/tcp/ParserSerializerTestsuite.xml
@@ -255,4 +255,39 @@
</xml>
</testcase>
+ <testcase>
+ <!-- An exception response carries the function code that failed, with the error flag set.
+ The typeSwitch case for ModbusPDUError matches on the error flag alone, so it has no
+ constant for functionFlag and the parsed value has to survive in order to be
+ serialized again. -->
+ <name>Error Response</name>
+ <raw>000100000003ff8302</raw>
+ <root-type>ModbusADU</root-type>
+ <parser-arguments>
+ <driverType>MODBUS_TCP</driverType>
+ <response>true</response>
+ </parser-arguments>
+ <xml>
+ <ModbusADU>
+ <ModbusTcpADU>
+ <transactionIdentifier dataType="uint" bitLength="16">1</transactionIdentifier>
+ <protocolIdentifier dataType="uint" bitLength="16">0</protocolIdentifier>
+ <length dataType="uint" bitLength="16">3</length>
+ <unitIdentifier dataType="uint" bitLength="8">255</unitIdentifier>
+ <pdu>
+ <ModbusPDU>
+ <errorFlag dataType="bit" bitLength="1">true</errorFlag>
+ <functionFlag dataType="uint" bitLength="7">3</functionFlag>
+ <ModbusPDUError>
+ <exceptionCode>
+ <ModbusErrorCode dataType="uint" bitLength="8" stringRepresentation="ILLEGAL_DATA_ADDRESS">2</ModbusErrorCode>
+ </exceptionCode>
+ </ModbusPDUError>
+ </ModbusPDU>
+ </pdu>
+ </ModbusTcpADU>
+ </ModbusADU>
+ </xml>
+ </testcase>
+
</test:testsuite>
diff --git a/plc4go/protocols/ads/readwrite/model/AdsErrorResponse.go b/plc4go/protocols/ads/readwrite/model/AdsErrorResponse.go
index 96497a02a2..e5780e2d0b 100644
--- a/plc4go/protocols/ads/readwrite/model/AdsErrorResponse.go
+++ b/plc4go/protocols/ads/readwrite/model/AdsErrorResponse.go
@@ -157,11 +157,11 @@ func (b *_AdsErrorResponse) CreateAdsErrorResponseBuilder() AdsErrorResponseBuil
///////////////////////
func (m *_AdsErrorResponse) GetCommandId() CommandId {
- return 0
+ return m.AmsPacketContract.(*_AmsPacket).commandId
}
func (m *_AdsErrorResponse) GetResponse() bool {
- return false
+ return m.AmsPacketContract.(*_AmsPacket).response
}
///////////////////////
diff --git a/plc4go/protocols/ads/readwrite/model/AmsPacket.go b/plc4go/protocols/ads/readwrite/model/AmsPacket.go
index a5459a89d1..963c7693ef 100644
--- a/plc4go/protocols/ads/readwrite/model/AmsPacket.go
+++ b/plc4go/protocols/ads/readwrite/model/AmsPacket.go
@@ -104,6 +104,12 @@ type _AmsPacket struct {
AmsPacketContract
AmsPacketRequirements
}
+ // commandId as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ commandId CommandId
+ // response as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ response bool
TargetAmsNetId AmsNetId
TargetAmsPort uint16
SourceAmsNetId AmsNetId
@@ -938,6 +944,10 @@ func (m *_AmsPacket) parse(ctx context.Context, readBuffer utils.ReadBuffer) (__
return nil, errors.Wrap(err, "Error parsing sub-type AdsReadWriteResponse for type-switch of AmsPacket")
}
case true: // AdsErrorResponse
+ // This case does not pin commandId, so AdsErrorResponse reads the parsed value back from here.
+ m.commandId = commandId
+ // This case does not pin response, so AdsErrorResponse reads the parsed value back from here.
+ m.response = response
if _child, err = new(_AdsErrorResponse).parse(ctx, readBuffer, m); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type AdsErrorResponse for type-switch of AmsPacket")
}
@@ -1059,6 +1069,8 @@ func (m *_AmsPacket) deepCopy() *_AmsPacket {
}
_AmsPacketCopy := &_AmsPacket{
nil, // will be set by child
+ m.commandId,
+ m.response,
utils.DeepCopy[AmsNetId](m.TargetAmsNetId),
m.TargetAmsPort,
utils.DeepCopy[AmsNetId](m.SourceAmsNetId),
diff --git a/plc4go/protocols/bacnetip/readwrite/model/APDU.go b/plc4go/protocols/bacnetip/readwrite/model/APDU.go
index 731ab5b34e..7f7cc4ee6c 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/APDU.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/APDU.go
@@ -70,6 +70,9 @@ type _APDU struct {
APDUContract
APDURequirements
}
+ // apduType as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ apduType ApduType
}
var _ APDUContract = (*_APDU)(nil)
@@ -400,6 +403,8 @@ func (m *_APDU) parse(ctx context.Context, readBuffer utils.ReadBuffer, apduLeng
return nil, errors.Wrap(err, "Error parsing sub-type APDUAbort for type-switch of APDU")
}
case 0 == 0: // APDUUnknown
+ // This case does not pin apduType, so APDUUnknown reads the parsed value back from here.
+ m.apduType = apduType
if _child, err = new(_APDUUnknown).parse(ctx, readBuffer, m, uint16(apduLength)); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type APDUUnknown for type-switch of APDU")
}
@@ -453,6 +458,7 @@ func (m *_APDU) deepCopy() *_APDU {
}
_APDUCopy := &_APDU{
nil, // will be set by child
+ m.apduType,
}
return _APDUCopy
}
diff --git a/plc4go/protocols/bacnetip/readwrite/model/APDUUnknown.go b/plc4go/protocols/bacnetip/readwrite/model/APDUUnknown.go
index 8b1e83a9fb..3dcb86907d 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/APDUUnknown.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/APDUUnknown.go
@@ -181,7 +181,7 @@ func (b *_APDUUnknown) CreateAPDUUnknownBuilder() APDUUnknownBuilder {
///////////////////////
func (m *_APDUUnknown) GetApduType() ApduType {
- return 0
+ return m.APDUContract.(*_APDU).apduType
}
///////////////////////
diff --git a/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequest.go b/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequest.go
index 536896067d..d34af808c3 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequest.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequest.go
@@ -75,6 +75,9 @@ type _BACnetConfirmedServiceRequest struct {
BACnetConfirmedServiceRequestContract
BACnetConfirmedServiceRequestRequirements
}
+ // serviceChoice as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ serviceChoice BACnetConfirmedServiceChoice
ServiceRequestLength uint32
}
@@ -820,6 +823,8 @@ func (m *_BACnetConfirmedServiceRequest) parse(ctx context.Context, readBuffer u
return nil, errors.Wrap(err, "Error parsing sub-type BACnetConfirmedServiceRequestReadPropertyConditional for type-switch of BACnetConfirmedServiceRequest")
}
case 0 == 0: // BACnetConfirmedServiceRequestUnknown
+ // This case does not pin serviceChoice, so BACnetConfirmedServiceRequestUnknown reads the parsed value back from here.
+ m.serviceChoice = serviceChoice
if _child, err = new(_BACnetConfirmedServiceRequestUnknown).parse(ctx, readBuffer, m, uint32(serviceRequestPayloadLength), uint32(serviceRequestLength)); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type BACnetConfirmedServiceRequestUnknown for type-switch of BACnetConfirmedServiceRequest")
}
@@ -879,6 +884,7 @@ func (m *_BACnetConfirmedServiceRequest) deepCopy() *_BACnetConfirmedServiceRequ
}
_BACnetConfirmedServiceRequestCopy := &_BACnetConfirmedServiceRequest{
nil, // will be set by child
+ m.serviceChoice,
m.ServiceRequestLength,
}
return _BACnetConfirmedServiceRequestCopy
diff --git a/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequestUnknown.go b/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequestUnknown.go
index 23225df135..e0865950d3 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequestUnknown.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/BACnetConfirmedServiceRequestUnknown.go
@@ -170,7 +170,7 @@ func (b *_BACnetConfirmedServiceRequestUnknown) CreateBACnetConfirmedServiceRequ
///////////////////////
func (m *_BACnetConfirmedServiceRequestUnknown) GetServiceChoice() BACnetConfirmedServiceChoice {
- return 0
+ return m.BACnetConfirmedServiceRequestContract.(*_BACnetConfirmedServiceRequest).serviceChoice
}
///////////////////////
diff --git a/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequest.go b/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequest.go
index 6f59512165..63f62de27a 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequest.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequest.go
@@ -70,6 +70,9 @@ type _BACnetUnconfirmedServiceRequest struct {
BACnetUnconfirmedServiceRequestContract
BACnetUnconfirmedServiceRequestRequirements
}
+ // serviceChoice as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ serviceChoice BACnetUnconfirmedServiceChoice
}
var _ BACnetUnconfirmedServiceRequestContract = (*_BACnetUnconfirmedServiceRequest)(nil)
@@ -464,6 +467,8 @@ func (m *_BACnetUnconfirmedServiceRequest) parse(ctx context.Context, readBuffer
return nil, errors.Wrap(err, "Error parsing sub-type BACnetUnconfirmedServiceRequestUnconfirmedCOVNotificationMultiple for type-switch of BACnetUnconfirmedServiceRequest")
}
case 0 == 0: // BACnetUnconfirmedServiceRequestUnknown
+ // This case does not pin serviceChoice, so BACnetUnconfirmedServiceRequestUnknown reads the parsed value back from here.
+ m.serviceChoice = serviceChoice
if _child, err = new(_BACnetUnconfirmedServiceRequestUnknown).parse(ctx, readBuffer, m, uint16(serviceRequestLength)); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type BACnetUnconfirmedServiceRequestUnknown for type-switch of BACnetUnconfirmedServiceRequest")
}
@@ -517,6 +522,7 @@ func (m *_BACnetUnconfirmedServiceRequest) deepCopy() *_BACnetUnconfirmedService
}
_BACnetUnconfirmedServiceRequestCopy := &_BACnetUnconfirmedServiceRequest{
nil, // will be set by child
+ m.serviceChoice,
}
return _BACnetUnconfirmedServiceRequestCopy
}
diff --git a/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequestUnknown.go b/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequestUnknown.go
index a39e8e24c8..7b72de27ee 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequestUnknown.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/BACnetUnconfirmedServiceRequestUnknown.go
@@ -170,7 +170,7 @@ func (b *_BACnetUnconfirmedServiceRequestUnknown) CreateBACnetUnconfirmedService
///////////////////////
func (m *_BACnetUnconfirmedServiceRequestUnknown) GetServiceChoice() BACnetUnconfirmedServiceChoice {
- return 0
+ return m.BACnetUnconfirmedServiceRequestContract.(*_BACnetUnconfirmedServiceRequest).serviceChoice
}
///////////////////////
diff --git a/plc4go/protocols/bacnetip/readwrite/model/NLM.go b/plc4go/protocols/bacnetip/readwrite/model/NLM.go
index f281afe331..690fab913c 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/NLM.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/NLM.go
@@ -74,6 +74,9 @@ type _NLM struct {
NLMContract
NLMRequirements
}
+ // messageType as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ messageType uint8
}
var _ NLMContract = (*_NLM)(nil)
@@ -633,10 +636,14 @@ func (m *_NLM) parse(ctx context.Context, readBuffer utils.ReadBuffer, apduLengt
return nil, errors.Wrap(err, "Error parsing sub-type NLMNetworkNumberIs for type-switch of NLM")
}
case 0 == 0 && isVendorProprietaryMessage == bool(false): // NLMReserved
+ // This case does not pin messageType, so NLMReserved reads the parsed value back from here.
+ m.messageType = messageType
if _child, err = new(_NLMReserved).parse(ctx, readBuffer, m, uint16(apduLength)); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type NLMReserved for type-switch of NLM")
}
case 0 == 0: // NLMVendorProprietaryMessage
+ // This case does not pin messageType, so NLMVendorProprietaryMessage reads the parsed value back from here.
+ m.messageType = messageType
if _child, err = new(_NLMVendorProprietaryMessage).parse(ctx, readBuffer, m, uint16(apduLength)); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type NLMVendorProprietaryMessage for type-switch of NLM")
}
@@ -696,6 +703,7 @@ func (m *_NLM) deepCopy() *_NLM {
}
_NLMCopy := &_NLM{
nil, // will be set by child
+ m.messageType,
}
return _NLMCopy
}
diff --git a/plc4go/protocols/bacnetip/readwrite/model/NLMReserved.go b/plc4go/protocols/bacnetip/readwrite/model/NLMReserved.go
index c179905fc1..814c670163 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/NLMReserved.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/NLMReserved.go
@@ -170,7 +170,7 @@ func (b *_NLMReserved) CreateNLMReservedBuilder() NLMReservedBuilder {
///////////////////////
func (m *_NLMReserved) GetMessageType() uint8 {
- return 0
+ return m.NLMContract.(*_NLM).messageType
}
///////////////////////
diff --git a/plc4go/protocols/bacnetip/readwrite/model/NLMVendorProprietaryMessage.go b/plc4go/protocols/bacnetip/readwrite/model/NLMVendorProprietaryMessage.go
index 2fc6d5d703..1f36c4e239 100644
--- a/plc4go/protocols/bacnetip/readwrite/model/NLMVendorProprietaryMessage.go
+++ b/plc4go/protocols/bacnetip/readwrite/model/NLMVendorProprietaryMessage.go
@@ -181,7 +181,7 @@ func (b *_NLMVendorProprietaryMessage) CreateNLMVendorProprietaryMessageBuilder(
///////////////////////
func (m *_NLMVendorProprietaryMessage) GetMessageType() uint8 {
- return 0
+ return m.NLMContract.(*_NLM).messageType
}
///////////////////////
diff --git a/plc4go/protocols/knxnetip/readwrite/model/LDataFrame.go b/plc4go/protocols/knxnetip/readwrite/model/LDataFrame.go
index 3a8ed3fed7..4a8de4b830 100644
--- a/plc4go/protocols/knxnetip/readwrite/model/LDataFrame.go
+++ b/plc4go/protocols/knxnetip/readwrite/model/LDataFrame.go
@@ -84,6 +84,9 @@ type _LDataFrame struct {
LDataFrameContract
LDataFrameRequirements
}
+ // polling as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ polling bool
FrameType bool
NotRepeated bool
Priority CEMIPriority
@@ -440,6 +443,8 @@ func (m *_LDataFrame) parse(ctx context.Context, readBuffer utils.ReadBuffer) (_
return nil, errors.Wrap(err, "Error parsing sub-type LPollData for type-switch of LDataFrame")
}
case notAckFrame == bool(false): // LDataFrameACK
+ // This case does not pin polling, so LDataFrameACK reads the parsed value back from here.
+ m.polling = polling
if _child, err = new(_LDataFrameACK).parse(ctx, readBuffer, m); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type LDataFrameACK for type-switch of LDataFrame")
}
@@ -517,6 +522,7 @@ func (m *_LDataFrame) deepCopy() *_LDataFrame {
}
_LDataFrameCopy := &_LDataFrame{
nil, // will be set by child
+ m.polling,
m.FrameType,
m.NotRepeated,
m.Priority,
diff --git a/plc4go/protocols/knxnetip/readwrite/model/LDataFrameACK.go b/plc4go/protocols/knxnetip/readwrite/model/LDataFrameACK.go
index 30fcbbb4c4..10201ff31a 100644
--- a/plc4go/protocols/knxnetip/readwrite/model/LDataFrameACK.go
+++ b/plc4go/protocols/knxnetip/readwrite/model/LDataFrameACK.go
@@ -161,7 +161,7 @@ func (m *_LDataFrameACK) GetNotAckFrame() bool {
}
func (m *_LDataFrameACK) GetPolling() bool {
- return false
+ return m.LDataFrameContract.(*_LDataFrame).polling
}
///////////////////////
diff --git a/plc4go/protocols/modbus/readwrite/model/ModbusPDU.go b/plc4go/protocols/modbus/readwrite/model/ModbusPDU.go
index 0d1b360537..252c1b8e25 100644
--- a/plc4go/protocols/modbus/readwrite/model/ModbusPDU.go
+++ b/plc4go/protocols/modbus/readwrite/model/ModbusPDU.go
@@ -74,6 +74,9 @@ type _ModbusPDU struct {
ModbusPDUContract
ModbusPDURequirements
}
+ // functionFlag as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ functionFlag uint8
}
var _ ModbusPDUContract = (*_ModbusPDU)(nil)
@@ -739,6 +742,8 @@ func (m *_ModbusPDU) parse(ctx context.Context, readBuffer utils.ReadBuffer, res
var _child ModbusPDU
switch {
case errorFlag == bool(true): // ModbusPDUError
+ // This case does not pin functionFlag, so ModbusPDUError reads the parsed value back from here.
+ m.functionFlag = functionFlag
if _child, err = new(_ModbusPDUError).parse(ctx, readBuffer, m, response); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type ModbusPDUError for type-switch of ModbusPDU")
}
@@ -948,6 +953,7 @@ func (m *_ModbusPDU) deepCopy() *_ModbusPDU {
}
_ModbusPDUCopy := &_ModbusPDU{
nil, // will be set by child
+ m.functionFlag,
}
return _ModbusPDUCopy
}
diff --git a/plc4go/protocols/modbus/readwrite/model/ModbusPDUError.go b/plc4go/protocols/modbus/readwrite/model/ModbusPDUError.go
index 1bb6e02438..0a78d4d447 100644
--- a/plc4go/protocols/modbus/readwrite/model/ModbusPDUError.go
+++ b/plc4go/protocols/modbus/readwrite/model/ModbusPDUError.go
@@ -174,7 +174,7 @@ func (m *_ModbusPDUError) GetErrorFlag() bool {
}
func (m *_ModbusPDUError) GetFunctionFlag() uint8 {
- return 0
+ return m.ModbusPDUContract.(*_ModbusPDU).functionFlag
}
func (m *_ModbusPDUError) GetResponse() bool {
diff --git a/plc4go/protocols/umas/readwrite/model/ModbusPDU.go b/plc4go/protocols/umas/readwrite/model/ModbusPDU.go
index e9d56fe5a5..12aed950b6 100644
--- a/plc4go/protocols/umas/readwrite/model/ModbusPDU.go
+++ b/plc4go/protocols/umas/readwrite/model/ModbusPDU.go
@@ -74,6 +74,9 @@ type _ModbusPDU struct {
ModbusPDUContract
ModbusPDURequirements
}
+ // functionFlag as it was read from the wire, kept because at least one sub type does not
+ // pin this discriminator and has no constant of its own to return.
+ functionFlag uint8
}
var _ ModbusPDUContract = (*_ModbusPDU)(nil)
@@ -295,6 +298,8 @@ func (m *_ModbusPDU) parse(ctx context.Context, readBuffer utils.ReadBuffer, uma
var _child ModbusPDU
switch {
case errorFlag == bool(true): // ModbusPDUError
+ // This case does not pin functionFlag, so ModbusPDUError reads the parsed value back from here.
+ m.functionFlag = functionFlag
if _child, err = new(_ModbusPDUError).parse(ctx, readBuffer, m, uint8(umasRequestFunctionKey), uint16(byteLength)); err != nil {
return nil, errors.Wrap(err, "Error parsing sub-type ModbusPDUError for type-switch of ModbusPDU")
}
@@ -356,6 +361,7 @@ func (m *_ModbusPDU) deepCopy() *_ModbusPDU {
}
_ModbusPDUCopy := &_ModbusPDU{
nil, // will be set by child
+ m.functionFlag,
}
return _ModbusPDUCopy
}
diff --git a/plc4go/protocols/umas/readwrite/model/ModbusPDUError.go b/plc4go/protocols/umas/readwrite/model/ModbusPDUError.go
index 2a74f81714..e64cd3a1c6 100644
--- a/plc4go/protocols/umas/readwrite/model/ModbusPDUError.go
+++ b/plc4go/protocols/umas/readwrite/model/ModbusPDUError.go
@@ -174,7 +174,7 @@ func (m *_ModbusPDUError) GetErrorFlag() bool {
}
func (m *_ModbusPDUError) GetFunctionFlag() uint8 {
- return 0
+ return m.ModbusPDUContract.(*_ModbusPDU).functionFlag
}
///////////////////////
diff --git a/protocols/bacnetip/src/test/resources/protocols/bacnet/ParserSerializerTestsuite.xml b/protocols/bacnetip/src/test/resources/protocols/bacnet/ParserSerializerTestsuite.xml
index 3c2caddaa8..50994e45ed 100644
--- a/protocols/bacnetip/src/test/resources/protocols/bacnet/ParserSerializerTestsuite.xml
+++ b/protocols/bacnetip/src/test/resources/protocols/bacnet/ParserSerializerTestsuite.xml
@@ -10037,4 +10037,92 @@
</xml>
</testcase>
+ <testcase>
+ <!-- A network layer message whose messageType falls in the reserved range. The typeSwitch
+ case matches it with a wildcard, so NLMReserved has no constant to return and the
+ parsed messageType has to survive in order to be serialized again. -->
+ <name>Network Layer Message: Reserved Message Type</name>
+ <raw>810a0007018042</raw>
+ <root-type>BVLC</root-type>
+ <xml>
+ <BVLC>
+ <bacnetType dataType="uint" bitLength="8">129</bacnetType>
+ <bvlcFunction dataType="uint" bitLength="8">10</bvlcFunction>
+ <bvlcLength dataType="uint" bitLength="16">7</bvlcLength>
+ <BVLCOriginalUnicastNPDU>
+ <npdu>
+ <NPDU>
+ <protocolVersionNumber dataType="uint" bitLength="8">1</protocolVersionNumber>
+ <control>
+ <NPDUControl>
+ <messageTypeFieldPresent dataType="bit" bitLength="1">true</messageTypeFieldPresent>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <destinationSpecified dataType="bit" bitLength="1">false</destinationSpecified>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <sourceSpecified dataType="bit" bitLength="1">false</sourceSpecified>
+ <expectingReply dataType="bit" bitLength="1">false</expectingReply>
+ <networkPriority>
+ <NPDUNetworkPriority dataType="uint" bitLength="2" stringRepresentation="NORMAL_MESSAGE">0</NPDUNetworkPriority>
+ </networkPriority>
+ </NPDUControl>
+ </control>
+ <nlm>
+ <NLM>
+ <messageType dataType="uint" bitLength="8">66</messageType>
+ <NLMReserved></NLMReserved>
+ </NLM>
+ </nlm>
+ <apdu></apdu>
+ </NPDU>
+ </npdu>
+ </BVLCOriginalUnicastNPDU>
+ </BVLC>
+ </xml>
+ </testcase>
+
+ <testcase>
+ <!-- Same situation for a vendor proprietary message, which is any messageType >= 128. -->
+ <name>Network Layer Message: Vendor Proprietary Message</name>
+ <raw>810a00090180800007</raw>
+ <root-type>BVLC</root-type>
+ <xml>
+ <BVLC>
+ <bacnetType dataType="uint" bitLength="8">129</bacnetType>
+ <bvlcFunction dataType="uint" bitLength="8">10</bvlcFunction>
+ <bvlcLength dataType="uint" bitLength="16">9</bvlcLength>
+ <BVLCOriginalUnicastNPDU>
+ <npdu>
+ <NPDU>
+ <protocolVersionNumber dataType="uint" bitLength="8">1</protocolVersionNumber>
+ <control>
+ <NPDUControl>
+ <messageTypeFieldPresent dataType="bit" bitLength="1">true</messageTypeFieldPresent>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <destinationSpecified dataType="bit" bitLength="1">false</destinationSpecified>
+ <reserved dataType="uint" bitLength="1">0</reserved>
+ <sourceSpecified dataType="bit" bitLength="1">false</sourceSpecified>
+ <expectingReply dataType="bit" bitLength="1">false</expectingReply>
+ <networkPriority>
+ <NPDUNetworkPriority dataType="uint" bitLength="2" stringRepresentation="NORMAL_MESSAGE">0</NPDUNetworkPriority>
+ </networkPriority>
+ </NPDUControl>
+ </control>
+ <nlm>
+ <NLM>
+ <messageType dataType="uint" bitLength="8">128</messageType>
+ <NLMVendorProprietaryMessage>
+ <vendorId>
+ <BACnetVendorId dataType="uint" bitLength="16" stringRepresentation="SIEMENS_SCHWEIZAG">7</BACnetVendorId>
+ </vendorId>
+ </NLMVendorProprietaryMessage>
+ </NLM>
+ </nlm>
+ <apdu></apdu>
+ </NPDU>
+ </npdu>
+ </BVLCOriginalUnicastNPDU>
+ </BVLC>
+ </xml>
+ </testcase>
+
</test:testsuite>
\ No newline at end of file
diff --git a/protocols/modbus/src/test/resources/protocols/modbus/tcp/ParserSerializerTestsuite.xml b/protocols/modbus/src/test/resources/protocols/modbus/tcp/ParserSerializerTestsuite.xml
index 76ff08103e..a85a351ead 100644
--- a/protocols/modbus/src/test/resources/protocols/modbus/tcp/ParserSerializerTestsuite.xml
+++ b/protocols/modbus/src/test/resources/protocols/modbus/tcp/ParserSerializerTestsuite.xml
@@ -255,4 +255,39 @@
</xml>
</testcase>
+ <testcase>
+ <!-- An exception response carries the function code that failed, with the error flag set.
+ The typeSwitch case for ModbusPDUError matches on the error flag alone, so it has no
+ constant for functionFlag and the parsed value has to survive in order to be
+ serialized again. -->
+ <name>Error Response</name>
+ <raw>000100000003ff8302</raw>
+ <root-type>ModbusADU</root-type>
+ <parser-arguments>
+ <driverType>MODBUS_TCP</driverType>
+ <response>true</response>
+ </parser-arguments>
+ <xml>
+ <ModbusADU>
+ <ModbusTcpADU>
+ <transactionIdentifier dataType="uint" bitLength="16">1</transactionIdentifier>
+ <protocolIdentifier dataType="uint" bitLength="16">0</protocolIdentifier>
+ <length dataType="uint" bitLength="16">3</length>
+ <unitIdentifier dataType="uint" bitLength="8">255</unitIdentifier>
+ <pdu>
+ <ModbusPDU>
+ <errorFlag dataType="bit" bitLength="1">true</errorFlag>
+ <functionFlag dataType="uint" bitLength="7">3</functionFlag>
+ <ModbusPDUError>
+ <exceptionCode>
+ <ModbusErrorCode dataType="uint" bitLength="8" stringRepresentation="ILLEGAL_DATA_ADDRESS">2</ModbusErrorCode>
+ </exceptionCode>
+ </ModbusPDUError>
+ </ModbusPDU>
+ </pdu>
+ </ModbusTcpADU>
+ </ModbusADU>
+ </xml>
+ </testcase>
+
</test:testsuite>