Hi Chris, Replies below inline:
> Am 29.11.2019 um 11:38 schrieb Christofer Dutz <[email protected]>: > > Hi Sebatian, > > I just had a look at your mspec work … it seems in the AMSHeader type, you > ran into a problem which I would like to help you with. > > The simple field: > [simple uint 32 '../data.lengthInBytes' ] > > What should this field contain? I am asking as the mspec code doesn’t have a > parent relationship so this will not work. However you can pass in data via > type arguments. Yep as I mentioned I just goofing around during the migration from the existing codebase to a „potential“ mspec. The packages are well documented by beckhoff: https://infosys.beckhoff.de/index.php?content=../content/1031/TcAdsAmsSpec/HTML/TcAdsAmsSpec_Intro.htm&id= <https://infosys.beckhoff.de/index.php?content=../content/1031/TcAdsAmsSpec/HTML/TcAdsAmsSpec_Intro.htm&id=> So I just keep on exploring how I could achive things with mspec. > > But for me it feels like perhaps refactoring the structure could eliminate > problems. Contrary to that I would stay as close as possible to the original spec liked above > The general problem is that we need the “data” object to produce the output. > So we would need this before parsing the header, which can’t happen as it > hasn’t been parsed yet. This is exactly the reason why I have the `org.apache.plc4x.java.ads.api.util.LengthSupplier` implemented I ADS. As far as I understand this is a marshaling issue only. > One option would be to make this a normal property in the header and to > reference this, if needed in the data. The downside would be that this might > introduce double data. Im not sure if I understood why this is a difference to the child relation. > > I would suggest to merge the AMSPacket and the AMSHeader. > > > [type 'AMSPacket' > [reserved uint 16 '0x0000' ] > [implicit uint 32 'totalLength' 'lengthInBytes - 6'] > [simple AMSNetId 'targetAmsNetId' ] > [simple uint 16 'targetAmsPort' ] > [simple AMSNetId 'sourceAmsNetId' ] > [simple uint 16 'sourceAmsPort' ] > [enum CommandId 'commandId' ] > [enum State 'state' ] > [implicit uint 32 'dataLength' 'data.lengthInBytes'] > [simple uint 32 'errorCode' ] > [simple byte 32 'invokeId' ] > [simple ADSData 'data' ] > ] > > I know why you might want to model the header separately, but sometimes > adjusting the structure to avoid complexities like this is a good thing to do. I would like to explore the impact on mspec because Im not sure right now what this would mean for the parser as stated above. > > The other thing is your proposed “bitmask” … I can’t really see the > difference between an enum and your bitmask. > I think it shouldn’t be a problem to make the enum parser to understand the > bit-pattern you propose (Which I really like, by the way). > If you are worried that enums are in java not extendable, I would really like > to change the enum code-generation to make extendable enum structures instead > of pure java enums. As you wrote in the second mail: Yes its a „real“ bit mask or so to say a collection of boolean flags. For reference here is the current implementation: `org.apache.plc4x.java.ads.api.generic.types.State` So the ‚booleans‘ I have implemented like enums and the bitmask is then a set of these enums. > > What do you think about this? > > But I am happy that you seem to have generally grasped how to build mspecs > which makes me really happy as I think you are one of the first to do that :-) I’m aware that not all what I had before can be projected onto mspec so currently I look for limits of the current approach and how we could pontial model this. > > > PS.: if there are some ’n’s missing in my mail. All glory to the Touch Bar MBP > >
