Sure +1 Sebastian
> Am 17.08.2018 um 10:48 schrieb Christofer Dutz <[email protected]>: > > Oh ... ok ... I was already preparing for a discussion :-) > > Well if we decide to generally go down that path, I would suggest that we > decide on some standard conventions (interfaces in base-driver) so we do > things the same way in all modules. > Would you agree? > > Chris > > > Am 17.08.18, 10:43 schrieb "Sebastian Rühl" > <[email protected]>: > > Just a small note: In ADS it is implemented already this way (Option 3) > > Sebastian > >> Am 17.08.2018 um 10:38 schrieb Christofer Dutz <[email protected]>: >> >> Hi all, >> >> Yesterday I have worked hard on implementing the feature of detecting the >> type of S7 as part of the connection process. >> Fortunately this was a lot simpler than expected as it turned out that >> Siemens documents the structure of the SZL (Engl. SSL) List elements in the >> handbooks of their devices. >> >> But it did bring up something I noticed when last working on the S7 driver. >> >> I implemented the driver keeping in mind the good concepts of software >> design and tried to follow them as good as possible. >> One of these rules is not to couple logic and model. >> >> As a result of this, we have the model classes (Driver internal model) and >> in the corresponding “XyzProtocol” class the logic for serializing and >> deserializing. >> >> This worked well until I started implementing the automatic fragmentation >> and simulation in the S7MessageProcessors. >> >> Now I needed little helpers that were able to estimate the size of the >> serialized form based on the model itself. >> >> The result of this, is that now we have at minimum 3 places, which are >> highly coupled that need maintaining and testing. >> >> This did make things tricky when implementing the SSL List processing and >> did blow up the size and complexity of the S7Driver class. >> >> Now to my proposal: >> >> In general I see 3 options for doing this: >> >> >> 1. (The current way) We have model classes without any logic, A driver >> implementation that contains the serialization/deserialization logic, Some >> helpers tightly linked >> 2. For each model item, we create a separate class which contains the logic >> for serialization/deserialization and size estimation >> 3. We add serialization/deserialization to the model classes >> >> I currently favor option 3. Here my explanation why: >> >> Well option 1 was clean in the beginning, but I am seeing more and more >> problems with maintaining it. >> >> If we went for option 2, we would also have to implement a set of factory >> classes, that help finding the serialization/deserialization class for a >> given input. This can be tricky especially when using inheritance. Also does >> it make things a little more complicated when digging into the code for new >> people. >> >> Option 3 would have everything related to a protocol element in one place. >> We would simply allow calling “toProtocol()”, “fromProtocol()”, >> “getSizeInBytes()”. This does mix model and logic, but in this case I would >> more treat it as additional form of serialization (after all each Object in >> java has “toString()” and the a little hidden serialization that allows >> serializing an object to a byte stream. This would simply add another option >> to that list. And after all the serialization/deserialization doesn’t really >> do much logic. Another downside would be that in order to know what type of >> object is coming, I have to shift a little knowledge to the upstream object. >> So the S7Message has to know which types of Parameter could come, >> instantiate the correct one and then have it fill itself with the data. >> Thinking more about it, I would probably add a second constructor to each >> model type, taking in the Netty ByteBuffer and implement a private >> “fromProtocol” method as a sort of convention. And the only reason the model >> exists, is to provide this one single from of serialization. So the argument >> that “but we could add a different serialization later on” is not valid in >> this case as this would actually mean a new driver. >> >> So what do you think? >> >> Please don’t just fall back to fundamentalist: “You always have to do >> something path X” or “You never should Y”. If you do please provide evidence >> of your concerns so we can discuss them. >> >> In the end I want to make our code: >> >> 1. Easily maintainable >> 2. Easily testable >> 3. Easily understandable >> >> Chris >> >> >> >> >> >> > > >
