Repository navigation
References inside messageHeader not working #435
Description
Hello Devs,
Example:
<set name="UpdateFlags" encodingType="uint8">
<choice name="Begin">0</choice>
<choice name="End">1</choice>
</set>
<composite name="messageHeader" description="Template ID and length of message root">
<type name="blockLength" primitiveType="uint16"/>
<type name="templateId" primitiveType="uint16"/>
<type name="schemaId" primitiveType="uint16"/>
<type name="version" primitiveType="uint16"/>
<ref name="flags" type="UpdateFlags"/>
</composite>
Generated C++ code looks like:
private:
Flags m_flags;
public:
Flags &flags()
{
m_flags.wrap(m_buffer, m_offset + 8, m_actingVersion, m_bufferLength);
return m_flags;
}
But class Flags not generated and I believe class name should be UpdateFlags (not Flags)
Thanks
Activity
[Java] ref types did not carry through there type into the IR. Now ad…
mjpt777 commentedon Feb 15, 2017
@billsegall Worth tracking this one to see the changes that need to be applied to the Golang version.
tmontgomery commentedon Feb 15, 2017
C++ has been changed to follow Java (as of commit 323ece0). Might be good to check and see if that addresses the issue.
[C++]: changed code-generation test schema to use a different string …
tmontgomery commentedon Feb 15, 2017
now it should be working, I think.
ksergey commentedon Feb 16, 2017
@tmontgomery not fully resolved - MessageHeader.h do not include required headers.
Example above should include "UpdateFlags.h"
[c++] Generate correct type for enum lookup method. Issue #435.
[Java] Generate MessageHeader as composite rather than just encoded p…
tmontgomery commentedon Feb 16, 2017
MessageHeader.h is only designed to include the MessageHeader type itself. It doesn't include any others.
mjpt777 commentedon Feb 16, 2017
@tmontgomery I think a message header should support the minimum four fields and it is OK to add on additional fields at the end. The Java implementation now allows this. The spec does not say it must only be these four fields.
tmontgomery commentedon Feb 16, 2017
@mjpt777 true. The spec does not forbid it. But the IR HeaderStructure class only picks up those 4 token names. So, the IR itself doesn't contain anything else for the header structure.
tmontgomery commentedon Feb 16, 2017
I believe this would effect the OTF as well, for example.
[C++]: added includes for messageHader. #435
tmontgomery commentedon Feb 16, 2017
i.e. https://github.com/real-logic/simple-binary-encoding/blob/master/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/HeaderStructure.java#L52
mjpt777 commentedon Feb 16, 2017
There is a default header with a minimum set of requirements. An OTF implementation could use its own extended header decoder.
tmontgomery commentedon Feb 16, 2017
It could, yes. But the header structure that is used for generating the codec MessageHeader seems to ignore all other fields than the named ones. And that header structure is also used for OTF, IIRC.
tmontgomery commentedon Feb 16, 2017
It's in the Ir class. Which creates the HeaderStructure which when capturing tokens, seems to ignore everything but the named fields. So, it would effect Java OTF and the generators.
For C++, I tried looking at this. A ref won't be seen in the generated MessageHeader because it isn't in the header structure at that point. Or it seems that way.
mjpt777 commentedon Feb 16, 2017
I'm pretty certain I tried this today in the Java version and it worked.
tmontgomery commentedon Feb 16, 2017
Actually, I think it is more subtle.
Update: 2 wrongs don't make a right. Looking in the wrong place and looking at wrong methods.
tmontgomery commentedon Feb 16, 2017
Should work now.
ksergey commentedon Feb 17, 2017
Resolved! Thanks
Merge pull request #439 from MarketFactory/golang-issue-435