You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by Christofer Dutz <ch...@c-ware.de> on 2019/11/29 10:38:19 UTC
AMS Mspec
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.
But for me it feels like perhaps refactoring the structure could eliminate problems.
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.
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.
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.
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.
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 :-)
Re: AMS Mspec
Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Sebastian,
Just had another idea ... perhaps I misunderstood your intention behind the bitmask ...
if this isn't intended to be a enum-like structure but a number of x Boolean fields?
If this is the case I strongly suggest to implement x bit simple fields.
Chris
Am 29.11.19, 11:38 schrieb "Christofer Dutz" <ch...@c-ware.de>:
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.
But for me it feels like perhaps refactoring the structure could eliminate problems.
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.
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.
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.
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.
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 :-)
Re: AMS Mspec
Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Sebastian,
so I think we have to generally implement a compromise. I think if we want to have 100% flexibility in the mspec, this will increase the complexity of the tool.
I have so-far intentionally done things in a way that it reduces complexity as well as parsing-complexity as spec-writing complexity.
Perhaps it would make sense to add something like an inline type (Sort of like an anonymous inner class in java) that we can keep the parser complexity simple and the mspec languase too.
Chris
Am 29.11.19, 12:48 schrieb "Sebastian Rühl" <se...@googlemail.com.INVALID>:
Hi Chris,
Replies below inline:
> Am 29.11.2019 um 11:38 schrieb Christofer Dutz <ch...@c-ware.de>:
>
> 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
>
>
Re: AMS Mspec
Posted by Sebastian Rühl <se...@googlemail.com.INVALID>.
Hi Chris,
Replies below inline:
> Am 29.11.2019 um 11:38 schrieb Christofer Dutz <ch...@c-ware.de>:
>
> 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
>
>