This is an automated email from the ASF dual-hosted git repository. chrisdutz pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit 80ed6f4a325993b6d3659df06b846352ebe479de Author: Christofer Dutz <[email protected]> AuthorDate: Sun Aug 16 18:02:13 2026 +0200 fix(plc4j/s7): Recognise the S7-300 form of an access-denied response A controller that refuses a request because PUT/GET communication is not enabled answers with an error class in the S7 header. Only 0x81/0x04 and class 0x85 were mapped to ACCESS_DENIED, so the 0x83/0x04 an S7-300 sends fell through to INTERNAL_ERROR and the caller could not tell a refused write from a failed one (GH-599). Map 0x83/0x04 as well. It is scoped to that exact pairing rather than the whole class, because class 0x83 otherwise reads as "no resources available" - only this combination is known to be a refusal. S7AccessDeniedTest covers both header forms and the neighbouring 0x83/0x01, which has to stay generic. It drives a real connection over the scripted transport rather than calling the mapping directly, so the per-tag response code the caller actually sees is what gets asserted. The harness gained two helpers for that: headerErrorResponse and tpduReferenceOfFrame. Note this does not add an error message naming the TIA Portal setting, which is the other half of what GH-599 asked for. A driver-level message about TIA Portal would be wrong for the same error arriving from a controller that is not configured through it. --- .../org/apache/plc4x/java/s7/S7CotpConnection.java | 6 +- .../apache/plc4x/java/s7/S7AccessDeniedTest.java | 91 ++++++++++++++++++++++ .../plc4x/java/s7/S7ScriptedConnectionHarness.java | 14 ++++ 3 files changed, 110 insertions(+), 1 deletion(-) diff --git a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/S7CotpConnection.java b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/S7CotpConnection.java index fafe798fa7..0c8feecc69 100644 --- a/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/S7CotpConnection.java +++ b/plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/S7CotpConnection.java @@ -1330,7 +1330,11 @@ public class S7CotpConnection extends ConnectionBase<S7Configuration> { private static PlcResponseCode mapPlcErrorCode(short errorClass, short errorCode) { // Ported error mapping from s7-light: 129/4 means PUT/GET disabled. - if (errorClass == 129 && errorCode == 4) return PlcResponseCode.ACCESS_DENIED; + if (errorClass == 0x81 && errorCode == 4) return PlcResponseCode.ACCESS_DENIED; + // An S7-300 reports the same refusal as 0x83/0x04 rather than 0x81/0x04 (GH-599). The + // generic reading of class 0x83 is "no resources available", so only this exact pairing + // is treated as a refusal - anything else in that class keeps falling through. + if (errorClass == 0x83 && errorCode == 4) return PlcResponseCode.ACCESS_DENIED; if (errorClass == 0x85) return PlcResponseCode.ACCESS_DENIED; return PlcResponseCode.INTERNAL_ERROR; } diff --git a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/S7AccessDeniedTest.java b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/S7AccessDeniedTest.java new file mode 100644 index 0000000000..5508e01765 --- /dev/null +++ b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/S7AccessDeniedTest.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.plc4x.java.s7; + +import org.apache.plc4x.java.api.messages.PlcWriteResponse; +import org.apache.plc4x.java.api.types.PlcResponseCode; +import org.apache.plc4x.java.s7.S7ScriptedConnectionHarness.ScriptedS7Transport; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * A controller that refuses a write because PUT/GET communication is not enabled answers with an + * error class in the S7 header, and the caller has to be able to tell that apart from a generic + * failure (GH-599). An S7-300 sends 0x83/0x04 where other controllers send 0x81/0x04, so both + * have to come out as ACCESS_DENIED. + */ +class S7AccessDeniedTest { + + @Test + void reportsAccessDeniedForTheS7300ErrorClass() throws Exception { + assertEquals(PlcResponseCode.ACCESS_DENIED, writeAnsweredWith(0x83, 0x04)); + } + + @Test + void reportsAccessDeniedForTheOtherErrorClass() throws Exception { + assertEquals(PlcResponseCode.ACCESS_DENIED, writeAnsweredWith(0x81, 0x04)); + } + + /** + * Class 0x83 on its own is "no resources available", so only the 0x04 pairing is a refusal. + */ + @Test + void keepsOtherCodesInThatClassGeneric() throws Exception { + assertEquals(PlcResponseCode.INTERNAL_ERROR, writeAnsweredWith(0x83, 0x01)); + } + + /** + * Writes a tag, answers the request with the given header error, and returns the code the + * caller ends up seeing for that tag. + */ + private PlcResponseCode writeAnsweredWith(int errorClass, int errorCode) throws Exception { + ScriptedS7Transport transport = new ScriptedS7Transport(); + S7CotpConnection connection = S7ScriptedConnectionHarness.newConnectedConnection(transport); + transport.resetCounters(); + + CompletableFuture<? extends PlcWriteResponse> future = connection.writeRequestBuilder() + .addTagAddress("value", "%DB1.DBW0:INT", 42) + .build() + .execute(); + + byte[] request = awaitFrame(transport); + transport.deliver(S7ScriptedConnectionHarness.headerErrorResponse( + S7ScriptedConnectionHarness.tpduReferenceOfFrame(request), errorClass, errorCode)); + transport.runDataListener(); + + PlcResponseCode code = future.get(5, TimeUnit.SECONDS).getResponseCode("value"); + connection.close(); + return code; + } + + private static byte[] awaitFrame(ScriptedS7Transport transport) throws InterruptedException { + long deadline = System.currentTimeMillis() + 5_000; + while (transport.writeCount() == 0 && System.currentTimeMillis() < deadline) { + Thread.sleep(10); + } + if (transport.writeCount() == 0) { + throw new IllegalStateException("the driver never sent the write request"); + } + return transport.writtenFrames().get(0); + } +} diff --git a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/S7ScriptedConnectionHarness.java b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/S7ScriptedConnectionHarness.java index a8408074ba..eb087515aa 100644 --- a/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/S7ScriptedConnectionHarness.java +++ b/plc4j/drivers/s7/src/test/java/org/apache/plc4x/java/s7/S7ScriptedConnectionHarness.java @@ -140,6 +140,20 @@ public final class S7ScriptedConnectionHarness { return S7Message.staticParse(readBuffer(frame)).getTpduReference(); } + /** Same, for tests that need to answer a specific request they watched go out. */ + public static int tpduReferenceOfFrame(byte[] frame) throws Exception { + return tpduReferenceOf(frame); + } + + /** + * A response carrying an error class/code in the S7 header and nothing else, which is what a + * controller sends when it refuses the whole request rather than individual items. + */ + public static byte[] headerErrorResponse(int tpduReference, int errorClass, int errorCode) throws Exception { + return wireBytes(new S7MessageResponseData(tpduReference, null, null, + (short) errorClass, (short) errorCode)); + } + private static byte[] s7SetupCommunicationResponse(int tpduReference) throws Exception { S7Message message = new S7MessageResponseData(tpduReference, new S7ParameterSetupCommunication(8, 8, 240), null, (short) 0, (short) 0);
