Hey Chris, I agree with both of your ideas.
But I think if one does really intense use of this "polling" then a dedicated "util" is better (working on one currently). We use "advanced" things like automatic adaption of poll rates if timeouts occur and all such stuff. So we can have a simple and ready to use solution for this subscription thing but I would suggest to add a separate util (I think I can provide our code base as soon as its stable) for a dedicated "scraper" (which uses a dedicated thread pool and some algorithms like possibly request collapsing and such stuff). Perhaps we should start to use the jira more to keep things structured when more people are involved and the number of concurrents topic raises. I filed the tickets PLC4X-49 and PLC4X-50 for these two distinct things. Julian Am 14.09.18, 14:09 schrieb "Christofer Dutz" <[email protected]>: Hi Julian, this warnings for misuse is a thing I had been thinking a lot about recently. After refactoring the S7 driver to use the PLC datatypes and to handle the type mapping correctly, Reading a lot of data using one of the Bit-Stream datatypes: BYTE, WORD, DWORD ... for reading numeric values is highly inefficient. Cause being a bit stream a Byte field item stores it's data as set of Boolean values. While being great when using them to read multiple Boolean values in one batch (What Bit-Stream Values are intended for) we should have a way to log warnings if being used to read only numeric values. So I was thinking of defining a dedicated logger topic to log warnings, if for example only numeric getters are being used for BooleanFields. And regarding the thread number ... as we're building all drivers on top of Netty, we could and should use Netties thread-pools for this sort of functionality. So in the end one and the same thread could handle sending the read-requests for multiple connections. Chris Am 14.09.18, 13:34 schrieb "Julian Feinauer" <[email protected]>: Hi Chris, we can do that and it is absolutely fine for simple use cases. But as such a solution usually has to create threads (or use a given infrastructure for the scheduler and pool or something) this becomes pretty unefficient if someone starts to use a lot of these connections, e.g., to subscribe to multiple dbs in the PLC or something. Furthermore, if you use settings that are not "good" for the PLC there is no one to stop you (we had severe issues with the cycle time for a S7 PLC where we "requested" too much too frequently). But it would be okay to provide a very simple solution with extensive warning for misuse. Julian Am 13.09.18, 09:14 schrieb "Christofer Dutz" <[email protected]>: Hi all, Why not do, what we (ok I) wrote on the Website? - If a protocol/device supports something, we use that directly - If a protocol/device doesn't directly support something, we try to emulate it So in this context I wouldn't care in my application how the subscription works. All I care about is that I subscribe to something and I get data. So if for example this application is based on the subscriber, then when using ADS, this would simply be used and if for example the application uses S7, then the Driver internally uses the "PollingSubscriber" (Or whatever we call it) and just offers the same functionality. I bet there never will be a user that is connected to a device that supports subscriptions natively, but requests to use the polling mechanism. I would strongly suggest not to add another type. Chris Am 13.09.18, 08:59 schrieb "Sebastian Rühl" <[email protected]>: Hi, We could also do something like a PlcPolledReader which extends the PlcSubscriber. With that in mind we would use the same API but avoid confusion as drivers in this case return an empty Optional for PlcSubscriber but an actual Reader for the polled case. But at the moment I would think some util in driver-base would be sufficient too. Sebastian > Am 12.09.2018 um 15:45 schrieb Christofer Dutz <[email protected]>: > > Hi, > > why not implement this in driver-base? Usually a driver could implement the subscription part directly or use the simulated subscriber, but any other combination doesn't really make sense, does it? > > Chris > > Am 12.09.18, 15:31 schrieb "Sebastian Rühl" <[email protected]>: > > Hi, > > I agree with Julian here; we could implement these things in a plc4j-util module or something like that (or pooling in a plc4j-pool). > > Regarding implementing a plcSubscriber based on PlcReader: I think I did that in the camel component when no plcSubscriber is available if I remember correctly. > I would vote against implementing such thing in the core module as PlcSubscriber shall only be used for „real“ notifications and not polled reads. > > Sebastian > >> Am 12.09.2018 um 13:33 schrieb Julian Feinauer <[email protected]>: >> >> Hi Andrey, >> >> we use this heavily (as we want frequent updates from S7 plcs) and have built a complete layer on top of the communication in the last year. >> So I strongly suggest to add this to the project but as another level. >> I think we can also provide some code for this feature. >> >> The main reason why this should be "standalone" from my perspective is that it has to deal with things like ThreadPools or Executors and handling of connection losses and all those things. >> And questions on how to deal with timeouts (if the plc is pretty busy) and things like that. >> Patterns like circuit breaker or request collapsing can be useful if you poll enough messages (or are even necessary). >> So in-depth we spent a lot of time with these things and do not see how we could make a simple and easy to use solution for the core. >> >> Best >> Julian >> >> Am 12.09.18, 13:04 schrieb "Andrey Skorikov" <[email protected]>: >> >> Hello all, >> >> since not all protocols and devices support the subscriber model, I >> believe that it is sensible to emulate the PlcSubscriber based on a >> PlcReader as a fallback. This could be realized by polling the device at >> a fixed rate in the client process. However, I am not sure whether this >> should be part of the core plc4j component, or implemented separately on >> top of it. What do you think? >> >> Regards, >> Andrey Skorikov >> >> >> > > >
