Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

References inside messageHeader not working #435

Closed
ksergey opened this issue Feb 15, 2017 · 16 comments
Closed

References inside messageHeader not working #435

ksergey opened this issue Feb 15, 2017 · 16 comments

Comments

@ksergey
Copy link
Contributor

ksergey commented Feb 15, 2017

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

mjpt777 added a commit that referenced this issue Feb 15, 2017
…ded at Token.referencedName() and use in JavaGenerator. Issue #435.
@mjpt777
Copy link
Contributor

mjpt777 commented Feb 15, 2017

@billsegall Worth tracking this one to see the changes that need to be applied to the Golang version.

@tmontgomery
Copy link
Contributor

C++ has been changed to follow Java (as of commit 323ece0). Might be good to check and see if that addresses the issue.

tmontgomery added a commit that referenced this issue Feb 15, 2017
…for Booster to catch issues like #435. Fixed includes and declarations use correct name.
@tmontgomery
Copy link
Contributor

now it should be working, I think.

@ksergey
Copy link
Contributor Author

ksergey commented Feb 16, 2017

@tmontgomery not fully resolved - MessageHeader.h do not include required headers.
Example above should include "UpdateFlags.h"

@tmontgomery
Copy link
Contributor

MessageHeader.h is only designed to include the MessageHeader type itself. It doesn't include any others.

@mjpt777
Copy link
Contributor

mjpt777 commented 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
Copy link
Contributor

tmontgomery commented 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
Copy link
Contributor

I believe this would effect the OTF as well, for example.

@mjpt777
Copy link
Contributor

mjpt777 commented 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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

mjpt777 commented Feb 16, 2017

I'm pretty certain I tried this today in the Java version and it worked.

@tmontgomery
Copy link
Contributor

tmontgomery commented 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
Copy link
Contributor

Should work now.

@ksergey
Copy link
Contributor Author

ksergey commented Feb 17, 2017

Resolved! Thanks

@ksergey ksergey closed this as completed Feb 17, 2017
tmontgomery added a commit that referenced this issue Feb 18, 2017
[golang] Issue #435 References inside messageHeader not working
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants