Skip to content

Commit 5c316cc

Browse files
sruehlsruehl
committed
test(plc4go/s7): re-enable the driver testsuite cases skipped since the s7h drift
Two fixes were needed to get them green: - the S7 MessageCodec never called FillBuffer, so transports that only surface queued data through fills (like the test transport) starved the receive worker forever - the setup handshake stalled after the COTP connection response. Fill up to the 4-byte TPKT header first, like the modbus codec does. - the testsuite XML still described the pre-s7h SZL identification exchange (structured SzlDataTreeItem response, fixed tpdu ids). It now matches the ported driver: COMPONENT_IDENTIFICATION probe with generator-assigned tpdu ids and a raw-bytes SZL response carrying the MLFB. The XML is updated in its canonical location (protocols/s7) and the build-copied plc4go/assets duplicate is kept in sync.
1 parent 203cfdd commit 5c316cc

4 files changed

Lines changed: 55 additions & 114 deletions

File tree

‎plc4go/assets/testing/protocols/s7/DriverTestsuite.xml‎

Lines changed: 17 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@
221221
<tpduCode dataType="uint" bitLength="8">240</tpduCode>
222222
<COTPPacketData>
223223
<eot dataType="bit" bitLength="1">true</eot>
224-
<tpduRef dataType="uint" bitLength="7">2</tpduRef>
224+
<tpduRef dataType="uint" bitLength="7">10</tpduRef>
225225
</COTPPacketData>
226226
<parameters isList="true">
227227
</parameters>
@@ -230,7 +230,7 @@
230230
<protocolId dataType="uint" bitLength="8">50</protocolId>
231231
<messageType dataType="uint" bitLength="8">7</messageType>
232232
<reserved dataType="uint" bitLength="16">0</reserved>
233-
<tpduReference dataType="uint" bitLength="16">1</tpduReference>
233+
<tpduReference dataType="uint" bitLength="16">10</tpduReference>
234234
<parameterLength dataType="uint" bitLength="16">8</parameterLength>
235235
<payloadLength dataType="uint" bitLength="16">8</payloadLength>
236236
<S7MessageUserData>
@@ -280,12 +280,12 @@
280280
<sublistExtract dataType="uint" bitLength="4">0</sublistExtract>
281281
<sublistList>
282282
<SzlSublist dataType="uint" bitLength="8"
283-
stringRepresentation="MODULE_IDENTIFICATION">17
283+
stringRepresentation="COMPONENT_IDENTIFICATION">28
284284
</SzlSublist>
285285
</sublistList>
286286
</SzlId>
287287
</szlId>
288-
<szlIndex dataType="uint" bitLength="16">0</szlIndex>
288+
<szlIndex dataType="uint" bitLength="16">1</szlIndex>
289289
</S7PayloadUserDataItemCpuFunctionReadSzlRequest>
290290
</S7PayloadUserDataItem>
291291
</items>
@@ -303,7 +303,7 @@
303303
<TPKTPacket>
304304
<protocolId dataType="uint" bitLength="8">3</protocolId>
305305
<reserved dataType="uint" bitLength="8">0</reserved>
306-
<len dataType="uint" bitLength="16">125</len>
306+
<len dataType="uint" bitLength="16">69</len>
307307
<payload>
308308
<COTPPacket>
309309
<headerLength dataType="uint" bitLength="8">2</headerLength>
@@ -319,9 +319,9 @@
319319
<protocolId dataType="uint" bitLength="8">50</protocolId>
320320
<messageType dataType="uint" bitLength="8">7</messageType>
321321
<reserved dataType="uint" bitLength="16">0</reserved>
322-
<tpduReference dataType="uint" bitLength="16">1</tpduReference>
322+
<tpduReference dataType="uint" bitLength="16">10</tpduReference>
323323
<parameterLength dataType="uint" bitLength="16">12</parameterLength>
324-
<payloadLength dataType="uint" bitLength="16">96</payloadLength>
324+
<payloadLength dataType="uint" bitLength="16">40</payloadLength>
325325
<S7MessageUserData>
326326
</S7MessageUserData>
327327
<parameter>
@@ -361,48 +361,11 @@
361361
<DataTransportSize dataType="uint" bitLength="8" stringRepresentation="OCTET_STRING">9
362362
</DataTransportSize>
363363
</transportSize>
364-
<dataLength dataType="uint" bitLength="16">92</dataLength>
364+
<dataLength dataType="uint" bitLength="16">36</dataLength>
365365
<S7PayloadUserDataItemCpuFunctionReadSzlResponse>
366-
<szlId>
367-
<SzlId>
368-
<typeClass>
369-
<SzlModuleTypeClass dataType="uint" bitLength="4" stringRepresentation="CPU">0
370-
</SzlModuleTypeClass>
371-
</typeClass>
372-
<sublistExtract dataType="uint" bitLength="4">0</sublistExtract>
373-
<sublistList>
374-
<SzlSublist dataType="uint" bitLength="8"
375-
stringRepresentation="MODULE_IDENTIFICATION">17
376-
</SzlSublist>
377-
</sublistList>
378-
</SzlId>
379-
</szlId>
380-
<szlIndex dataType="uint" bitLength="16">0</szlIndex>
381-
<szlItemLength dataType="uint" bitLength="16">28</szlItemLength>
382-
<szlItemCount dataType="uint" bitLength="16">3</szlItemCount>
383-
<items isList="true">
384-
<SzlDataTreeItem>
385-
<itemIndex dataType="uint" bitLength="16">1</itemIndex>
386-
<mlfb dataType="byte" bitLength="160">0x36455337203231322d31424433302d3058423020</mlfb>
387-
<moduleTypeId dataType="uint" bitLength="16">8224</moduleTypeId>
388-
<ausbg dataType="uint" bitLength="16">1</ausbg>
389-
<ausbe dataType="uint" bitLength="16">8224</ausbe>
390-
</SzlDataTreeItem>
391-
<SzlDataTreeItem>
392-
<itemIndex dataType="uint" bitLength="16">6</itemIndex>
393-
<mlfb dataType="byte" bitLength="160">0x36455337203231322d31424433302d3058423020</mlfb>
394-
<moduleTypeId dataType="uint" bitLength="16">8224</moduleTypeId>
395-
<ausbg dataType="uint" bitLength="16">1</ausbg>
396-
<ausbe dataType="uint" bitLength="16">8224</ausbe>
397-
</SzlDataTreeItem>
398-
<SzlDataTreeItem>
399-
<itemIndex dataType="uint" bitLength="16">7</itemIndex>
400-
<mlfb dataType="byte" bitLength="160">0x36455337203231322d31424433302d3058423020</mlfb>
401-
<moduleTypeId dataType="uint" bitLength="16">8224</moduleTypeId>
402-
<ausbg dataType="uint" bitLength="16">22018</ausbg>
403-
<ausbe dataType="uint" bitLength="16">2</ausbe>
404-
</SzlDataTreeItem>
405-
</items>
366+
<!-- SZL header (id/index/itemLength/itemCount) + one 28-byte item
367+
carrying the MLFB "6ES7 212-1BD30-0XB0 " -->
368+
<items dataType="byte" bitLength="288">0x001c0001001c0001000136455337203231322d31424433302d3058423020202000012020</items>
406369
</S7PayloadUserDataItemCpuFunctionReadSzlResponse>
407370
</S7PayloadUserDataItem>
408371
</items>
@@ -441,7 +404,7 @@
441404
<tpduCode dataType="uint" bitLength="8">240</tpduCode>
442405
<COTPPacketData>
443406
<eot dataType="bit" bitLength="1">true</eot>
444-
<tpduRef dataType="uint" bitLength="7">10</tpduRef>
407+
<tpduRef dataType="uint" bitLength="7">11</tpduRef>
445408
</COTPPacketData>
446409
<parameters isList="true">
447410
</parameters>
@@ -450,7 +413,7 @@
450413
<protocolId dataType="uint" bitLength="8">50</protocolId>
451414
<messageType dataType="uint" bitLength="8">1</messageType>
452415
<reserved dataType="uint" bitLength="16">0</reserved>
453-
<tpduReference dataType="uint" bitLength="16">10</tpduReference>
416+
<tpduReference dataType="uint" bitLength="16">11</tpduReference>
454417
<parameterLength dataType="uint" bitLength="16">14</parameterLength>
455418
<payloadLength dataType="uint" bitLength="16">0</payloadLength>
456419
<S7MessageRequest>
@@ -517,7 +480,7 @@
517480
<protocolId dataType="uint" bitLength="8">50</protocolId>
518481
<messageType dataType="uint" bitLength="8">3</messageType>
519482
<reserved dataType="uint" bitLength="16">0</reserved>
520-
<tpduReference dataType="uint" bitLength="16">10</tpduReference>
483+
<tpduReference dataType="uint" bitLength="16">11</tpduReference>
521484
<parameterLength dataType="uint" bitLength="16">2</parameterLength>
522485
<payloadLength dataType="uint" bitLength="16">6</payloadLength>
523486
<S7MessageResponseData>
@@ -619,7 +582,7 @@
619582
<tpduCode dataType="uint" bitLength="8">240</tpduCode>
620583
<COTPPacketData>
621584
<eot dataType="bit" bitLength="1">true</eot>
622-
<tpduRef dataType="uint" bitLength="7">10</tpduRef>
585+
<tpduRef dataType="uint" bitLength="7">11</tpduRef>
623586
</COTPPacketData>
624587
<parameters isList="true">
625588
</parameters>
@@ -628,7 +591,7 @@
628591
<protocolId dataType="uint" bitLength="8">50</protocolId>
629592
<messageType dataType="uint" bitLength="8">1</messageType>
630593
<reserved dataType="uint" bitLength="16">0</reserved>
631-
<tpduReference dataType="uint" bitLength="16">10</tpduReference>
594+
<tpduReference dataType="uint" bitLength="16">11</tpduReference>
632595
<parameterLength dataType="uint" bitLength="16">14</parameterLength>
633596
<payloadLength dataType="uint" bitLength="16">0</payloadLength>
634597
<S7MessageRequest>
@@ -695,7 +658,7 @@
695658
<protocolId dataType="uint" bitLength="8">50</protocolId>
696659
<messageType dataType="uint" bitLength="8">2</messageType>
697660
<reserved dataType="uint" bitLength="16">0</reserved>
698-
<tpduReference dataType="uint" bitLength="16">10</tpduReference>
661+
<tpduReference dataType="uint" bitLength="16">11</tpduReference>
699662
<parameterLength dataType="uint" bitLength="16">0</parameterLength>
700663
<payloadLength dataType="uint" bitLength="16">0</payloadLength>
701664
<S7MessageResponse>

