chrisdutz commented on a change in pull request #170:
URL: https://github.com/apache/plc4x/pull/170#discussion_r543274480
##########
File path:
plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java
##########
@@ -69,7 +72,7 @@
*/
public class S7ProtocolLogic extends Plc4xProtocolBase<TPKTPacket> {
- private static final Logger LOGGER =
LoggerFactory.getLogger(S7ProtocolLogic.class);
Review comment:
as it's a static, I would prefer the LOGGER ... or make it non-static,
then use the lowercase version.
##########
File path:
plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java
##########
@@ -91,35 +94,50 @@ public void setDriverContext(DriverContext driverContext) {
@Override
public void onConnect(ConversationContext<TPKTPacket> context) {
+ if (context.isPassive()) {
+ logger.info("S7 Driver running in PASSIVE mode.");
+ s7DriverContext.setPassiveMode(true);
+ // No login required, just confirm that we're connected.
+ context.fireConnected();
+ return;
+ }
+
// Only the TCP transport supports login.
- if(!context.isPassive()) {
Review comment:
Why remove this?
##########
File path:
plc4j/drivers/s7/src/main/java/org/apache/plc4x/java/s7/readwrite/protocol/S7ProtocolLogic.java
##########
@@ -136,32 +154,27 @@ public void onConnect(ConversationContext<TPKTPacket>
context) {
return;
}
- // Prepare a message to request the remote to
identify itself.
- LOGGER.debug("Sending S7 Identification Request");
- TPKTPacket tpktPacket =
createIdentifyRemoteMessage();
- context.sendRequest(tpktPacket)
- .expectResponse(TPKTPacket.class,
REQUEST_TIMEOUT)
- .check(p -> p.getPayload() instanceof
COTPPacketData)
- .unwrap(p -> ((COTPPacketData) p.getPayload()))
- .check(p -> p.getPayload() instanceof
S7MessageUserData)
- .unwrap(p -> ((S7MessageUserData)
p.getPayload()))
- .check(p -> p.getPayload() instanceof
S7PayloadUserData)
- .handle(messageUserData -> {
- LOGGER.debug("Got S7 Identification
Response");
- S7PayloadUserData payloadUserData =
(S7PayloadUserData) messageUserData.getPayload();
-
extractControllerTypeAndFireConnected(context, payloadUserData);
- });
- });
- });
- }
- // This usually when we're running a passive mode river.
- else {
- LOGGER.info("S7 Driver running in PASSIVE mode.");
- s7DriverContext.setPassiveMode(true);
-
- // No login required, just confirm that we're connected.
- context.fireConnected();
- }
Review comment:
We should keep this as we are planning on making the drivers passive
capable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
