search

openigtlink / OpenIGTLink

Free, open-source network communication library for image-guided therapy
http://openigtlink.org
BSD 3-Clause "New" or "Revised" License
103 stars 184 forks source link

Default header version member is still IGTL_HEADER_VERSION_1 #200

Open djromberg opened 5 years ago

djromberg commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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!