You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by Sebastian R��hl <sr...@apache.org> on 2021/09/24 21:52:32 UTC

Changes on mspec: parameterized type refs, assert, try, const

Hi together,

I have some exciting changes in the pipeline regarding the mspec:

1. parameters on type refs
    with that change it is now possible to target a discriminated child in advance.
2. assert keyword
    with that change it is possible to throw a ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop)
3. try keyword to prefix fields:
    with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
4. const is now extended to type reference
   this change allows enums to be used as const values.

All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.

Here is a example working with bacnet:
        ['0x07' BACnetUnconfirmedServiceRequestWhoHas
            [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
            [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
            [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
            [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
        ]

The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.

Here is the snippet from the parent type:

[discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
    [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
    [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
    [simple        uint 3           'lengthValueType'                                                                         ]
    .....
    [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
    [typeSwitch 'dataType'
        ....
        ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
            // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
            [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
            [simple     string 'actualLengthInBit' 'ASCII'   'theString']
        ]

Would love to hear some opinions! If there are no objections I would push this change to develop soon.

- Sebastian

PatchContent:
Index: code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
<+>UTF-8
===================================================================
diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
--- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
+++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
@@ -34,7 +34,7 @@
  ;
 
 fieldDefinition
- : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? RBRACKET
+ : LBRACKET tryParse? field (LBRACKET params=multipleExpressions RBRACKET)? RBRACKET
  ;
 
 dataIoDefinition
@@ -49,6 +49,7 @@
  | discriminatorField
  | enumField
  | implicitField
+ | assertField
  | manualArrayField
  | manualField
  | optionalField
@@ -73,7 +74,7 @@
  ;
 
 constField
- : 'const' type=dataType name=idExpression expected=expression
+ : 'const' type=typeReference name=idExpression expected=expression
  ;
 
 discriminatorField
@@ -88,6 +89,10 @@
  : 'implicit' type=dataType name=idExpression serializeExpression=expression
  ;
 
+assertField
+ : 'assert' type=typeReference name=idExpression condition=expression
+ ;
+
 manualArrayField
  : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
  ;
@@ -129,7 +134,7 @@
  ;
 
 typeReference
- : complexTypeReference=IDENTIFIER_LITERAL
+ : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
  | simpleTypeReference=dataType
  ;
 
@@ -150,6 +155,10 @@
  | base='dateTime'
  ;
 
+tryParse
+ : 'try'
+ ;
+
 argument
  : type=typeReference name=idExpression
  ;


Re: AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
An alternative might be to have one static function per field type and used simple type and one for each field type for complex types and not using lambdas.

Holen Sie sich Outlook für Android<https://aka.ms/AAb9ysg>
________________________________
From: Christofer Dutz <ch...@c-ware.de>
Sent: Friday, October 8, 2021 8:25:11 AM
To: dev@plc4x.apache.org <de...@plc4x.apache.org>
Subject: Re: AW: Changes on mspec: parameterized type refs, assert, try, const

Good point...

As I said, I'll create a new template and then we can experiment with it.

Chris

Holen Sie sich Outlook für Android<https://aka.ms/AAb9ysg>
________________________________
From: Łukasz Dywicki <lu...@code-house.org>
Sent: Thursday, October 7, 2021 11:23:55 PM
To: dev@plc4x.apache.org <de...@plc4x.apache.org>
Subject: Re: AW: Changes on mspec: parameterized type refs, assert, try, const

Hey Chris,

Given the code generation is a serious piece of work and code to go with
I wanted to highlight just one thing. Lambda expressions might result in
messy stack traces.
Since frame parsing in some ways is recursive process it migth result in
multiple lambdas on stack trace. As long as we have conditionality only
with primitive fields it will work. Yet it will be much harder to debug
generated code if we will chain complex types (de)serialization.

I did update a while ago java ConversationContext to not rely on lambdas
so stack traces are always starting with org.apache.plc4x package. ;-)
All this should not hold you from doing your thing. I believe I will be
able to de-lambdize java templates if we will find it necessary later on.

Best,
Łukasz


On 07.10.2021 19:08, Christofer Dutz wrote:
> Hi,
>
> now that I'm sort of allmost finished with adjusting the mspec parsers and model types, I'm planning up update the code generation templates.
>
> I know that if I do it the normal way, that this would excessivley blow up the complexity and size of the template files as well as that of the produced code.
>
> As this code has become more and more complex over the last year anyway, I would like to radically clean up the code here.
>
> For that I had an idea, that I formalized a bit in the Confluence page: https://cwiki.apache.org/confluence/display/PLC4X/MSPEC+improvements
>
> TL/DR: I would create static methods that handle the reading/writing of every field type and simply use these instead of generating code for every field.
>
> Feel free to comment,
>
> Chris
>
>
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Montag, 4. Oktober 2021 08:55
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
>
> Hi all,
>
> so over the weekend Sebastian and I have been working hard on an updated mspec version.
> In the "feature/mspec-ng" is where all of this is happening.
>
> So far the changes:
> - We introduced a concept of "attributes" which can be added to fields and types (These are name=expression) tuples. These are added before any argument-block. These can currently be used for setting the encoding for string types (possibly even for other types in case we need them). And for the upcomming "endianess" attribute which should allow switching endianess.
> - We introduced a new simple type "vstring" for variable length strings. The expression however is optional.
> - The previous "string" now only accepts fixed number of bits.
> - The encoding for String fields (string and vstring) defaults to "UTF-8" but you can override it with an attribute with: encoding='"UTF-16"'
> - The previous encoding string has been removed from string types)
> - We added a new type of "field": "batchSet" which takes a list of attributes. These are automatically added to all fields it contains. (Currently this is only available in "type", "complexTyte" elements or inside typeSwitch case elements.
>
> I think this was about it for now ... still need to implement the attribute functionality ... it's currently parsed, but not processed yet.
>
> Chris
>
>
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Dienstag, 28. September 2021 14:57
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
>
> Another thought I had last night ... while sort of not being able to sleep (at least it hat something good)
>
> So I think a "try" field should always be an optional field. As it might be set or not, just with the difference that the condition is different.
>
> This got me thinking even further ... how about making the condition of an "optional" field optional? ... So in general what Sebastian was proposing with a:
>
> [try simple SomeType 'coolField']
>
> Would actually be:
>
> [optional SomeType 'coolField']
>
> This would simplify things even more...
>
> Chris
>
>
>
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Sonntag, 26. September 2021 16:03
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
>
> Bringing the thought of the other discussion here too ...
>
> How about adding name-value paris to the type declarations as well as to the fields? Then the "try" flag could be replaced with a name-value pair or even a name=expression pair ... sort of like "parse-behaviour=try" or something similar. Then we wouldn't be starting to add all sorts of "flags".
>
> Chris
>
> -----Ursprüngliche Nachricht-----
> Von: Sebastian Rühl <sr...@apache.org>
> Gesendet: Sonntag, 26. September 2021 15:44
> An: dev@plc4x.apache.org
> Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const
>
> Hey Lukaz,
>
> the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
> These blocks could be indeed come in handy as an option to define things...
>
> - Sebastian
>
> On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote:
>> Hey Sebastian,
>> Not that far ago we had a similar discussion. Yet then it was mainly
>> about optionals. See "[DISCUSS] Change the way we use "optional".
>>
>> Back then we did not do any changes to mspec, just ranted about
>> possible approach to make better use of optional fields.
>> I remember that current mspec/code can't for example handle ie. a
>> bacnet whois without all fields. Looking at your example I guess you
>> try to solve that puzzle.
>>
>> I don't have any strong feelings here, but why not using optional
>> field with additional "reset" flag?
>> To me what rings a bell in your mail is an assumption that try is
>> always followed by optional field. This tells me that 'try' should
>> rather be a block than a field. By this way we will be able to pair
>> these visually as well as handle situation when there is one try
>> statement but multiple optional fields.
>>
>> Best,
>> Łukasz
>>
>>
>> On 24.09.2021 23:52, Sebastian Rühl wrote:
>>> Hi together,
>>>
>>> I have some exciting changes in the pipeline regarding the mspec:
>>>
>>> 1. parameters on type refs
>>>       with that change it is now possible to target a discriminated child in advance.
>>> 2. assert keyword
>>>       with that change it is possible to throw a
>>> ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
>>>       with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
>>> 4. const is now extended to type reference
>>>      this change allows enums to be used as const values.
>>>
>>> All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
>>>
>>> Here is a example working with bacnet:
>>>           ['0x07' BACnetUnconfirmedServiceRequestWhoHas
>>>               [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
>>>               [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
>>>               [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
>>>               [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
>>>           ]
>>>
>>> The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
>>>
>>> Here is the snippet from the parent type:
>>>
>>> [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
>>>       [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
>>>       [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
>>>       [simple        uint 3           'lengthValueType'                                                                         ]
>>>       .....
>>>       [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
>>>       [typeSwitch 'dataType'
>>>           ....
>>>           ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
>>>               // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
>>>               [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
>>>               [simple     string 'actualLengthInBit' 'ASCII'   'theString']
>>>           ]
>>>
>>> Would love to hear some opinions! If there are no objections I would push this change to develop soon.
>>>
>>> - Sebastian
>>>
>>> PatchContent:
>>> Index:
>>> code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x
>>> /plugins/codegenerator/language/mspec/MSpec.g4
>>> <+>UTF-8
>>> ===================================================================
>>> diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
>>> --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4    (revision ef35531d5a872f29dccddb3a11a135b166958185)
>>> +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4    (date 1632518519426)
>>> @@ -34,7 +34,7 @@
>>>     ;
>>>
>>>    fieldDefinition
>>> - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)?
>>> RBRACKET
>>> + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions
>>> + RBRACKET)? RBRACKET
>>>     ;
>>>
>>>    dataIoDefinition
>>> @@ -49,6 +49,7 @@
>>>     | discriminatorField
>>>     | enumField
>>>     | implicitField
>>> + | assertField
>>>     | manualArrayField
>>>     | manualField
>>>     | optionalField
>>> @@ -73,7 +74,7 @@
>>>     ;
>>>
>>>    constField
>>> - : 'const' type=dataType name=idExpression expected=expression
>>> + : 'const' type=typeReference name=idExpression expected=expression
>>>     ;
>>>
>>>    discriminatorField
>>> @@ -88,6 +89,10 @@
>>>     : 'implicit' type=dataType name=idExpression serializeExpression=expression
>>>     ;
>>>
>>> +assertField
>>> + : 'assert' type=typeReference name=idExpression
>>> +condition=expression  ;
>>> +
>>>    manualArrayField
>>>     : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
>>>     ;
>>> @@ -129,7 +134,7 @@
>>>     ;
>>>
>>>    typeReference
>>> - : complexTypeReference=IDENTIFIER_LITERAL
>>> + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
>>>     | simpleTypeReference=dataType
>>>     ;
>>>
>>> @@ -150,6 +155,10 @@
>>>     | base='dateTime'
>>>     ;
>>>
>>> +tryParse
>>> + : 'try'
>>> + ;
>>> +
>>>    argument
>>>     : type=typeReference name=idExpression
>>>     ;
>>>
>>

Re: AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
Good point...

As I said, I'll create a new template and then we can experiment with it.

Chris

Holen Sie sich Outlook für Android<https://aka.ms/AAb9ysg>
________________________________
From: Łukasz Dywicki <lu...@code-house.org>
Sent: Thursday, October 7, 2021 11:23:55 PM
To: dev@plc4x.apache.org <de...@plc4x.apache.org>
Subject: Re: AW: Changes on mspec: parameterized type refs, assert, try, const

Hey Chris,

Given the code generation is a serious piece of work and code to go with
I wanted to highlight just one thing. Lambda expressions might result in
messy stack traces.
Since frame parsing in some ways is recursive process it migth result in
multiple lambdas on stack trace. As long as we have conditionality only
with primitive fields it will work. Yet it will be much harder to debug
generated code if we will chain complex types (de)serialization.

I did update a while ago java ConversationContext to not rely on lambdas
so stack traces are always starting with org.apache.plc4x package. ;-)
All this should not hold you from doing your thing. I believe I will be
able to de-lambdize java templates if we will find it necessary later on.

Best,
Łukasz


On 07.10.2021 19:08, Christofer Dutz wrote:
> Hi,
>
> now that I'm sort of allmost finished with adjusting the mspec parsers and model types, I'm planning up update the code generation templates.
>
> I know that if I do it the normal way, that this would excessivley blow up the complexity and size of the template files as well as that of the produced code.
>
> As this code has become more and more complex over the last year anyway, I would like to radically clean up the code here.
>
> For that I had an idea, that I formalized a bit in the Confluence page: https://cwiki.apache.org/confluence/display/PLC4X/MSPEC+improvements
>
> TL/DR: I would create static methods that handle the reading/writing of every field type and simply use these instead of generating code for every field.
>
> Feel free to comment,
>
> Chris
>
>
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Montag, 4. Oktober 2021 08:55
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
>
> Hi all,
>
> so over the weekend Sebastian and I have been working hard on an updated mspec version.
> In the "feature/mspec-ng" is where all of this is happening.
>
> So far the changes:
> - We introduced a concept of "attributes" which can be added to fields and types (These are name=expression) tuples. These are added before any argument-block. These can currently be used for setting the encoding for string types (possibly even for other types in case we need them). And for the upcomming "endianess" attribute which should allow switching endianess.
> - We introduced a new simple type "vstring" for variable length strings. The expression however is optional.
> - The previous "string" now only accepts fixed number of bits.
> - The encoding for String fields (string and vstring) defaults to "UTF-8" but you can override it with an attribute with: encoding='"UTF-16"'
> - The previous encoding string has been removed from string types)
> - We added a new type of "field": "batchSet" which takes a list of attributes. These are automatically added to all fields it contains. (Currently this is only available in "type", "complexTyte" elements or inside typeSwitch case elements.
>
> I think this was about it for now ... still need to implement the attribute functionality ... it's currently parsed, but not processed yet.
>
> Chris
>
>
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Dienstag, 28. September 2021 14:57
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
>
> Another thought I had last night ... while sort of not being able to sleep (at least it hat something good)
>
> So I think a "try" field should always be an optional field. As it might be set or not, just with the difference that the condition is different.
>
> This got me thinking even further ... how about making the condition of an "optional" field optional? ... So in general what Sebastian was proposing with a:
>
> [try simple SomeType 'coolField']
>
> Would actually be:
>
> [optional SomeType 'coolField']
>
> This would simplify things even more...
>
> Chris
>
>
>
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Sonntag, 26. September 2021 16:03
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
>
> Bringing the thought of the other discussion here too ...
>
> How about adding name-value paris to the type declarations as well as to the fields? Then the "try" flag could be replaced with a name-value pair or even a name=expression pair ... sort of like "parse-behaviour=try" or something similar. Then we wouldn't be starting to add all sorts of "flags".
>
> Chris
>
> -----Ursprüngliche Nachricht-----
> Von: Sebastian Rühl <sr...@apache.org>
> Gesendet: Sonntag, 26. September 2021 15:44
> An: dev@plc4x.apache.org
> Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const
>
> Hey Lukaz,
>
> the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
> These blocks could be indeed come in handy as an option to define things...
>
> - Sebastian
>
> On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote:
>> Hey Sebastian,
>> Not that far ago we had a similar discussion. Yet then it was mainly
>> about optionals. See "[DISCUSS] Change the way we use "optional".
>>
>> Back then we did not do any changes to mspec, just ranted about
>> possible approach to make better use of optional fields.
>> I remember that current mspec/code can't for example handle ie. a
>> bacnet whois without all fields. Looking at your example I guess you
>> try to solve that puzzle.
>>
>> I don't have any strong feelings here, but why not using optional
>> field with additional "reset" flag?
>> To me what rings a bell in your mail is an assumption that try is
>> always followed by optional field. This tells me that 'try' should
>> rather be a block than a field. By this way we will be able to pair
>> these visually as well as handle situation when there is one try
>> statement but multiple optional fields.
>>
>> Best,
>> Łukasz
>>
>>
>> On 24.09.2021 23:52, Sebastian Rühl wrote:
>>> Hi together,
>>>
>>> I have some exciting changes in the pipeline regarding the mspec:
>>>
>>> 1. parameters on type refs
>>>       with that change it is now possible to target a discriminated child in advance.
>>> 2. assert keyword
>>>       with that change it is possible to throw a
>>> ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
>>>       with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
>>> 4. const is now extended to type reference
>>>      this change allows enums to be used as const values.
>>>
>>> All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
>>>
>>> Here is a example working with bacnet:
>>>           ['0x07' BACnetUnconfirmedServiceRequestWhoHas
>>>               [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
>>>               [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
>>>               [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
>>>               [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
>>>           ]
>>>
>>> The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
>>>
>>> Here is the snippet from the parent type:
>>>
>>> [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
>>>       [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
>>>       [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
>>>       [simple        uint 3           'lengthValueType'                                                                         ]
>>>       .....
>>>       [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
>>>       [typeSwitch 'dataType'
>>>           ....
>>>           ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
>>>               // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
>>>               [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
>>>               [simple     string 'actualLengthInBit' 'ASCII'   'theString']
>>>           ]
>>>
>>> Would love to hear some opinions! If there are no objections I would push this change to develop soon.
>>>
>>> - Sebastian
>>>
>>> PatchContent:
>>> Index:
>>> code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x
>>> /plugins/codegenerator/language/mspec/MSpec.g4
>>> <+>UTF-8
>>> ===================================================================
>>> diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
>>> --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4    (revision ef35531d5a872f29dccddb3a11a135b166958185)
>>> +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4    (date 1632518519426)
>>> @@ -34,7 +34,7 @@
>>>     ;
>>>
>>>    fieldDefinition
>>> - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)?
>>> RBRACKET
>>> + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions
>>> + RBRACKET)? RBRACKET
>>>     ;
>>>
>>>    dataIoDefinition
>>> @@ -49,6 +49,7 @@
>>>     | discriminatorField
>>>     | enumField
>>>     | implicitField
>>> + | assertField
>>>     | manualArrayField
>>>     | manualField
>>>     | optionalField
>>> @@ -73,7 +74,7 @@
>>>     ;
>>>
>>>    constField
>>> - : 'const' type=dataType name=idExpression expected=expression
>>> + : 'const' type=typeReference name=idExpression expected=expression
>>>     ;
>>>
>>>    discriminatorField
>>> @@ -88,6 +89,10 @@
>>>     : 'implicit' type=dataType name=idExpression serializeExpression=expression
>>>     ;
>>>
>>> +assertField
>>> + : 'assert' type=typeReference name=idExpression
>>> +condition=expression  ;
>>> +
>>>    manualArrayField
>>>     : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
>>>     ;
>>> @@ -129,7 +134,7 @@
>>>     ;
>>>
>>>    typeReference
>>> - : complexTypeReference=IDENTIFIER_LITERAL
>>> + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
>>>     | simpleTypeReference=dataType
>>>     ;
>>>
>>> @@ -150,6 +155,10 @@
>>>     | base='dateTime'
>>>     ;
>>>
>>> +tryParse
>>> + : 'try'
>>> + ;
>>> +
>>>    argument
>>>     : type=typeReference name=idExpression
>>>     ;
>>>
>>

Re: AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Łukasz Dywicki <lu...@code-house.org>.
Hey Chris,

Given the code generation is a serious piece of work and code to go with 
I wanted to highlight just one thing. Lambda expressions might result in 
messy stack traces.
Since frame parsing in some ways is recursive process it migth result in 
multiple lambdas on stack trace. As long as we have conditionality only 
with primitive fields it will work. Yet it will be much harder to debug 
generated code if we will chain complex types (de)serialization.

I did update a while ago java ConversationContext to not rely on lambdas 
so stack traces are always starting with org.apache.plc4x package. ;-)
All this should not hold you from doing your thing. I believe I will be 
able to de-lambdize java templates if we will find it necessary later on.

Best,
Łukasz


On 07.10.2021 19:08, Christofer Dutz wrote:
> Hi,
> 
> now that I'm sort of allmost finished with adjusting the mspec parsers and model types, I'm planning up update the code generation templates.
> 
> I know that if I do it the normal way, that this would excessivley blow up the complexity and size of the template files as well as that of the produced code.
> 
> As this code has become more and more complex over the last year anyway, I would like to radically clean up the code here.
> 
> For that I had an idea, that I formalized a bit in the Confluence page: https://cwiki.apache.org/confluence/display/PLC4X/MSPEC+improvements
> 
> TL/DR: I would create static methods that handle the reading/writing of every field type and simply use these instead of generating code for every field.
> 
> Feel free to comment,
> 
> Chris
> 
> 
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Montag, 4. Oktober 2021 08:55
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
> 
> Hi all,
> 
> so over the weekend Sebastian and I have been working hard on an updated mspec version.
> In the "feature/mspec-ng" is where all of this is happening.
> 
> So far the changes:
> - We introduced a concept of "attributes" which can be added to fields and types (These are name=expression) tuples. These are added before any argument-block. These can currently be used for setting the encoding for string types (possibly even for other types in case we need them). And for the upcomming "endianess" attribute which should allow switching endianess.
> - We introduced a new simple type "vstring" for variable length strings. The expression however is optional.
> - The previous "string" now only accepts fixed number of bits.
> - The encoding for String fields (string and vstring) defaults to "UTF-8" but you can override it with an attribute with: encoding='"UTF-16"'
> - The previous encoding string has been removed from string types)
> - We added a new type of "field": "batchSet" which takes a list of attributes. These are automatically added to all fields it contains. (Currently this is only available in "type", "complexTyte" elements or inside typeSwitch case elements.
> 
> I think this was about it for now ... still need to implement the attribute functionality ... it's currently parsed, but not processed yet.
> 
> Chris
> 
> 
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Dienstag, 28. September 2021 14:57
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
> 
> Another thought I had last night ... while sort of not being able to sleep (at least it hat something good)
> 
> So I think a "try" field should always be an optional field. As it might be set or not, just with the difference that the condition is different.
> 
> This got me thinking even further ... how about making the condition of an "optional" field optional? ... So in general what Sebastian was proposing with a:
> 
> [try simple SomeType 'coolField']
> 
> Would actually be:
> 
> [optional SomeType 'coolField']
> 
> This would simplify things even more...
> 
> Chris
> 
> 
> 
> -----Ursprüngliche Nachricht-----
> Von: Christofer Dutz <ch...@c-ware.de>
> Gesendet: Sonntag, 26. September 2021 16:03
> An: dev@plc4x.apache.org
> Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const
> 
> Bringing the thought of the other discussion here too ...
> 
> How about adding name-value paris to the type declarations as well as to the fields? Then the "try" flag could be replaced with a name-value pair or even a name=expression pair ... sort of like "parse-behaviour=try" or something similar. Then we wouldn't be starting to add all sorts of "flags".
> 
> Chris
> 
> -----Ursprüngliche Nachricht-----
> Von: Sebastian Rühl <sr...@apache.org>
> Gesendet: Sonntag, 26. September 2021 15:44
> An: dev@plc4x.apache.org
> Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const
> 
> Hey Lukaz,
> 
> the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
> These blocks could be indeed come in handy as an option to define things...
> 
> - Sebastian
> 
> On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote:
>> Hey Sebastian,
>> Not that far ago we had a similar discussion. Yet then it was mainly
>> about optionals. See "[DISCUSS] Change the way we use "optional".
>>
>> Back then we did not do any changes to mspec, just ranted about
>> possible approach to make better use of optional fields.
>> I remember that current mspec/code can't for example handle ie. a
>> bacnet whois without all fields. Looking at your example I guess you
>> try to solve that puzzle.
>>
>> I don't have any strong feelings here, but why not using optional
>> field with additional "reset" flag?
>> To me what rings a bell in your mail is an assumption that try is
>> always followed by optional field. This tells me that 'try' should
>> rather be a block than a field. By this way we will be able to pair
>> these visually as well as handle situation when there is one try
>> statement but multiple optional fields.
>>
>> Best,
>> Łukasz
>>
>>
>> On 24.09.2021 23:52, Sebastian Rühl wrote:
>>> Hi together,
>>>
>>> I have some exciting changes in the pipeline regarding the mspec:
>>>
>>> 1. parameters on type refs
>>>       with that change it is now possible to target a discriminated child in advance.
>>> 2. assert keyword
>>>       with that change it is possible to throw a
>>> ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
>>>       with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
>>> 4. const is now extended to type reference
>>>      this change allows enums to be used as const values.
>>>
>>> All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
>>>
>>> Here is a example working with bacnet:
>>>           ['0x07' BACnetUnconfirmedServiceRequestWhoHas
>>>               [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
>>>               [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
>>>               [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
>>>               [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
>>>           ]
>>>
>>> The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
>>>
>>> Here is the snippet from the parent type:
>>>
>>> [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
>>>       [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
>>>       [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
>>>       [simple        uint 3           'lengthValueType'                                                                         ]
>>>       .....
>>>       [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
>>>       [typeSwitch 'dataType'
>>>           ....
>>>           ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
>>>               // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
>>>               [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
>>>               [simple     string 'actualLengthInBit' 'ASCII'   'theString']
>>>           ]
>>>
>>> Would love to hear some opinions! If there are no objections I would push this change to develop soon.
>>>
>>> - Sebastian
>>>
>>> PatchContent:
>>> Index:
>>> code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x
>>> /plugins/codegenerator/language/mspec/MSpec.g4
>>> <+>UTF-8
>>> ===================================================================
>>> diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
>>> --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
>>> +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
>>> @@ -34,7 +34,7 @@
>>>     ;
>>>    
>>>    fieldDefinition
>>> - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)?
>>> RBRACKET
>>> + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions
>>> + RBRACKET)? RBRACKET
>>>     ;
>>>    
>>>    dataIoDefinition
>>> @@ -49,6 +49,7 @@
>>>     | discriminatorField
>>>     | enumField
>>>     | implicitField
>>> + | assertField
>>>     | manualArrayField
>>>     | manualField
>>>     | optionalField
>>> @@ -73,7 +74,7 @@
>>>     ;
>>>    
>>>    constField
>>> - : 'const' type=dataType name=idExpression expected=expression
>>> + : 'const' type=typeReference name=idExpression expected=expression
>>>     ;
>>>    
>>>    discriminatorField
>>> @@ -88,6 +89,10 @@
>>>     : 'implicit' type=dataType name=idExpression serializeExpression=expression
>>>     ;
>>>    
>>> +assertField
>>> + : 'assert' type=typeReference name=idExpression
>>> +condition=expression  ;
>>> +
>>>    manualArrayField
>>>     : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
>>>     ;
>>> @@ -129,7 +134,7 @@
>>>     ;
>>>    
>>>    typeReference
>>> - : complexTypeReference=IDENTIFIER_LITERAL
>>> + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
>>>     | simpleTypeReference=dataType
>>>     ;
>>>    
>>> @@ -150,6 +155,10 @@
>>>     | base='dateTime'
>>>     ;
>>>    
>>> +tryParse
>>> + : 'try'
>>> + ;
>>> +
>>>    argument
>>>     : type=typeReference name=idExpression
>>>     ;
>>>
>>

AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi,

now that I'm sort of allmost finished with adjusting the mspec parsers and model types, I'm planning up update the code generation templates.

I know that if I do it the normal way, that this would excessivley blow up the complexity and size of the template files as well as that of the produced code. 

As this code has become more and more complex over the last year anyway, I would like to radically clean up the code here. 

For that I had an idea, that I formalized a bit in the Confluence page: https://cwiki.apache.org/confluence/display/PLC4X/MSPEC+improvements

TL/DR: I would create static methods that handle the reading/writing of every field type and simply use these instead of generating code for every field.

Feel free to comment,

Chris


-----Ursprüngliche Nachricht-----
Von: Christofer Dutz <ch...@c-ware.de> 
Gesendet: Montag, 4. Oktober 2021 08:55
An: dev@plc4x.apache.org
Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const

Hi all,

so over the weekend Sebastian and I have been working hard on an updated mspec version.
In the "feature/mspec-ng" is where all of this is happening.

So far the changes:
- We introduced a concept of "attributes" which can be added to fields and types (These are name=expression) tuples. These are added before any argument-block. These can currently be used for setting the encoding for string types (possibly even for other types in case we need them). And for the upcomming "endianess" attribute which should allow switching endianess.
- We introduced a new simple type "vstring" for variable length strings. The expression however is optional.
- The previous "string" now only accepts fixed number of bits.
- The encoding for String fields (string and vstring) defaults to "UTF-8" but you can override it with an attribute with: encoding='"UTF-16"'
- The previous encoding string has been removed from string types)
- We added a new type of "field": "batchSet" which takes a list of attributes. These are automatically added to all fields it contains. (Currently this is only available in "type", "complexTyte" elements or inside typeSwitch case elements.

I think this was about it for now ... still need to implement the attribute functionality ... it's currently parsed, but not processed yet.

Chris


-----Ursprüngliche Nachricht-----
Von: Christofer Dutz <ch...@c-ware.de>
Gesendet: Dienstag, 28. September 2021 14:57
An: dev@plc4x.apache.org
Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const

Another thought I had last night ... while sort of not being able to sleep (at least it hat something good)

So I think a "try" field should always be an optional field. As it might be set or not, just with the difference that the condition is different.

This got me thinking even further ... how about making the condition of an "optional" field optional? ... So in general what Sebastian was proposing with a:

[try simple SomeType 'coolField']

Would actually be:

[optional SomeType 'coolField']

This would simplify things even more...

Chris



-----Ursprüngliche Nachricht-----
Von: Christofer Dutz <ch...@c-ware.de>
Gesendet: Sonntag, 26. September 2021 16:03
An: dev@plc4x.apache.org
Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const

Bringing the thought of the other discussion here too ... 

How about adding name-value paris to the type declarations as well as to the fields? Then the "try" flag could be replaced with a name-value pair or even a name=expression pair ... sort of like "parse-behaviour=try" or something similar. Then we wouldn't be starting to add all sorts of "flags".

Chris

-----Ursprüngliche Nachricht-----
Von: Sebastian Rühl <sr...@apache.org>
Gesendet: Sonntag, 26. September 2021 15:44
An: dev@plc4x.apache.org
Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const

Hey Lukaz,

the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
These blocks could be indeed come in handy as an option to define things...

- Sebastian

On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote: 
> Hey Sebastian,
> Not that far ago we had a similar discussion. Yet then it was mainly 
> about optionals. See "[DISCUSS] Change the way we use "optional".
> 
> Back then we did not do any changes to mspec, just ranted about 
> possible approach to make better use of optional fields.
> I remember that current mspec/code can't for example handle ie. a 
> bacnet whois without all fields. Looking at your example I guess you 
> try to solve that puzzle.
> 
> I don't have any strong feelings here, but why not using optional 
> field with additional "reset" flag?
> To me what rings a bell in your mail is an assumption that try is 
> always followed by optional field. This tells me that 'try' should 
> rather be a block than a field. By this way we will be able to pair 
> these visually as well as handle situation when there is one try 
> statement but multiple optional fields.
> 
> Best,
> Łukasz
> 
> 
> On 24.09.2021 23:52, Sebastian Rühl wrote:
> > Hi together,
> > 
> > I have some exciting changes in the pipeline regarding the mspec:
> > 
> > 1. parameters on type refs
> >      with that change it is now possible to target a discriminated child in advance.
> > 2. assert keyword
> >      with that change it is possible to throw a 
> > ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
> >      with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
> > 4. const is now extended to type reference
> >     this change allows enums to be used as const values.
> > 
> > All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
> > 
> > Here is a example working with bacnet:
> >          ['0x07' BACnetUnconfirmedServiceRequestWhoHas
> >              [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
> >              [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
> >              [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
> >              [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
> >          ]
> > 
> > The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
> > 
> > Here is the snippet from the parent type:
> > 
> > [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
> >      [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
> >      [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
> >      [simple        uint 3           'lengthValueType'                                                                         ]
> >      .....
> >      [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
> >      [typeSwitch 'dataType'
> >          ....
> >          ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
> >              // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
> >              [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
> >              [simple     string 'actualLengthInBit' 'ASCII'   'theString']
> >          ]
> > 
> > Would love to hear some opinions! If there are no objections I would push this change to develop soon.
> > 
> > - Sebastian
> > 
> > PatchContent:
> > Index: 
> > code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x
> > /plugins/codegenerator/language/mspec/MSpec.g4
> > <+>UTF-8
> > ===================================================================
> > diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> > --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
> > +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
> > @@ -34,7 +34,7 @@
> >    ;
> >   
> >   fieldDefinition
> > - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? 
> > RBRACKET
> > + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions 
> > + RBRACKET)? RBRACKET
> >    ;
> >   
> >   dataIoDefinition
> > @@ -49,6 +49,7 @@
> >    | discriminatorField
> >    | enumField
> >    | implicitField
> > + | assertField
> >    | manualArrayField
> >    | manualField
> >    | optionalField
> > @@ -73,7 +74,7 @@
> >    ;
> >   
> >   constField
> > - : 'const' type=dataType name=idExpression expected=expression
> > + : 'const' type=typeReference name=idExpression expected=expression
> >    ;
> >   
> >   discriminatorField
> > @@ -88,6 +89,10 @@
> >    : 'implicit' type=dataType name=idExpression serializeExpression=expression
> >    ;
> >   
> > +assertField
> > + : 'assert' type=typeReference name=idExpression 
> > +condition=expression  ;
> > +
> >   manualArrayField
> >    : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
> >    ;
> > @@ -129,7 +134,7 @@
> >    ;
> >   
> >   typeReference
> > - : complexTypeReference=IDENTIFIER_LITERAL
> > + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
> >    | simpleTypeReference=dataType
> >    ;
> >   
> > @@ -150,6 +155,10 @@
> >    | base='dateTime'
> >    ;
> >   
> > +tryParse
> > + : 'try'
> > + ;
> > +
> >   argument
> >    : type=typeReference name=idExpression
> >    ;
> > 
> 

AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi all,

so over the weekend Sebastian and I have been working hard on an updated mspec version.
In the "feature/mspec-ng" is where all of this is happening.

So far the changes:
- We introduced a concept of "attributes" which can be added to fields and types (These are name=expression) tuples. These are added before any argument-block. These can currently be used for setting the encoding for string types (possibly even for other types in case we need them). And for the upcomming "endianess" attribute which should allow switching endianess.
- We introduced a new simple type "vstring" for variable length strings. The expression however is optional.
- The previous "string" now only accepts fixed number of bits.
- The encoding for String fields (string and vstring) defaults to "UTF-8" but you can override it with an attribute with: encoding='"UTF-16"'
- The previous encoding string has been removed from string types)
- We added a new type of "field": "batchSet" which takes a list of attributes. These are automatically added to all fields it contains. (Currently this is only available in "type", "complexTyte" elements or inside typeSwitch case elements.

I think this was about it for now ... still need to implement the attribute functionality ... it's currently parsed, but not processed yet.

Chris


-----Ursprüngliche Nachricht-----
Von: Christofer Dutz <ch...@c-ware.de> 
Gesendet: Dienstag, 28. September 2021 14:57
An: dev@plc4x.apache.org
Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const

Another thought I had last night ... while sort of not being able to sleep (at least it hat something good)

So I think a "try" field should always be an optional field. As it might be set or not, just with the difference that the condition is different.

This got me thinking even further ... how about making the condition of an "optional" field optional? ... So in general what Sebastian was proposing with a:

[try simple SomeType 'coolField']

Would actually be:

[optional SomeType 'coolField']

This would simplify things even more...

Chris



-----Ursprüngliche Nachricht-----
Von: Christofer Dutz <ch...@c-ware.de>
Gesendet: Sonntag, 26. September 2021 16:03
An: dev@plc4x.apache.org
Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const

Bringing the thought of the other discussion here too ... 

How about adding name-value paris to the type declarations as well as to the fields? Then the "try" flag could be replaced with a name-value pair or even a name=expression pair ... sort of like "parse-behaviour=try" or something similar. Then we wouldn't be starting to add all sorts of "flags".

Chris

-----Ursprüngliche Nachricht-----
Von: Sebastian Rühl <sr...@apache.org>
Gesendet: Sonntag, 26. September 2021 15:44
An: dev@plc4x.apache.org
Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const

Hey Lukaz,

the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
These blocks could be indeed come in handy as an option to define things...

- Sebastian

On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote: 
> Hey Sebastian,
> Not that far ago we had a similar discussion. Yet then it was mainly 
> about optionals. See "[DISCUSS] Change the way we use "optional".
> 
> Back then we did not do any changes to mspec, just ranted about 
> possible approach to make better use of optional fields.
> I remember that current mspec/code can't for example handle ie. a 
> bacnet whois without all fields. Looking at your example I guess you 
> try to solve that puzzle.
> 
> I don't have any strong feelings here, but why not using optional 
> field with additional "reset" flag?
> To me what rings a bell in your mail is an assumption that try is 
> always followed by optional field. This tells me that 'try' should 
> rather be a block than a field. By this way we will be able to pair 
> these visually as well as handle situation when there is one try 
> statement but multiple optional fields.
> 
> Best,
> Łukasz
> 
> 
> On 24.09.2021 23:52, Sebastian Rühl wrote:
> > Hi together,
> > 
> > I have some exciting changes in the pipeline regarding the mspec:
> > 
> > 1. parameters on type refs
> >      with that change it is now possible to target a discriminated child in advance.
> > 2. assert keyword
> >      with that change it is possible to throw a 
> > ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
> >      with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
> > 4. const is now extended to type reference
> >     this change allows enums to be used as const values.
> > 
> > All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
> > 
> > Here is a example working with bacnet:
> >          ['0x07' BACnetUnconfirmedServiceRequestWhoHas
> >              [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
> >              [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
> >              [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
> >              [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
> >          ]
> > 
> > The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
> > 
> > Here is the snippet from the parent type:
> > 
> > [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
> >      [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
> >      [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
> >      [simple        uint 3           'lengthValueType'                                                                         ]
> >      .....
> >      [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
> >      [typeSwitch 'dataType'
> >          ....
> >          ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
> >              // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
> >              [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
> >              [simple     string 'actualLengthInBit' 'ASCII'   'theString']
> >          ]
> > 
> > Would love to hear some opinions! If there are no objections I would push this change to develop soon.
> > 
> > - Sebastian
> > 
> > PatchContent:
> > Index: 
> > code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x
> > /plugins/codegenerator/language/mspec/MSpec.g4
> > <+>UTF-8
> > ===================================================================
> > diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> > --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
> > +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
> > @@ -34,7 +34,7 @@
> >    ;
> >   
> >   fieldDefinition
> > - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? 
> > RBRACKET
> > + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions 
> > + RBRACKET)? RBRACKET
> >    ;
> >   
> >   dataIoDefinition
> > @@ -49,6 +49,7 @@
> >    | discriminatorField
> >    | enumField
> >    | implicitField
> > + | assertField
> >    | manualArrayField
> >    | manualField
> >    | optionalField
> > @@ -73,7 +74,7 @@
> >    ;
> >   
> >   constField
> > - : 'const' type=dataType name=idExpression expected=expression
> > + : 'const' type=typeReference name=idExpression expected=expression
> >    ;
> >   
> >   discriminatorField
> > @@ -88,6 +89,10 @@
> >    : 'implicit' type=dataType name=idExpression serializeExpression=expression
> >    ;
> >   
> > +assertField
> > + : 'assert' type=typeReference name=idExpression 
> > +condition=expression  ;
> > +
> >   manualArrayField
> >    : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
> >    ;
> > @@ -129,7 +134,7 @@
> >    ;
> >   
> >   typeReference
> > - : complexTypeReference=IDENTIFIER_LITERAL
> > + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
> >    | simpleTypeReference=dataType
> >    ;
> >   
> > @@ -150,6 +155,10 @@
> >    | base='dateTime'
> >    ;
> >   
> > +tryParse
> > + : 'try'
> > + ;
> > +
> >   argument
> >    : type=typeReference name=idExpression
> >    ;
> > 
> 

AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
Another thought I had last night ... while sort of not being able to sleep (at least it hat something good)

So I think a "try" field should always be an optional field. As it might be set or not, just with the difference that the condition is different.

This got me thinking even further ... how about making the condition of an "optional" field optional? ... So in general what Sebastian was proposing with a:

[try simple SomeType 'coolField']

Would actually be:

[optional SomeType 'coolField']

This would simplify things even more...

Chris



-----Ursprüngliche Nachricht-----
Von: Christofer Dutz <ch...@c-ware.de> 
Gesendet: Sonntag, 26. September 2021 16:03
An: dev@plc4x.apache.org
Betreff: AW: Changes on mspec: parameterized type refs, assert, try, const

Bringing the thought of the other discussion here too ... 

How about adding name-value paris to the type declarations as well as to the fields? Then the "try" flag could be replaced with a name-value pair or even a name=expression pair ... sort of like "parse-behaviour=try" or something similar. Then we wouldn't be starting to add all sorts of "flags".

Chris

-----Ursprüngliche Nachricht-----
Von: Sebastian Rühl <sr...@apache.org>
Gesendet: Sonntag, 26. September 2021 15:44
An: dev@plc4x.apache.org
Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const

Hey Lukaz,

the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
These blocks could be indeed come in handy as an option to define things...

- Sebastian

On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote: 
> Hey Sebastian,
> Not that far ago we had a similar discussion. Yet then it was mainly 
> about optionals. See "[DISCUSS] Change the way we use "optional".
> 
> Back then we did not do any changes to mspec, just ranted about 
> possible approach to make better use of optional fields.
> I remember that current mspec/code can't for example handle ie. a 
> bacnet whois without all fields. Looking at your example I guess you 
> try to solve that puzzle.
> 
> I don't have any strong feelings here, but why not using optional 
> field with additional "reset" flag?
> To me what rings a bell in your mail is an assumption that try is 
> always followed by optional field. This tells me that 'try' should 
> rather be a block than a field. By this way we will be able to pair 
> these visually as well as handle situation when there is one try 
> statement but multiple optional fields.
> 
> Best,
> Łukasz
> 
> 
> On 24.09.2021 23:52, Sebastian Rühl wrote:
> > Hi together,
> > 
> > I have some exciting changes in the pipeline regarding the mspec:
> > 
> > 1. parameters on type refs
> >      with that change it is now possible to target a discriminated child in advance.
> > 2. assert keyword
> >      with that change it is possible to throw a 
> > ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
> >      with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
> > 4. const is now extended to type reference
> >     this change allows enums to be used as const values.
> > 
> > All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
> > 
> > Here is a example working with bacnet:
> >          ['0x07' BACnetUnconfirmedServiceRequestWhoHas
> >              [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
> >              [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
> >              [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
> >              [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
> >          ]
> > 
> > The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
> > 
> > Here is the snippet from the parent type:
> > 
> > [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
> >      [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
> >      [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
> >      [simple        uint 3           'lengthValueType'                                                                         ]
> >      .....
> >      [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
> >      [typeSwitch 'dataType'
> >          ....
> >          ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
> >              // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
> >              [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
> >              [simple     string 'actualLengthInBit' 'ASCII'   'theString']
> >          ]
> > 
> > Would love to hear some opinions! If there are no objections I would push this change to develop soon.
> > 
> > - Sebastian
> > 
> > PatchContent:
> > Index: 
> > code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x
> > /plugins/codegenerator/language/mspec/MSpec.g4
> > <+>UTF-8
> > ===================================================================
> > diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> > --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
> > +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
> > @@ -34,7 +34,7 @@
> >    ;
> >   
> >   fieldDefinition
> > - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? 
> > RBRACKET
> > + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions 
> > + RBRACKET)? RBRACKET
> >    ;
> >   
> >   dataIoDefinition
> > @@ -49,6 +49,7 @@
> >    | discriminatorField
> >    | enumField
> >    | implicitField
> > + | assertField
> >    | manualArrayField
> >    | manualField
> >    | optionalField
> > @@ -73,7 +74,7 @@
> >    ;
> >   
> >   constField
> > - : 'const' type=dataType name=idExpression expected=expression
> > + : 'const' type=typeReference name=idExpression expected=expression
> >    ;
> >   
> >   discriminatorField
> > @@ -88,6 +89,10 @@
> >    : 'implicit' type=dataType name=idExpression serializeExpression=expression
> >    ;
> >   
> > +assertField
> > + : 'assert' type=typeReference name=idExpression 
> > +condition=expression  ;
> > +
> >   manualArrayField
> >    : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
> >    ;
> > @@ -129,7 +134,7 @@
> >    ;
> >   
> >   typeReference
> > - : complexTypeReference=IDENTIFIER_LITERAL
> > + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
> >    | simpleTypeReference=dataType
> >    ;
> >   
> > @@ -150,6 +155,10 @@
> >    | base='dateTime'
> >    ;
> >   
> > +tryParse
> > + : 'try'
> > + ;
> > +
> >   argument
> >    : type=typeReference name=idExpression
> >    ;
> > 
> 

AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
Bringing the thought of the other discussion here too ... 

How about adding name-value paris to the type declarations as well as to the fields? Then the "try" flag could be replaced with a name-value pair or even a name=expression pair ... sort of like "parse-behaviour=try" or something similar. Then we wouldn't be starting to add all sorts of "flags".

Chris

-----Ursprüngliche Nachricht-----
Von: Sebastian Rühl <sr...@apache.org> 
Gesendet: Sonntag, 26. September 2021 15:44
An: dev@plc4x.apache.org
Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const

Hey Lukaz,

the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
These blocks could be indeed come in handy as an option to define things...

- Sebastian

On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote: 
> Hey Sebastian,
> Not that far ago we had a similar discussion. Yet then it was mainly 
> about optionals. See "[DISCUSS] Change the way we use "optional".
> 
> Back then we did not do any changes to mspec, just ranted about 
> possible approach to make better use of optional fields.
> I remember that current mspec/code can't for example handle ie. a 
> bacnet whois without all fields. Looking at your example I guess you 
> try to solve that puzzle.
> 
> I don't have any strong feelings here, but why not using optional 
> field with additional "reset" flag?
> To me what rings a bell in your mail is an assumption that try is 
> always followed by optional field. This tells me that 'try' should 
> rather be a block than a field. By this way we will be able to pair 
> these visually as well as handle situation when there is one try 
> statement but multiple optional fields.
> 
> Best,
> Łukasz
> 
> 
> On 24.09.2021 23:52, Sebastian Rühl wrote:
> > Hi together,
> > 
> > I have some exciting changes in the pipeline regarding the mspec:
> > 
> > 1. parameters on type refs
> >      with that change it is now possible to target a discriminated child in advance.
> > 2. assert keyword
> >      with that change it is possible to throw a 
> > ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
> >      with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
> > 4. const is now extended to type reference
> >     this change allows enums to be used as const values.
> > 
> > All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
> > 
> > Here is a example working with bacnet:
> >          ['0x07' BACnetUnconfirmedServiceRequestWhoHas
> >              [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
> >              [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
> >              [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
> >              [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
> >          ]
> > 
> > The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
> > 
> > Here is the snippet from the parent type:
> > 
> > [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
> >      [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
> >      [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
> >      [simple        uint 3           'lengthValueType'                                                                         ]
> >      .....
> >      [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
> >      [typeSwitch 'dataType'
> >          ....
> >          ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
> >              // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
> >              [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
> >              [simple     string 'actualLengthInBit' 'ASCII'   'theString']
> >          ]
> > 
> > Would love to hear some opinions! If there are no objections I would push this change to develop soon.
> > 
> > - Sebastian
> > 
> > PatchContent:
> > Index: 
> > code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x
> > /plugins/codegenerator/language/mspec/MSpec.g4
> > <+>UTF-8
> > ===================================================================
> > diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> > --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
> > +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
> > @@ -34,7 +34,7 @@
> >    ;
> >   
> >   fieldDefinition
> > - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? 
> > RBRACKET
> > + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions 
> > + RBRACKET)? RBRACKET
> >    ;
> >   
> >   dataIoDefinition
> > @@ -49,6 +49,7 @@
> >    | discriminatorField
> >    | enumField
> >    | implicitField
> > + | assertField
> >    | manualArrayField
> >    | manualField
> >    | optionalField
> > @@ -73,7 +74,7 @@
> >    ;
> >   
> >   constField
> > - : 'const' type=dataType name=idExpression expected=expression
> > + : 'const' type=typeReference name=idExpression expected=expression
> >    ;
> >   
> >   discriminatorField
> > @@ -88,6 +89,10 @@
> >    : 'implicit' type=dataType name=idExpression serializeExpression=expression
> >    ;
> >   
> > +assertField
> > + : 'assert' type=typeReference name=idExpression 
> > +condition=expression  ;
> > +
> >   manualArrayField
> >    : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
> >    ;
> > @@ -129,7 +134,7 @@
> >    ;
> >   
> >   typeReference
> > - : complexTypeReference=IDENTIFIER_LITERAL
> > + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
> >    | simpleTypeReference=dataType
> >    ;
> >   
> > @@ -150,6 +155,10 @@
> >    | base='dateTime'
> >    ;
> >   
> > +tryParse
> > + : 'try'
> > + ;
> > +
> >   argument
> >    : type=typeReference name=idExpression
> >    ;
> > 
> 

Re: Changes on mspec: parameterized type refs, assert, try, const

Posted by Sebastian R��hl <sr...@apache.org>.
Hey Lukaz,

the optional after the try is just a coincidence from the bac spec. Like it say the low should always appear with a high and if the id is set then no name should not be set. But in itself the try is self sufficient and behaves indeed like an optional without an expression. Other than that you can always group dependended fields as a complex... but let's see how this evolves.
These blocks could be indeed come in handy as an option to define things...

- Sebastian

On 2021/09/25 20:23:08, Łukasz Dywicki <lu...@code-house.org> wrote: 
> Hey Sebastian,
> Not that far ago we had a similar discussion. Yet then it was mainly 
> about optionals. See "[DISCUSS] Change the way we use "optional".
> 
> Back then we did not do any changes to mspec, just ranted about possible 
> approach to make better use of optional fields.
> I remember that current mspec/code can't for example handle ie. a bacnet 
> whois without all fields. Looking at your example I guess you try to 
> solve that puzzle.
> 
> I don't have any strong feelings here, but why not using optional field 
> with additional "reset" flag?
> To me what rings a bell in your mail is an assumption that try is always 
> followed by optional field. This tells me that 'try' should rather be a 
> block than a field. By this way we will be able to pair these visually 
> as well as handle situation when there is one try statement but multiple 
> optional fields.
> 
> Best,
> Łukasz
> 
> 
> On 24.09.2021 23:52, Sebastian Rühl wrote:
> > Hi together,
> > 
> > I have some exciting changes in the pipeline regarding the mspec:
> > 
> > 1. parameters on type refs
> >      with that change it is now possible to target a discriminated child in advance.
> > 2. assert keyword
> >      with that change it is possible to throw a ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop)
> > 3. try keyword to prefix fields:
> >      with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
> > 4. const is now extended to type reference
> >     this change allows enums to be used as const values.
> > 
> > All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
> > 
> > Here is a example working with bacnet:
> >          ['0x07' BACnetUnconfirmedServiceRequestWhoHas
> >              [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
> >              [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
> >              [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
> >              [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
> >          ]
> > 
> > The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
> > 
> > Here is the snippet from the parent type:
> > 
> > [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
> >      [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
> >      [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
> >      [simple        uint 3           'lengthValueType'                                                                         ]
> >      .....
> >      [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
> >      [typeSwitch 'dataType'
> >          ....
> >          ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
> >              // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
> >              [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
> >              [simple     string 'actualLengthInBit' 'ASCII'   'theString']
> >          ]
> > 
> > Would love to hear some opinions! If there are no objections I would push this change to develop soon.
> > 
> > - Sebastian
> > 
> > PatchContent:
> > Index: code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> > <+>UTF-8
> > ===================================================================
> > diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> > --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
> > +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
> > @@ -34,7 +34,7 @@
> >    ;
> >   
> >   fieldDefinition
> > - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? RBRACKET
> > + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions RBRACKET)? RBRACKET
> >    ;
> >   
> >   dataIoDefinition
> > @@ -49,6 +49,7 @@
> >    | discriminatorField
> >    | enumField
> >    | implicitField
> > + | assertField
> >    | manualArrayField
> >    | manualField
> >    | optionalField
> > @@ -73,7 +74,7 @@
> >    ;
> >   
> >   constField
> > - : 'const' type=dataType name=idExpression expected=expression
> > + : 'const' type=typeReference name=idExpression expected=expression
> >    ;
> >   
> >   discriminatorField
> > @@ -88,6 +89,10 @@
> >    : 'implicit' type=dataType name=idExpression serializeExpression=expression
> >    ;
> >   
> > +assertField
> > + : 'assert' type=typeReference name=idExpression condition=expression
> > + ;
> > +
> >   manualArrayField
> >    : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
> >    ;
> > @@ -129,7 +134,7 @@
> >    ;
> >   
> >   typeReference
> > - : complexTypeReference=IDENTIFIER_LITERAL
> > + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
> >    | simpleTypeReference=dataType
> >    ;
> >   
> > @@ -150,6 +155,10 @@
> >    | base='dateTime'
> >    ;
> >   
> > +tryParse
> > + : 'try'
> > + ;
> > +
> >   argument
> >    : type=typeReference name=idExpression
> >    ;
> > 
> 

AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
Hmm ... you actually bring up a good point here Lukasz.

In the end doesn't the "try" flag simply make it an optional with a different type of optionality condition?
Cause right now there are differences between optional and simple types in the way the references are stored.
For example a "uint 8" is of type "short" for a simple and "Short" for an optional. I know that probably simple type references are probably not going to use "try", but perhaps we should instead make the optional field have an expression and/or the try flag.

This would make it a bit more consistent ... 

Sort of something like this:


Classical:
[optional MyCoolType 'coolOptionalField' 'expression']

New "try" option:
[optional MyCoolType 'coolOptionalField' try]

Combined "try" with "condition":
[optional MyCoolType 'coolOptionalField' try 'expression']

What do you folks think?

Chris


-----Ursprüngliche Nachricht-----
Von: Łukasz Dywicki <lu...@code-house.org> 
Gesendet: Samstag, 25. September 2021 22:23
An: dev@plc4x.apache.org
Betreff: Re: Changes on mspec: parameterized type refs, assert, try, const

Hey Sebastian,
Not that far ago we had a similar discussion. Yet then it was mainly about optionals. See "[DISCUSS] Change the way we use "optional".

Back then we did not do any changes to mspec, just ranted about possible approach to make better use of optional fields.
I remember that current mspec/code can't for example handle ie. a bacnet whois without all fields. Looking at your example I guess you try to solve that puzzle.

I don't have any strong feelings here, but why not using optional field with additional "reset" flag?
To me what rings a bell in your mail is an assumption that try is always followed by optional field. This tells me that 'try' should rather be a block than a field. By this way we will be able to pair these visually as well as handle situation when there is one try statement but multiple optional fields.

Best,
Łukasz


On 24.09.2021 23:52, Sebastian Rühl wrote:
> Hi together,
> 
> I have some exciting changes in the pipeline regarding the mspec:
> 
> 1. parameters on type refs
>      with that change it is now possible to target a discriminated child in advance.
> 2. assert keyword
>      with that change it is possible to throw a ParserAssertException 
> (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
>      with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
> 4. const is now extended to type reference
>     this change allows enums to be used as const values.
> 
> All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
> 
> Here is a example working with bacnet:
>          ['0x07' BACnetUnconfirmedServiceRequestWhoHas
>              [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
>              [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
>              [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
>              [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
>          ]
> 
> The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
> 
> Here is the snippet from the parent type:
> 
> [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
>      [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
>      [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
>      [simple        uint 3           'lengthValueType'                                                                         ]
>      .....
>      [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
>      [typeSwitch 'dataType'
>          ....
>          ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
>              // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
>              [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
>              [simple     string 'actualLengthInBit' 'ASCII'   'theString']
>          ]
> 
> Would love to hear some opinions! If there are no objections I would push this change to develop soon.
> 
> - Sebastian
> 
> PatchContent:
> Index: 
> code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/p
> lugins/codegenerator/language/mspec/MSpec.g4
> <+>UTF-8
> ===================================================================
> diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
> +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
> @@ -34,7 +34,7 @@
>    ;
>   
>   fieldDefinition
> - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? 
> RBRACKET
> + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions 
> + RBRACKET)? RBRACKET
>    ;
>   
>   dataIoDefinition
> @@ -49,6 +49,7 @@
>    | discriminatorField
>    | enumField
>    | implicitField
> + | assertField
>    | manualArrayField
>    | manualField
>    | optionalField
> @@ -73,7 +74,7 @@
>    ;
>   
>   constField
> - : 'const' type=dataType name=idExpression expected=expression
> + : 'const' type=typeReference name=idExpression expected=expression
>    ;
>   
>   discriminatorField
> @@ -88,6 +89,10 @@
>    : 'implicit' type=dataType name=idExpression serializeExpression=expression
>    ;
>   
> +assertField
> + : 'assert' type=typeReference name=idExpression condition=expression  
> +;
> +
>   manualArrayField
>    : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
>    ;
> @@ -129,7 +134,7 @@
>    ;
>   
>   typeReference
> - : complexTypeReference=IDENTIFIER_LITERAL
> + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
>    | simpleTypeReference=dataType
>    ;
>   
> @@ -150,6 +155,10 @@
>    | base='dateTime'
>    ;
>   
> +tryParse
> + : 'try'
> + ;
> +
>   argument
>    : type=typeReference name=idExpression
>    ;
> 

Re: Changes on mspec: parameterized type refs, assert, try, const

Posted by Łukasz Dywicki <lu...@code-house.org>.
Hey Sebastian,
Not that far ago we had a similar discussion. Yet then it was mainly 
about optionals. See "[DISCUSS] Change the way we use "optional".

Back then we did not do any changes to mspec, just ranted about possible 
approach to make better use of optional fields.
I remember that current mspec/code can't for example handle ie. a bacnet 
whois without all fields. Looking at your example I guess you try to 
solve that puzzle.

I don't have any strong feelings here, but why not using optional field 
with additional "reset" flag?
To me what rings a bell in your mail is an assumption that try is always 
followed by optional field. This tells me that 'try' should rather be a 
block than a field. By this way we will be able to pair these visually 
as well as handle situation when there is one try statement but multiple 
optional fields.

Best,
Łukasz


On 24.09.2021 23:52, Sebastian Rühl wrote:
> Hi together,
> 
> I have some exciting changes in the pipeline regarding the mspec:
> 
> 1. parameters on type refs
>      with that change it is now possible to target a discriminated child in advance.
> 2. assert keyword
>      with that change it is possible to throw a ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop)
> 3. try keyword to prefix fields:
>      with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
> 4. const is now extended to type reference
>     this change allows enums to be used as const values.
> 
> All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.
> 
> Here is a example working with bacnet:
>          ['0x07' BACnetUnconfirmedServiceRequestWhoHas
>              [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
>              [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
>              [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
>              [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
>          ]
> 
> The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.
> 
> Here is the snippet from the parent type:
> 
> [discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
>      [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
>      [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
>      [simple        uint 3           'lengthValueType'                                                                         ]
>      .....
>      [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
>      [typeSwitch 'dataType'
>          ....
>          ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
>              // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
>              [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
>              [simple     string 'actualLengthInBit' 'ASCII'   'theString']
>          ]
> 
> Would love to hear some opinions! If there are no objections I would push this change to develop soon.
> 
> - Sebastian
> 
> PatchContent:
> Index: code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> <+>UTF-8
> ===================================================================
> diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
> --- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
> +++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
> @@ -34,7 +34,7 @@
>    ;
>   
>   fieldDefinition
> - : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? RBRACKET
> + : LBRACKET tryParse? field (LBRACKET params=multipleExpressions RBRACKET)? RBRACKET
>    ;
>   
>   dataIoDefinition
> @@ -49,6 +49,7 @@
>    | discriminatorField
>    | enumField
>    | implicitField
> + | assertField
>    | manualArrayField
>    | manualField
>    | optionalField
> @@ -73,7 +74,7 @@
>    ;
>   
>   constField
> - : 'const' type=dataType name=idExpression expected=expression
> + : 'const' type=typeReference name=idExpression expected=expression
>    ;
>   
>   discriminatorField
> @@ -88,6 +89,10 @@
>    : 'implicit' type=dataType name=idExpression serializeExpression=expression
>    ;
>   
> +assertField
> + : 'assert' type=typeReference name=idExpression condition=expression
> + ;
> +
>   manualArrayField
>    : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
>    ;
> @@ -129,7 +134,7 @@
>    ;
>   
>   typeReference
> - : complexTypeReference=IDENTIFIER_LITERAL
> + : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
>    | simpleTypeReference=dataType
>    ;
>   
> @@ -150,6 +155,10 @@
>    | base='dateTime'
>    ;
>   
> +tryParse
> + : 'try'
> + ;
> +
>   argument
>    : type=typeReference name=idExpression
>    ;
> 

AW: Changes on mspec: parameterized type refs, assert, try, const

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Sebastian,

As you've applied all of the comments that we talked about during that call yesterday, I don't have any objections.
The change to use enums in consts makes a lot of sense. 

One thing I think you missed out on explaining in detail. Just as "discriminator" fields are the natural partners of "typeSwitch" fields, are "assert" fields the partners of references with the "try" keyword.

The reasoning behind this is, that if a normal parse exception happens while parsing try type, this will fail the parsing just as all others. For example if a constant has the wrong value, no typeSwitch case is found, ... The only exception for this are these ParserAssertException which are only thrown if a parsed value doesn't match an expected value in an "assert" field. These ParserAssertException are then caught by the try functionality. Before parsing a try field, the position in the ReadBuffer is saved and if the try fails because of an AssertException, then the ReadBuffer is reset to this position, so it's generally in the same state as before trying to read the field.

Chris

 

-----Ursprüngliche Nachricht-----
Von: Sebastian Rühl <sr...@apache.org> 
Gesendet: Freitag, 24. September 2021 23:53
An: dev@plc4x.apache.org
Betreff: Changes on mspec: parameterized type refs, assert, try, const

Hi together,

I have some exciting changes in the pipeline regarding the mspec:

1. parameters on type refs
    with that change it is now possible to target a discriminated child in advance.
2. assert keyword
    with that change it is possible to throw a ParserAssertException (in java, or errors in other languages). This field is similar to a const but instead of a ParseException a ParserAssertException is thrown. In contrast to a const the check expression can be dynamic (e.g. virtual fields now working on develop) 3. try keyword to prefix fields:
    with that change it is possible to try to parse some content and in case an assert fails it resets the buffer.
4. const is now extended to type reference
   this change allows enums to be used as const values.

All theses changes allow to encapsulate behavior in complex types so you don't need to DRY.

Here is a example working with bacnet:
        ['0x07' BACnetUnconfirmedServiceRequestWhoHas
            [try simple     BACnetComplexTagUnsignedInteger ['0', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeLowLimit'  ]
            [optional       BACnetComplexTagUnsignedInteger ['1', 'BACnetDataType.UNSIGNED_INTEGER' ] 'deviceInstanceRangeHighLimit' 'deviceInstanceRangeLowLimit != null']
            [try simple     BACnetComplexTagOctetString     ['2', 'BACnetDataType.OCTET_STRING'     ] 'objectIdentifier' ]
            [optional       BACnetComplexTagOctetString     ['3', 'BACnetDataType.OCTET_STRING'     ] 'objectName' 'objectIdentifier == null' ]
        ]

The logic if a type matches is asserted in the type itself. The second optional implies when the first element appears the second must be present. The last one tries to read and if it fails it uses the second type.

Here is the snippet from the parent type:

[discriminatedType 'BACnetComplexTag' [uint 4 'tagNumberArgument', BACnetDataType 'dataType']
    [assert        uint 4           'tagNumber'                 'tagNumberArgument'                                           ]
    [const         TagClass         'tagClass'                  'TagClass.CONTEXT_SPECIFIC_TAGS'                              ]
    [simple        uint 3           'lengthValueType'                                                                         ]
    .....
    [virtual       uint 32          'actualLength'     'lengthValueType == 5 ....']
    [typeSwitch 'dataType'
        ....
        ['OCTET_STRING' BACnetComplexTagOctetString [uint 32 'actualLength']
            // TODO: The reader expects int but uint32 get's mapped to long so even uint32 would easily overflow...
            [virtual    uint    16                           'actualLengthInBit' 'actualLength * 8']
            [simple     string 'actualLengthInBit' 'ASCII'   'theString']
        ]

Would love to hear some opinions! If there are no objections I would push this change to develop soon.

- Sebastian

PatchContent:
Index: code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
<+>UTF-8
===================================================================
diff --git a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4 b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4
--- a/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(revision ef35531d5a872f29dccddb3a11a135b166958185)
+++ b/code-generation/protocol-base-mspec/src/main/antlr4/org/apache/plc4x/plugins/codegenerator/language/mspec/MSpec.g4	(date 1632518519426)
@@ -34,7 +34,7 @@
  ;
 
 fieldDefinition
- : LBRACKET field (LBRACKET params=multipleExpressions RBRACKET)? RBRACKET
+ : LBRACKET tryParse? field (LBRACKET params=multipleExpressions 
+ RBRACKET)? RBRACKET
  ;
 
 dataIoDefinition
@@ -49,6 +49,7 @@
  | discriminatorField
  | enumField
  | implicitField
+ | assertField
  | manualArrayField
  | manualField
  | optionalField
@@ -73,7 +74,7 @@
  ;
 
 constField
- : 'const' type=dataType name=idExpression expected=expression
+ : 'const' type=typeReference name=idExpression expected=expression
  ;
 
 discriminatorField
@@ -88,6 +89,10 @@
  : 'implicit' type=dataType name=idExpression serializeExpression=expression
  ;
 
+assertField
+ : 'assert' type=typeReference name=idExpression condition=expression  
+;
+
 manualArrayField
  : 'manualArray' type=typeReference name=idExpression loopType=ARRAY_LOOP_TYPE loopExpression=expression parseExpression=expression serializeExpression=expression lengthExpression=expression
  ;
@@ -129,7 +134,7 @@
  ;
 
 typeReference
- : complexTypeReference=IDENTIFIER_LITERAL
+ : complexTypeReference=IDENTIFIER_LITERAL (LBRACKET params=multipleExpressions RBRACKET)?
  | simpleTypeReference=dataType
  ;
 
@@ -150,6 +155,10 @@
  | base='dateTime'
  ;
 
+tryParse
+ : 'try'
+ ;
+
 argument
  : type=typeReference name=idExpression
  ;