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 <l...@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 > ; >