‎plc4go/internal/s7/MessageCodec.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,27 @@ func (m *MessageCodec) Send(ctx context.Context, interactionInfo string, message
128128
}
129129

130130
func (m *MessageCodec) Receive(ctx context.Context) (spi.Message, error) {
131-
// We need at least 6 bytes in order to know how big the packet is in total
131+
transportInstance := m.GetTransportInstance()
132+
if !transportInstance.IsConnected() {
133+
return nil, errors.New("Transport instance not connected")
134+
}
135+
// Pull data from the transport until at least the 4-byte TPKT header is buffered.
136+
// Some transports (e.g. the test transport) only surface queued data through fills,
137+
// so checking the buffer without filling first would starve the receive worker.
138+
if err := transportInstance.FillBuffer(ctx, func(pos uint, currentByte byte, reader transports.ExtendedReader) bool {
139+
numBytesAvailable, err := transportInstance.GetNumBytesAvailableInBuffer()
140+
if err != nil {
141+
return false
142+
}
143+
return numBytesAvailable < 4
144+
}); err != nil {
145+
if transportError, ok := transports.AsTransportError(err); ok && transportError.Kind() == transports.TransportErrorFatal {
146+
return nil, err
147+
}
148+
// Fall through on non-fatal errors, we might have enough data buffered already.
149+
m.log.Trace().Err(err).Msg("Error filling buffer, continuing with what's available")
150+
}
151+
// We need at least 4 bytes in order to know how big the packet is in total
132152
if num, err := m.GetTransportInstance().GetNumBytesAvailableInBuffer(); (err == nil) && (num >= 4) {
133153
m.log.Debug().Uint32("num", num).Msg("we got %d readable bytes")
134154
data, err := m.GetTransportInstance().PeekReadableBytes(ctx, 4)

‎plc4go/tests/drivers/tests/s7_driver_test.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,6 @@ func TestS7Driver(t *testing.T) {
4141
s7IO.S7XmlParserHelper{},
4242
append(optionsForTesting,
4343
testutils.WithRootTypeParser(parser),
44-
testutils.WithSkippedTestCases(
45-
// TODO: ignored due to carcia changes
46-
"Single element read request",
47-
"Single element read request with disabled PUT/GET",
48-
),
4944
)...,
5045
)
5146
}

0 commit comments

Comments
 (0)