You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by Ben Hutcheson <be...@gmail.com> on 2021/02/14 11:47:38 UTC
Changes to mspec
Hi,
While working on the OPC UA native driver I have come across a couple of
cases where it was beneficial to modify mspec. I'd like to bring these up
for discussion.
- OPC UA makes heavy use of pascal strings. This is where the length of the
string precedes the string. Currently when specifying a string in mspec we
have two options. If the string is a predefined length just use the length
in bits. If it is a variable length then we would need to call a manual
function such as what the S7 mspec does.
['IEC61131_STRING' STRING
[manual string 'UTF-8' 'value'
'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.parseS7String",
io, stringLength, _type.encoding)'
'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.serializeS7String",
io, _value, stringLength, _type.encoding)' '_value.length + 2']
]
My proposal would be to allow the length of the string length to be an
inline mspec function.
[type 'PascalString'
[simple int 32 'stringLength']
[optional string 'stringLength * 8' 'UTF-8' 'stringValue'
'stringLength >= 0']
]
This allows us to specify a variable length string using the preceding
string length. We could also expand this to have a variable length field
for any data type, however I'll leave that out of this scope.
- We have a field type in mspec to be able to specify a value is an
enumerated type. However if we then need to make that field a discriminator
for a typeswitch we can't.
discriminatedType 'ExpandedNodeId'
[simple bit 'namespaceURISpecified']
[simple bit 'serverIndexSpecified']
[discriminator NodeIdType 'nodeIdType']
[typeSwitch 'nodeIdType'
['NodeIdType.TwoByte' ExpandedNodeIdTwoByte
[simple TwoByteNodeId 'id']
]
['NodeIdType.FourByte' ExpandedNodeIdFourByte
[simple FourByteNodeId 'id']
]
['NodeIdType.Numeric' ExpandedNodeIdNumeric
[simple NumericNodeId 'id']
]
['NodeIdType.String' ExpandedNodeIdString
[simple StringNodeId 'id']
]
['NodeIdType.Guid' ExpandedNodeIdGuid
[simple GuidNodeId 'id']
]
['NodeIdType.nodeIdTypeByteString' ExpandedNodeIdByteString
[simple ByteStringNodeId 'id']
]
]
[optional PascalString 'namespaceURI' 'namespaceURISpecified']
[optional uint 32 'serverIndex' 'serverIndexSpecified']
]
Shown above is my proposal where the nodeIdType is an enum and is used as
the typeSwitch parameter. This allows us to retain the use of an enum when
we also need to use it as a discriminator.
Looking forward to hearing your opinion.
Ben
AW: Changes to mspec
Posted by Christofer Dutz <ch...@c-ware.de>.
Oh wow,
happy to review the changes then ... especially as I might have an explicit usage for these changes in the pipeline ;)
Chris
-----Ursprüngliche Nachricht-----
Von: Ben Hutcheson <be...@gmail.com>
Gesendet: Sonntag, 14. Februar 2021 14:26
An: dev@plc4x.apache.org
Betreff: Re: Changes to mspec
Hi,
Just a clarification, I have these to changes complete (at least for java) in the opcua branch. I'd need to see what changes I made and port them over to the other three languages.
I might create a branch just for these at some point to make it a bit clearer as to what was done.
Kind Regards,
Ben
On Sun, Feb 14, 2021 at 8:14 AM Christofer Dutz <ch...@c-ware.de>
wrote:
> Hi Ben,
>
> first off all ... a big +1 for allowing Enums in discriminator field.
> Actually there shouldn't be any change to mspec itself required to
> support that. Just in the code-generation we need to tweak a bit. If
> we use an enum value as discriminator (a feature I recently added) it
> only makes sense to use the enum value in a discriminator.
>
> The second proposal took a while till I understood what you are
> referring to. But do I understand correctly:
>
> - Currently, if someone uses a string field, he provides the length in
> bits.
> - Now if we wanted to provide dynamic strings, you propose to make the
> size-argument also accept expressions?
>
> I agree that this would be great ... especially in S7 it would help
> clean up things a LOT. I have also thought about this in the past and
> I think I even tried working on it, but gave up every time. The reason
> for this was, that it requires changing the Antlr4 grammar, the
> parser, the model, all LanguageHelpers as well as all Language templates.
>
> So this "little" change would be quite a big change requiring a LOT of
> work. I wasn't going to invest the time on my own to do this allone.
> But if we want to do it together, I'd be happy to do that on a branch.
>
> Chris
>
> -----Ursprüngliche Nachricht-----
> Von: Ben Hutcheson <be...@gmail.com>
> Gesendet: Sonntag, 14. Februar 2021 12:48
> An: dev@plc4x.apache.org
> Betreff: Changes to mspec
>
> Hi,
>
> While working on the OPC UA native driver I have come across a couple
> of cases where it was beneficial to modify mspec. I'd like to bring
> these up for discussion.
>
> - OPC UA makes heavy use of pascal strings. This is where the length
> of the string precedes the string. Currently when specifying a string
> in mspec we have two options. If the string is a predefined length
> just use the length in bits. If it is a variable length then we would
> need to call a manual function such as what the S7 mspec does.
>
> ['IEC61131_STRING' STRING
> [manual string 'UTF-8' 'value'
> 'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.parseS7Strin
> g",
> io, stringLength, _type.encoding)'
>
> 'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.serializeS7S
> tring", io, _value, stringLength, _type.encoding)' '_value.length +
> 2'] ]
>
> My proposal would be to allow the length of the string length to be an
> inline mspec function.
>
> [type 'PascalString'
> [simple int 32 'stringLength']
> [optional string 'stringLength * 8' 'UTF-8' 'stringValue'
> 'stringLength >= 0']
> ]
>
> This allows us to specify a variable length string using the preceding
> string length. We could also expand this to have a variable length
> field for any data type, however I'll leave that out of this scope.
>
>
> - We have a field type in mspec to be able to specify a value is an
> enumerated type. However if we then need to make that field a
> discriminator for a typeswitch we can't.
>
> discriminatedType 'ExpandedNodeId'
> [simple bit 'namespaceURISpecified']
> [simple bit 'serverIndexSpecified']
> [discriminator NodeIdType 'nodeIdType']
> [typeSwitch 'nodeIdType'
> ['NodeIdType.TwoByte' ExpandedNodeIdTwoByte
> [simple TwoByteNodeId 'id']
> ]
> ['NodeIdType.FourByte' ExpandedNodeIdFourByte
> [simple FourByteNodeId 'id']
> ]
> ['NodeIdType.Numeric' ExpandedNodeIdNumeric
> [simple NumericNodeId 'id']
> ]
> ['NodeIdType.String' ExpandedNodeIdString
> [simple StringNodeId 'id']
> ]
> ['NodeIdType.Guid' ExpandedNodeIdGuid
> [simple GuidNodeId 'id']
> ]
> ['NodeIdType.nodeIdTypeByteString' ExpandedNodeIdByteString
> [simple ByteStringNodeId 'id']
> ]
> ]
> [optional PascalString 'namespaceURI' 'namespaceURISpecified']
> [optional uint 32 'serverIndex' 'serverIndexSpecified'] ]
>
> Shown above is my proposal where the nodeIdType is an enum and is used
> as the typeSwitch parameter. This allows us to retain the use of an
> enum when we also need to use it as a discriminator.
>
> Looking forward to hearing your opinion.
>
> Ben
>
Re: Changes to mspec
Posted by Ben Hutcheson <be...@gmail.com>.
Hi,
Just a clarification, I have these to changes complete (at least for java)
in the opcua branch. I'd need to see what changes I made and port them over
to the other three languages.
I might create a branch just for these at some point to make it a bit
clearer as to what was done.
Kind Regards,
Ben
On Sun, Feb 14, 2021 at 8:14 AM Christofer Dutz <ch...@c-ware.de>
wrote:
> Hi Ben,
>
> first off all ... a big +1 for allowing Enums in discriminator field.
> Actually there shouldn't be any change to mspec itself required to support
> that. Just in the code-generation we need to tweak a bit. If we use an enum
> value as discriminator (a feature I recently added) it only makes sense to
> use the enum value in a discriminator.
>
> The second proposal took a while till I understood what you are referring
> to. But do I understand correctly:
>
> - Currently, if someone uses a string field, he provides the length in
> bits.
> - Now if we wanted to provide dynamic strings, you propose to make the
> size-argument also accept expressions?
>
> I agree that this would be great ... especially in S7 it would help clean
> up things a LOT. I have also thought about this in the past and I think I
> even tried working on it, but gave up every time. The reason for this was,
> that it requires changing the Antlr4 grammar, the parser, the model, all
> LanguageHelpers as well as all Language templates.
>
> So this "little" change would be quite a big change requiring a LOT of
> work. I wasn't going to invest the time on my own to do this allone. But if
> we want to do it together, I'd be happy to do that on a branch.
>
> Chris
>
> -----Ursprüngliche Nachricht-----
> Von: Ben Hutcheson <be...@gmail.com>
> Gesendet: Sonntag, 14. Februar 2021 12:48
> An: dev@plc4x.apache.org
> Betreff: Changes to mspec
>
> Hi,
>
> While working on the OPC UA native driver I have come across a couple of
> cases where it was beneficial to modify mspec. I'd like to bring these up
> for discussion.
>
> - OPC UA makes heavy use of pascal strings. This is where the length of
> the string precedes the string. Currently when specifying a string in mspec
> we have two options. If the string is a predefined length just use the
> length in bits. If it is a variable length then we would need to call a
> manual function such as what the S7 mspec does.
>
> ['IEC61131_STRING' STRING
> [manual string 'UTF-8' 'value'
> 'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.parseS7String",
> io, stringLength, _type.encoding)'
>
> 'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.serializeS7String",
> io, _value, stringLength, _type.encoding)' '_value.length + 2'] ]
>
> My proposal would be to allow the length of the string length to be an
> inline mspec function.
>
> [type 'PascalString'
> [simple int 32 'stringLength']
> [optional string 'stringLength * 8' 'UTF-8' 'stringValue'
> 'stringLength >= 0']
> ]
>
> This allows us to specify a variable length string using the preceding
> string length. We could also expand this to have a variable length field
> for any data type, however I'll leave that out of this scope.
>
>
> - We have a field type in mspec to be able to specify a value is an
> enumerated type. However if we then need to make that field a discriminator
> for a typeswitch we can't.
>
> discriminatedType 'ExpandedNodeId'
> [simple bit 'namespaceURISpecified']
> [simple bit 'serverIndexSpecified']
> [discriminator NodeIdType 'nodeIdType']
> [typeSwitch 'nodeIdType'
> ['NodeIdType.TwoByte' ExpandedNodeIdTwoByte
> [simple TwoByteNodeId 'id']
> ]
> ['NodeIdType.FourByte' ExpandedNodeIdFourByte
> [simple FourByteNodeId 'id']
> ]
> ['NodeIdType.Numeric' ExpandedNodeIdNumeric
> [simple NumericNodeId 'id']
> ]
> ['NodeIdType.String' ExpandedNodeIdString
> [simple StringNodeId 'id']
> ]
> ['NodeIdType.Guid' ExpandedNodeIdGuid
> [simple GuidNodeId 'id']
> ]
> ['NodeIdType.nodeIdTypeByteString' ExpandedNodeIdByteString
> [simple ByteStringNodeId 'id']
> ]
> ]
> [optional PascalString 'namespaceURI' 'namespaceURISpecified']
> [optional uint 32 'serverIndex' 'serverIndexSpecified'] ]
>
> Shown above is my proposal where the nodeIdType is an enum and is used as
> the typeSwitch parameter. This allows us to retain the use of an enum when
> we also need to use it as a discriminator.
>
> Looking forward to hearing your opinion.
>
> Ben
>
AW: Changes to mspec
Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Ben,
first off all ... a big +1 for allowing Enums in discriminator field. Actually there shouldn't be any change to mspec itself required to support that. Just in the code-generation we need to tweak a bit. If we use an enum value as discriminator (a feature I recently added) it only makes sense to use the enum value in a discriminator.
The second proposal took a while till I understood what you are referring to. But do I understand correctly:
- Currently, if someone uses a string field, he provides the length in bits.
- Now if we wanted to provide dynamic strings, you propose to make the size-argument also accept expressions?
I agree that this would be great ... especially in S7 it would help clean up things a LOT. I have also thought about this in the past and I think I even tried working on it, but gave up every time. The reason for this was, that it requires changing the Antlr4 grammar, the parser, the model, all LanguageHelpers as well as all Language templates.
So this "little" change would be quite a big change requiring a LOT of work. I wasn't going to invest the time on my own to do this allone. But if we want to do it together, I'd be happy to do that on a branch.
Chris
-----Ursprüngliche Nachricht-----
Von: Ben Hutcheson <be...@gmail.com>
Gesendet: Sonntag, 14. Februar 2021 12:48
An: dev@plc4x.apache.org
Betreff: Changes to mspec
Hi,
While working on the OPC UA native driver I have come across a couple of cases where it was beneficial to modify mspec. I'd like to bring these up for discussion.
- OPC UA makes heavy use of pascal strings. This is where the length of the string precedes the string. Currently when specifying a string in mspec we have two options. If the string is a predefined length just use the length in bits. If it is a variable length then we would need to call a manual function such as what the S7 mspec does.
['IEC61131_STRING' STRING
[manual string 'UTF-8' 'value'
'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.parseS7String",
io, stringLength, _type.encoding)'
'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.serializeS7String",
io, _value, stringLength, _type.encoding)' '_value.length + 2'] ]
My proposal would be to allow the length of the string length to be an inline mspec function.
[type 'PascalString'
[simple int 32 'stringLength']
[optional string 'stringLength * 8' 'UTF-8' 'stringValue'
'stringLength >= 0']
]
This allows us to specify a variable length string using the preceding string length. We could also expand this to have a variable length field for any data type, however I'll leave that out of this scope.
- We have a field type in mspec to be able to specify a value is an enumerated type. However if we then need to make that field a discriminator for a typeswitch we can't.
discriminatedType 'ExpandedNodeId'
[simple bit 'namespaceURISpecified']
[simple bit 'serverIndexSpecified']
[discriminator NodeIdType 'nodeIdType']
[typeSwitch 'nodeIdType'
['NodeIdType.TwoByte' ExpandedNodeIdTwoByte
[simple TwoByteNodeId 'id']
]
['NodeIdType.FourByte' ExpandedNodeIdFourByte
[simple FourByteNodeId 'id']
]
['NodeIdType.Numeric' ExpandedNodeIdNumeric
[simple NumericNodeId 'id']
]
['NodeIdType.String' ExpandedNodeIdString
[simple StringNodeId 'id']
]
['NodeIdType.Guid' ExpandedNodeIdGuid
[simple GuidNodeId 'id']
]
['NodeIdType.nodeIdTypeByteString' ExpandedNodeIdByteString
[simple ByteStringNodeId 'id']
]
]
[optional PascalString 'namespaceURI' 'namespaceURISpecified']
[optional uint 32 'serverIndex' 'serverIndexSpecified'] ]
Shown above is my proposal where the nodeIdType is an enum and is used as the typeSwitch parameter. This allows us to retain the use of an enum when we also need to use it as a discriminator.
Looking forward to hearing your opinion.
Ben
Re: Changes to mspec
Posted by Łukasz Dywicki <lu...@code-house.org>.
I can second Ben idea of String handling cause each and every driver now
is forced to have a helper just to read/write variable length text. I
believe that almost every driver have it.
One suggestion I have here is to *make* encoding an expression too. This
is due to fact that for example some BACnet devices can be deployed in
different countries which have different character set than UTF-8. This
is really an runtime parameter with UTF-8 as an default.
One thing Chris showed to me earlier when I worked with ADS strings, you
can use implicit field to automatically prepend size of string value
when writing to the wire:
[type 'PascalString'
[implicit int 32 'stringLength' 'COUNT(stringValue)']
[optional string 'stringLength * 8' 'UTF-8' 'stringValue'
'stringLength >= 0']
]
Best,
Łukasz
On 14.02.2021 12:47, Ben Hutcheson wrote:
> Hi,
>
> While working on the OPC UA native driver I have come across a couple of
> cases where it was beneficial to modify mspec. I'd like to bring these up
> for discussion.
>
> - OPC UA makes heavy use of pascal strings. This is where the length of the
> string precedes the string. Currently when specifying a string in mspec we
> have two options. If the string is a predefined length just use the length
> in bits. If it is a variable length then we would need to call a manual
> function such as what the S7 mspec does.
>
> ['IEC61131_STRING' STRING
> [manual string 'UTF-8' 'value'
> 'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.parseS7String",
> io, stringLength, _type.encoding)'
> 'STATIC_CALL("org.apache.plc4x.java.s7.utils.StaticHelper.serializeS7String",
> io, _value, stringLength, _type.encoding)' '_value.length + 2']
> ]
>
> My proposal would be to allow the length of the string length to be an
> inline mspec function.
>
> [type 'PascalString'
> [simple int 32 'stringLength']
> [optional string 'stringLength * 8' 'UTF-8' 'stringValue'
> 'stringLength >= 0']
> ]
>
> This allows us to specify a variable length string using the preceding
> string length. We could also expand this to have a variable length field
> for any data type, however I'll leave that out of this scope.
>
>
> - We have a field type in mspec to be able to specify a value is an
> enumerated type. However if we then need to make that field a discriminator
> for a typeswitch we can't.
>
> discriminatedType 'ExpandedNodeId'
> [simple bit 'namespaceURISpecified']
> [simple bit 'serverIndexSpecified']
> [discriminator NodeIdType 'nodeIdType']
> [typeSwitch 'nodeIdType'
> ['NodeIdType.TwoByte' ExpandedNodeIdTwoByte
> [simple TwoByteNodeId 'id']
> ]
> ['NodeIdType.FourByte' ExpandedNodeIdFourByte
> [simple FourByteNodeId 'id']
> ]
> ['NodeIdType.Numeric' ExpandedNodeIdNumeric
> [simple NumericNodeId 'id']
> ]
> ['NodeIdType.String' ExpandedNodeIdString
> [simple StringNodeId 'id']
> ]
> ['NodeIdType.Guid' ExpandedNodeIdGuid
> [simple GuidNodeId 'id']
> ]
> ['NodeIdType.nodeIdTypeByteString' ExpandedNodeIdByteString
> [simple ByteStringNodeId 'id']
> ]
> ]
> [optional PascalString 'namespaceURI' 'namespaceURISpecified']
> [optional uint 32 'serverIndex' 'serverIndexSpecified']
> ]
>
> Shown above is my proposal where the nodeIdType is an enum and is used as
> the typeSwitch parameter. This allows us to retain the use of an enum when
> we also need to use it as a discriminator.
>
> Looking forward to hearing your opinion.
>
> Ben
>