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 <[email protected]> 
Gesendet: Freitag, 24. September 2021 23:53
An: [email protected]
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
  ;

Reply via email to