search

openigtlink / OpenIGTLink

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

ENH: Add parameter to make CRC calculation optional for header v2+ messages #232

Closed adamrankin closed 3 years ago

leochan2009 commented 3 years ago

The tests failed due to additional bytes added to meta data. need to update the tests due to the new meta data format. Also in the UnpackBody function, we need to use the meta information to decide if the crc calculation is correct.

adamrankin commented 3 years ago

So, metadata is packed into the body, and unpacked as well. The optional crc calculation needs information before it unpacks.

One way I was thinking of accomplishing this was to take a bit from the uint16 header_version to use as a crc valid flag instead of meta data. This would require incrementing the protocol version/header version. Thoughts?

adamrankin commented 3 years ago

So, metadata is packed into the body, and unpacked as well. The optional crc calculation needs information before it unpacks.

One way I was thinking of accomplishing this was to take a bit from the uint16 header_version to use as a crc valid flag instead of meta data. This would require incrementing the protocol version/header version. Thoughts?

Basically, an optional CRC check flag would have to be passed in the original 58 byte header, which I don't think is possible without a non-backwards compatible change. If we're interested in having a non-backwards compatible change, I would suggest revamping the header completely to support a number of new features.

leochan2009 commented 3 years ago

@adamrankin , yes, back-ward compatibility is not possible with the CRC option. i agree with you that a new header including new features would be a better solution. let's discuss this among the developers.