From 13d53e6b00e3f87f424312a58cbed58c1a00bb38 Mon Sep 17 00:00:00 2001 From: cdutz Date: Tue, 22 Feb 2022 10:09:40 +0100 Subject: [PATCH] feat(plc4j/modbus): Cleaned up and added some more code comments explaining why things are done the way they are. --- .../modbus/discovery/ModbusPlcDiscoverer.java | 24 ++++++++++++------- .../resources/protocols/modbus/modbus.mspec | 2 +- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/discovery/ModbusPlcDiscoverer.java b/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/discovery/ModbusPlcDiscoverer.java index e3eba0c11f5..144ee217e66 100644 --- a/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/discovery/ModbusPlcDiscoverer.java +++ b/plc4j/drivers/modbus/src/main/java/org/apache/plc4x/java/modbus/discovery/ModbusPlcDiscoverer.java @@ -112,7 +112,21 @@ private void executeDiscovery(CompletableFuture future, Pl final OutputStream outputStream = socket.getOutputStream(); final InputStream inputStream = new BufferedInputStream(socket.getInputStream()); - // Iterate over all possible unit-identifiers + // As we not only need to provide the IP but also the unit-identifier, we need + // to iterate over all possible values until we find one that works. + // Unfortunately the way devices react to invalid requests differ: + // - My heating system doesn't sends an error response for an invalid uint-identifier + // - The Modbus Server on a S7 simply accepts any unit-identifier + // - Modbus pal only responds to correct unit-identifiers and doesn't send anything + // for invalid ones. + // + // So-far the only way I have found to reliably check if a Modbus device exists, + // was by trying to read a coil or register. Even if Modbus generally supports + // commands for diagnosing connections, it turns out none of these were actually + // supported by any device I came across. The spec is a bit unclear here, but + // it seems as if these are only supported on Serial (Modbus RTU) + // TODO: We should probably not only try to read a coil, but try any of the types and if one works, that's a match. + // Possibly we can fine tune this to speed up things. int transactionIdentifier = 1; for(short unitIdentifier = 1; unitIdentifier <= 247; unitIdentifier++) { ModbusTcpADU packet = new ModbusTcpADU(transactionIdentifier++, unitIdentifier, @@ -177,11 +191,8 @@ private void executeDiscovery(CompletableFuture future, Pl ModbusTcpADU response = ModbusTcpADU.staticParse(readBuffer, true); PlcDiscoveryItem discoveryItem; if (!response.getPdu().getErrorFlag()) { - //final ModbusPDUReadDeviceIdentificationResponse identificationResponse = - // (ModbusPDUReadDeviceIdentificationResponse) response.getPdu(); - // TODO: Unfortunately we need to find a device that's actually correctly responding to this request in order to implement this. discoveryItem = new DefaultPlcDiscoveryItem( - "modbus", "tcp", possibleAddress.getHostAddress(), Collections.singletonMap("unit-identifier", Integer.toString(unitIdentifier)), "known"); + "modbus", "tcp", possibleAddress.getHostAddress(), Collections.singletonMap("unit-identifier", Integer.toString(unitIdentifier)), "unknown"); discoveryItems.add(discoveryItem); // Give a handler the chance to react on the found device. @@ -189,9 +200,6 @@ private void executeDiscovery(CompletableFuture future, Pl handler.handle(discoveryItem); } break; - /*} else { - final ModbusPDUError errorPdu = (ModbusPDUError) response.getPdu(); - logger.info("Got error at address: {} with unit-identifier {} error {}", possibleAddress, unitIdentifier, errorPdu.getExceptionCode().toString());*/ } } catch (ParseException e) { // Ignore. diff --git a/protocols/modbus/src/main/resources/protocols/modbus/modbus.mspec b/protocols/modbus/src/main/resources/protocols/modbus/modbus.mspec index 7617d828e1c..bb799e7bc84 100644 --- a/protocols/modbus/src/main/resources/protocols/modbus/modbus.mspec +++ b/protocols/modbus/src/main/resources/protocols/modbus/modbus.mspec @@ -238,7 +238,7 @@ // Remark: Even if the Modbus spec states that supporting this type of request is mandatory // I have not come across a single device that really supported it. Some devices just reacted - // with an error, however the Modbus support of my S7 1200 just terminates the connection. + // with an error. ['false','0x2B','false' ModbusPDUReadDeviceIdentificationRequest [const uint 8 meiType 0x0E] [simple ModbusDeviceInformationLevel level ]