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

Default header version member is still IGTL_HEADER_VERSION_1 #200

Open
djromberg opened this issue Nov 16, 2018 · 3 comments
Open

Default header version member is still IGTL_HEADER_VERSION_1 #200

djromberg opened this issue Nov 16, 2018 · 3 comments

Comments

@djromberg
Copy link

djromberg commented Nov 16, 2018

I am using the new meta data system that is "activated" via compiler switch ("#ifdef IGTL_HEADER_VERSION >= 2") by default when building the current version. However, when doing the Pack(), "send-pack", InitBuffer(), "read-header", AllocateBuffer(), "read-rest" stuff, I realized that my meta data is missing. It took me quite a while to find out that the default value for the m_HeaderVersion member is still set to IGTL_HEADER_VERSION_1 which lets the Packing omit the meta data. When setting it to IGTL_HEADER_VERSION_2 after message creation, things work as expected.
This issue is rather meant for clarification than as bug report. I would like to know whether this was just forgotten to update or whether it would be a backwards-compatibility problem when messages with the new version are received by a system that still uses the old protocol (and structures) and therefore intended to use it explicitly if wanted. If so, however, I think the SetMetaDataElement methods should somehow fail or raise an exception in order to make it clear that the data won't make it into the buffer.

@adamrankin
Copy link
Contributor

adamrankin commented Nov 16, 2018

It is default to 1 for backwards compatibility.

When you create a message, it is up to the application to set the desired header to 2. See igtl message factory CreateSendMessage function (or similar, sorry I'm on mobile)

@adamrankin
Copy link
Contributor

SetMetaDataElement failing/warning on v1 message is a good idea.

I think a warning is better, as you can set it to v2 after adding metadata and it should work.

@djromberg
Copy link
Author

I noticed an access violation when creating an igtl::ImageMessage, filling it with content then switching the header version to 2 in order to use the metadata system. After these steps a call to Pack() results in an access violation. I guess one is not allowed to use SetHeaderVersion after the message has been modified? I wonder whether this can be avoided, meaning that the client code doesn't have to think so much about the order of operations. Sure - the message factory is already a good helper!

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

2 participants