Hi, first, thanks Chris for the reporting here and ofc Tim and Sebastian for taking your time (especially to my favorite Pair-programming Partner Sebastian, although it always cost me nerves...:D).
To give a short summarization. We had multiple discussions about some code-depth that is deeply burrowed in the internals of how the code currently works, especially with the old drivers and S7 (which is related, e.g. to the Issues Sebastian Wiendl saw, and we also see sometimes in PROD). Thus, we started a complete rewrite of the driver which has about a tenth of the code of the current one I would guess (99% of the credit for that goes to Chris with mspec, code generation and implementation of protocol behavior). The crucial difference is that all Netty is now out of the protocol logic and handled centrally (and we will add a shitload of checks there) so we can easily tune or fix that layer. In fact we could even take it out and replace with something different as IO Layer. Furthermore, we extended the SPI to make it easier to write Request-Response like communication in combination with the generated objects which makes the code a bit easier to understand and to debug (IMHO). The best example is the current next-gen s7 driver from Chris, which really is a community effort from Chris, Sebastian and myself (the latter two mostly refactoring to the new SPI). It’s the first driver which I can easily follow now, so that’s a good benchmark : ) Furthermore, I'm working on a general optimization layer (made some ascidoc yesterday already, hope to finish it this weekend). I made architectural drawing to explain how it works. Generally, every driver can plugin in to this layer and provide his "merge" or "splitup" operations, or just skip that part. So long from me : ) Julian Am 20.12.19, 08:54 schrieb "Christofer Dutz" <[email protected]>: Hi Alvaro, I think the new driver will not have the same issues the old one had. Julian and Sebastian completely cleaned up that part. I agree that investing your time on the new generation driver might be better invested as we are planning on deleting the old drivers as soon as possible. While general implementation stuff like your modbus findings will be valuable for a new driver too, I am not sure we're going to release another old-style driver set. While we intentionally planned on being able to release bugfix releases of the 0.5.0 drivers, still I think going in on the Next-Generation drivers would be better. The new S7 driver would be the one in the sandbox/test-java-s7-driver ... the connection string is now without rack and slot and with a temporary prefix of "s7ng" So a connection string would be: s7ng://10.10.64.20 (Of course with your Ip ;-) The generated drivers have the code for serializing and parsing generated as well as the model classes, the driver logic is still manually implemented and we'll probably port existing driver code to the new structure. So it's not 100% new code then, but a lot less hand-written code. But till it will be quite a bit of work, for which we are grateful for people willing to join and help. Even if we might run into a little longer delay before the next big release, I think it's definitely worth the effort. After that's done, I think well be able to crank up the release speed dramatically. Thanks for your time and efforts :-) Chris Am 20.12.19, 04:37 schrieb "Álvaro Del Castillo" <[email protected]>: Morning guys, On Thu, 2019-12-19 at 16:23 +0000, Christofer Dutz wrote: > Ok, > > so I just came back home from 3 very productive days. > I think Julian and Sebastian out did themselves with the SPI > refactoring (Driver internal API). Code is a lot cleaner now than it > was before. > Also now things nicely fit together with the generated drivers. > > I managed to finish a first functional generated S7 driver. However > we really need you folks to help out with testing it. > So far it doesn't have all the bells and whistles the old one had. > Right now it doesn't automatically split up large requests and > doesn't automatically handle the queueing of requests as the old S7 > driver did, but we're going to get to that. > > Tim has started to dig into the Modbus protocol and will probably > soon start working on this ... perhaps Alvaro and Tim can work on > this together? > I'm definitely going to be available for help. > Sure, I would like to help on this. I plan to complete the modbus data types supported with the unsigned ones, and also, to work an issue with the writer future. But for example, if the issue with the writer future won't appear in the new refactored driver, it is better to invest time developing and testing the new driver. So Tim, what are your plans for the new modbus driver development? I can join the effort where you find it more valuable. > I'm still not 100% happy about how I'm currently converting the S7 > Data Items into PLC4X Data Items, but I think we'll be able to > finally also fix that soon. > > I know that only a part of the community working on something and > then dropping it on the rest is not the Apache Way. So I would like > to encourage you all to check what we did. > Please inspect the branch "next-gen-core" and give us feedback on it. > Ideally please test the existing drivers on this branch as there was > a lot of refactoring being done. Ok, I will take a look. What drivers are the ones we should put focus specially? > Don't hesitate to ask questions and post suggestions, if you're not > 100% happy with what we propose. > > I guess after a phase of consolidation we'll then merge the branch > back to develop and make the changes official. > Great! > Till then we'll continue fixing the old drivers if bugs are reported, > but expect the 0.6.0 branch to become the one with 100% generated > drivers. Cool, pretty ambitious. I am not aware about the timings between releases. I have analyzed the past ones, and it seems that a new release appears around each 2 months. > It is our goal to get rid of the existing ones as soon as possible. > But there are code from the existing ones that is included in the new drivers, right? Ok, I'll take a look to the work in the "next-gen-core" branch to understand the gory details. Cheers -- Alvaro > Chris > > > Am 18.12.19, 21:20 schrieb "Álvaro Del Castillo" < > [email protected]>: > > Good night! > > On Wed, 2019-12-18 at 16:25 +0000, Christofer Dutz wrote: > > Hi all, > > > > I will try to use a small phase of having to wait on others to > sum up > > what we’ve been up to. > > Currently nothing is really being done on the develop branch as > we > > will put the changes up for discussion before merging. > > The branch to watch is “next-gen-core”. > > https://github.com/apache/plc4x/tree/next-gen-core > > > * Extending the API: > > * We added general support for complex types (Protocols > > however have to be extended to support this) > > > > Nice. As we evolve on our platform, more complex types are needed > and > supported so having this kind of stuff in PLC4x will make easier > the > data processing logic after PLC4x. > > > * Cleaning up the modules > > * We moved and renamed some modules > > * All the plc4j/protocols/driver-bases/ modules are > moved > > away > > * plc4j-protocol-driver-base is now more or less the > new > > SPI module > > * plc4j-protocol-driver-base-xyz modules are now > > transport/xyz (plc4j-transport-xyz) > > Nice. > > > * Refactoring the protocols > > * Introduction of a SPI module > > I was not aware of this SPI module thing: > https://www.baeldung.com/java-spi A good pattern to hide > complexity > when implementig services following the facade approach. I like > it. > > > * Addressed common Netty problems (Buffer Leaks and > alike) > > * Abstracted the protocol layers to no longer have > direct > > dependencies on Netty stuff > > * Implementing new generated protocols > > * Implemented a version of a generated working S7 driver > which > > is able to perform connection as well as reading of values > > * We started working on a port of the Modbus driver > (Which > > previously was implemented by using an external library) > > > > Cool. I will take a look at it to play with this new version. > > > > So far from a user perspective we haven’t done any API changes > (Just > > extended: We added general support for complex data types and > data > > structures) > > > > In general we want to prevent new protocol implementers to run > into > > common Netty problems by hiding all Netty interaction behind a > new > > SPI façade. > > In the background we will be handling all of the difficult > stuff and > > the developer won’t have to think about that sort of things. > > Also are we planning on introducing a completely different way > to > > specify the logic … right now everything is encoding and > decoding > > things and the matching of response to request is some-times > tricky. > > With the proposed changes it will be similar to: > > “sendRequest(xyz).onResponse({do something})”. > > Much easier to understand and track. > > > > > Will continue as things come up … > > > > Thank you Chris for this first report. > > Cheers > > -- Alvaro > > > Chris > > >
