chrisdutz commented on a change in pull request #241:
URL: https://github.com/apache/plc4x/pull/241#discussion_r616419965



##########
File path: 
plc4j/spi/src/main/java/org/apache/plc4x/java/spi/connection/SingleProtocolStackConfigurer.java
##########
@@ -142,31 +141,86 @@ public 
SingleProtocolStackBuilder(Class<BASE_PACKET_CLASS> basePacketClass, Clas
         }
 
         public SingleProtocolStackBuilder<BASE_PACKET_CLASS> 
withProtocol(Class<? extends Plc4xProtocolBase<BASE_PACKET_CLASS>> protocol) {
-            this.protocol = protocol;
+            return withProtocol(configuredType(protocol));

Review comment:
       I have to admit, that I would prefer if we refactored the existing 
drivers to use the new version instead of doing this.

##########
File path: 
plc4j/spi/src/main/java/org/apache/plc4x/java/spi/EventListenerMessageCodec.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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
+ *
+ *   http://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.spi;
+
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageCodec;
+import org.apache.plc4x.java.api.listener.EventListener;
+import org.apache.plc4x.java.api.listener.MessageExchangeListener;
+import org.apache.plc4x.java.spi.generation.Message;
+
+import java.util.List;
+
+/**
+ * Codec which propagate received or sent messages to connection event 
listeners.
+ */
+public class EventListenerMessageCodec extends MessageToMessageCodec<Message, 
Message> {
+
+    private final List<EventListener> listeners;
+
+    public EventListenerMessageCodec(List<EventListener> listeners) {
+        this.listeners = listeners;
+    }
+
+    @Override
+    protected void encode(ChannelHandlerContext ctx, Message msg, List<Object> 
out) throws Exception {
+        if (msg instanceof Message) {

Review comment:
       Isn't this trivially true ... I mean ... if msg is of type Message, it 
will probably also match this?

##########
File path: 
plc4j/api/src/main/java/org/apache/plc4x/java/api/EventPlcConnection.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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
+ *
+ *   http://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.api;
+
+import org.apache.plc4x.java.api.listener.EventListener;
+
+/**
+ * An extended kind of connection which can broadcast events.
+ *
+ * Particular type of event might vary, it might be a connection state or 
message exchange.
+ */
+public interface EventPlcConnection extends PlcConnection {

Review comment:
       And in addition, we could use the opportunity to add more events ... 
like "Connected" event "ConnectionFailed", "Identified" (fired as soon as a 
connection has identified the remote PLC type (Might be interesting for the 
users), "MetadataChanged", ... 

##########
File path: 
plc4j/api/src/main/java/org/apache/plc4x/java/api/EventPlcConnection.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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
+ *
+ *   http://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.api;
+
+import org.apache.plc4x.java.api.listener.EventListener;
+
+/**
+ * An extended kind of connection which can broadcast events.
+ *
+ * Particular type of event might vary, it might be a connection state or 
message exchange.
+ */
+public interface EventPlcConnection extends PlcConnection {

Review comment:
       I think as this is in the API module, there shouldn't be an 
event-emitting set of connections and non emitting ones. I would rather extend 
the PlcConnection with these two methods and have them implemented in all 
drivers.




-- 
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:
[email protected]


Reply via email to