simple-binary-encoding icon indicating copy to clipboard operation
simple-binary-encoding copied to clipboard

Published 20 hours ago • verified publisher icon real-logic

Adding support for the package attribute on the types element.

Open ratcashdev opened this issue 1 year ago • 6 comments

Implementation of #507 for Java.

The main idea is that we store the package attribute read from the XML to the IR, into token[0]. Then it's up to the generator to make use of that extra piece of information (controllable through sbe.type.package.override argument, defaulting to false ).

The serialization of the IR is not touched, therefore the package information is not present in the serialized IR.

Some classes in this PR, like the MultiPackageOutputManager and StringWriterOutputManager actually belong to agrona, but have been implemented here to have a testable code. Before merging, those files/classes should be moved and substituted with a new release of agrona.

ratcashdev avatar Jul 05 '22 20:07 ratcashdev

@mjpt777 @tmontgomery Hi, subject to Your time permitting, I'd appreciate a 1st, rough review of this. Thanks.

ratcashdev avatar Jul 27 '22 10:07 ratcashdev

Sorry but we have been busy. We need to give priority to our commercial support customers.

mjpt777 avatar Jul 27 '22 12:07 mjpt777

Sure, take your time. Just wanted to make sure this blipped on your radar.

ratcashdev avatar Aug 08 '22 12:08 ratcashdev

Created https://github.com/real-logic/agrona/pull/266 for the required changes in agrona.

ratcashdev avatar Sep 05 '22 09:09 ratcashdev

@ratcashdev Agrona 1.17.0 was released and it contains https://github.com/real-logic/agrona/pull/266.

vyazelenko avatar Sep 14 '22 16:09 vyazelenko

updated to latest master.

ratcashdev avatar Sep 14 '22 18:09 ratcashdev

Ok, will enhance the tests as a separate PR. Thanks for the fix.

ratcashdev avatar Sep 28 '22 08:09 ratcashdev

just realized, the case below also needs to be taken care of or at least tested (REF pointing to a type in a different package):

 <types package="specific.package.name.A">
       <enum name="booleanEnum" encodingType="uint8" semanticType="Boolean">
            <validValue name="F">0</validValue>
            <validValue name="T">1</validValue>
        </enum>
</types>

 <types package="specific.package.name.B">
       <composite name="futuresPrice">
            <type name="mantissa" primitiveType="int64" />
            <type name="exponent" primitiveType="int8" />
            <ref name="isSettlement" type="booleanEnum" offset="10" />
        </composite>
</types>

ratcashdev avatar Sep 28 '22 11:09 ratcashdev

Yes the <ref> case needs to be considered.

mjpt777 avatar Sep 29 '22 12:09 mjpt777

When do you think you can address this?

mjpt777 avatar Oct 03 '22 10:10 mjpt777

Apologies, got sick for the past week, still on meds. Fingers cross - this week.

ratcashdev avatar Oct 03 '22 10:10 ratcashdev

please have a look on https://github.com/real-logic/simple-binary-encoding/pull/915

ratcashdev avatar Oct 03 '22 13:10 ratcashdev