Skip to content

Fix 6D Settings Parsing for Versions Below 1.21 - #130

Merged
OliverGlandberger merged 2 commits into
masterfrom
fix_6dof_labels_bug
Sep 15, 2026
Merged

OliverGlandberger merged 2 commits into
masterfrom
fix_6dof_labels_bug

Conversation

@OliverGlandberger

@OliverGlandberger OliverGlandberger commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Read6DOFSettings always failed against the pre-1.21 XML format, so clients pinned below 1.21 (e.g. QTM Connect for MATLAB at 1.17) got no 6DOF bodies. Regression from the tinyxml2 rewrite (#109).

Problem

In the legacy branch of Deserialize6DOFSettings:

  • Bodies and Body were looked up on the document root instead of inside The_6D (returned false immediately)
  • TryReadSetPointsOld iterated Bone elements (previously Point)
  • TryReadSetEuler ran once per body and treated a missing <Euler> as an error
  • dataAvailable was only set when at least one body was found

Solution

  • Update the lookups to use The_6D, read Point elements, read Euler once after the body loop and treat it as optional (complete or absent)
  • Set dataAvailable for the element rather than per body
  • Read6DOFSettings now sets an error string naming the protocol version on parse failure

Tests Run

  • Ran full test suite
  • Verified end to end against QTM with QTM Connect for MATLAB at 1.17

@OliverGlandberger

Copy link
Copy Markdown
Contributor Author

@Capelliexp I was hesitant to add the tests because I think they are likely overkill. I kept them for the initial draft, to give you a chance to weigh in.

@OliverGlandberger

Copy link
Copy Markdown
Contributor Author

Spoke with @Capelliexp and we're ignoring the windows build failure caused by doctest and is unrelated to this change.

@OliverGlandberger
OliverGlandberger merged commit ba8adba into master Sep 15, 2026
7 of 10 checks passed
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

Successfully merging this pull request may close these issues.

2 participants