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
