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
;
