Skip to content

Commit

Permalink
fix(plc4j): add fireDiscoverEvent (replacing awaitDiscoverComplete) a…
Browse files Browse the repository at this point in the history
…nd moving awaitDiscoverComplete to it's true prupose

Rational: logic is wrong here. The awaits primary purpose it for the Driver test runner to not block. So the action should be done in any case just the waiting should be controlled. As you can see here the event in only fired if the variable is through hence affecting logic. So there should be a new variable called fire discover and the keep the current one to wait for something
  • Loading branch information
sruehl committed Aug 8, 2023
1 parent b6f1628 commit 8e20f3c
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 4 deletions.
Expand Up @@ -165,6 +165,12 @@ public PlcConnection getConnection(String connectionString) throws PlcConnection
// Give drivers the option to customize the channel.
initializePipeline(channelFactory);

// Make the "fire discover event" overridable via system property.
boolean fireDiscoverEvent = fireDiscoverEvent();
if(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT) != null) {
fireDiscoverEvent = Boolean.parseBoolean(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT));
}

// Make the "await setup complete" overridable via system property.
boolean awaitSetupComplete = awaitSetupComplete();
if(System.getProperty(PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE) != null) {
Expand All @@ -189,6 +195,7 @@ public PlcConnection getConnection(String connectionString) throws PlcConnection
getValueHandler(),
configuration,
channelFactory,
fireDiscoverEvent,
awaitSetupComplete,
awaitDisconnectComplete,
awaitDiscoverComplete,
Expand Down
Expand Up @@ -75,7 +75,7 @@ protected String getDefaultTransport() {
}

@Override
protected boolean awaitDiscoverComplete() {
protected boolean fireDiscoverEvent() {
return isEncrypted;
}

Expand Down Expand Up @@ -186,6 +186,12 @@ public PlcConnection getConnection(String connectionString) throws PlcConnection
// Give drivers the option to customize the channel.
initializePipeline(channelFactory);

// Make the "fire discover event" overridable via system property.
boolean fireDiscoverEvent = fireDiscoverEvent();
if(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT) != null) {
fireDiscoverEvent = Boolean.parseBoolean(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT));
}

// Make the "await setup complete" overridable via system property.
boolean awaitSetupComplete = awaitSetupComplete();
if (System.getProperty(PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE) != null) {
Expand Down Expand Up @@ -220,6 +226,7 @@ public PlcConnection getConnection(String connectionString) throws PlcConnection
getValueHandler(),
configuration,
channelFactory,
fireDiscoverEvent,
awaitSetupComplete,
awaitDisconnectComplete,
awaitDiscoverComplete,
Expand Down
Expand Up @@ -124,6 +124,12 @@ public PlcConnection getConnection(String connectionString) throws PlcConnection
}
}

// Make the "fire discover event" overridable via system property.
boolean fireDiscoverEvent = fireDiscoverEvent();
if(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT) != null) {
fireDiscoverEvent = Boolean.parseBoolean(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT));
}

// Make the "await setup complete" overridable via system property.
boolean awaitSetupComplete = awaitSetupComplete();
if (System.getProperty(PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE) != null) {
Expand All @@ -149,6 +155,7 @@ public PlcConnection getConnection(String connectionString) throws PlcConnection
configuration,
channelFactory,
secondaryChannelFactory,
fireDiscoverEvent,
awaitSetupComplete,
awaitDisconnectComplete,
awaitDiscoverComplete,
Expand Down
Expand Up @@ -76,6 +76,7 @@ public S7HDefaultNettyPlcConnection(boolean canRead,
Configuration configuration,
ChannelFactory channelFactory,
ChannelFactory secondaryChannelFactory,
boolean fireDiscoverEvent,
boolean awaitSessionSetupComplete,
boolean awaitSessionDisconnectComplete,
boolean awaitSessionDiscoverComplete,
Expand All @@ -90,6 +91,7 @@ public S7HDefaultNettyPlcConnection(boolean canRead,
valueHandler,
configuration,
channelFactory,
fireDiscoverEvent,
awaitSessionSetupComplete,
awaitSessionDisconnectComplete,
awaitSessionDiscoverComplete,
Expand Down
Expand Up @@ -54,7 +54,7 @@ public class DefaultNettyPlcConnection extends AbstractPlcConnection implements

protected final Configuration configuration;
protected final ChannelFactory channelFactory;

protected final boolean fireDiscoverEvent;
protected final boolean awaitSessionSetupComplete;
protected final boolean awaitSessionDisconnectComplete;
protected final boolean awaitSessionDiscoverComplete;
Expand All @@ -73,6 +73,7 @@ public DefaultNettyPlcConnection(boolean canRead,
PlcValueHandler valueHandler,
Configuration configuration,
ChannelFactory channelFactory,
boolean fireDiscoverEvent,
boolean awaitSessionSetupComplete,
boolean awaitSessionDisconnectComplete,
boolean awaitSessionDiscoverComplete,
Expand All @@ -82,6 +83,7 @@ public DefaultNettyPlcConnection(boolean canRead,
super(canRead, canWrite, canSubscribe, canBrowse, tagHandler, valueHandler, optimizer, authentication);
this.configuration = configuration;
this.channelFactory = channelFactory;
this.fireDiscoverEvent = fireDiscoverEvent;
this.awaitSessionSetupComplete = awaitSessionSetupComplete;
//Used to signal that a disconnect has completed while closing a connection.
this.awaitSessionDisconnectComplete = awaitSessionDisconnectComplete;
Expand Down Expand Up @@ -109,7 +111,7 @@ public void connect() throws PlcConnectionException {
ConfigurationFactory.configure(configuration, channelFactory);

// Have the channel factory create a new channel instance.
if (awaitSessionDiscoverComplete) {
if (fireDiscoverEvent) {
channel = channelFactory.createChannel(getChannelHandler(sessionSetupCompleteFuture, sessionDisconnectCompleteFuture, sessionDiscoveredCompleteFuture));
channel.closeFuture().addListener(future -> {
if (!sessionDiscoveredCompleteFuture.isDone()) {
Expand All @@ -123,7 +125,8 @@ public void connect() throws PlcConnectionException {
}
});
channel.pipeline().fireUserEventTriggered(new DiscoverEvent());

}
if (awaitSessionDiscoverComplete) {
// Wait till the connection is established.
sessionDiscoveredCompleteFuture.get();
}
Expand Down
Expand Up @@ -37,6 +37,7 @@

public abstract class GeneratedDriverBase<BASE_PACKET extends Message> implements PlcDriver {

public static final String PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT = "PLC4X_FORCE_FIRE_DISCOVER_EVENT";
public static final String PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE = "PLC4X_FORCE_AWAIT_SETUP_COMPLETE";
public static final String PROPERTY_PLC4X_FORCE_AWAIT_DISCONNECT_COMPLETE = "PLC4X_FORCE_AWAIT_DISCONNECT_COMPLETE";
public static final String PROPERTY_PLC4X_FORCE_AWAIT_DISCOVER_COMPLETE = "PLC4X_FORCE_AWAIT_DISCOVER_COMPLETE";
Expand All @@ -62,6 +63,10 @@ protected boolean canBrowse() {
return false;
}

protected boolean fireDiscoverEvent() {
return true;
}

protected boolean awaitSetupComplete() {
return true;
}
Expand Down Expand Up @@ -154,6 +159,12 @@ public PlcConnection getConnection(String connectionString, PlcAuthentication au
// Give drivers the option to customize the channel.
initializePipeline(channelFactory);

// Make the "fire discover event" overridable via system property.
boolean fireDiscoverEvent = fireDiscoverEvent();
if(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT) != null) {
fireDiscoverEvent = Boolean.parseBoolean(System.getProperty(PROPERTY_PLC4X_FORCE_FIRE_DISCOVER_EVENT));
}

// Make the "await setup complete" overridable via system property.
boolean awaitSetupComplete = awaitSetupComplete();
if(System.getProperty(PROPERTY_PLC4X_FORCE_AWAIT_SETUP_COMPLETE) != null) {
Expand All @@ -178,6 +189,7 @@ public PlcConnection getConnection(String connectionString, PlcAuthentication au
getValueHandler(),
configuration,
channelFactory,
fireDiscoverEvent,
awaitSetupComplete,
awaitDisconnectComplete,
awaitDiscoverComplete,
Expand Down

0 comments on commit 8e20f3c

Please sign in to comment.