Fix issue with c3d marker labels more than 255

[mrrezaie]

Closing #3606

c3d.pointNames() reads all labels.

Testing I've completed

Worked in my test.

CHANGELOG.md (choose one)

• if accepted.

---

This change is 

[nickbianco]

Hi @mrrezaie, thanks for contributing a fix! If you have a working test, please consider adding it to testC3DFileAdapter.cpp:

opensim-core/OpenSim/Common/Test/testC3DFileAdapter.cpp

Line 2 in 232b7f9

* OpenSim: testC3DFileAdapter.cpp *

Additionally, based on discussion in #3606, it seems like some additional filtering of the point data returned by c3d.pointNames() will be necessary to ensure that only marker data is returned. The suggestion from @halleysfifthinc seems like a good one:

I mean, I wouldn't call it a workaround, it is a standards compliant extension of C3D capabilities. Regardless, my point was that if it is desired to remove non-marker point data, the the POINT:ANGLES, POINT:POWERS, POINT:FORCES, and POINT:MOMENTS parameters are arrays of strings that give the names of the point data (i.e. items in POINT:LABELS) that are not markers. That would be a simpler approach with fewer assumptions than hard-coding point names that are known™ to be non-markers.

[mrrezaie]

Hi, I just wanted to bring this to your attention. Unfortunately, I don't have a working example, and I don't know cpp.
I just modified the C3DFileAdapter.cpp file, built from the source, and tested in Python.

Filtering the marker labels sounds like a good idea, on the other hand, it's also great that users have access to every signal in the c3d. It might be a good idea to store the extra labels as table meta data. For example, adding a new meta data for each ANGLES, FORCES, MOMENTS, POWERS, MODELED_MARKERS, and SCALARS, like others:

c3dAdapter = osim.C3DFileAdapter()
tables  = c3dAdapter.read( 'path_to_c3d_file.c3d' )

markers = c3dAdapter.getMarkersTable(tables)
markers.getTableMetaDataKeys()
# ('DataRate', 'Units', 'events')

forces  = c3dAdapter.getForcesTable(tables)
forces.getTableMetaDataKeys()
# ('CalibrationMatrices', 'Corners', 'DataRate', 'Origins', 'Types', 'events')

They could be retrieved by one of the getTableMetaData variants. Then, it's up to the user to whether filter the labels or not.

[nickbianco]

@mrrezaie you've modified the source code, re-built, and tested the Python bindings -- I'd say you're well on your way to understanding C++! If this is really important to you, I would consider investing some time in implementing the changes that you want. This would likely go faster than waiting on one of the few contributors to OpenSim to take it on. I'm happy to review additional changes to the PR.

[mrrezaie]

Thanks @nickbianco, it's nice to hear that from you. I think I have to learn cpp ;)
In the meantime, at least there wouldn't be any error with this PR.

[nickbianco]

I think I have to learn cpp ;)

You should! There will never be a shortage of a need for C++ devs (until Rust takes over the world...).

In the meantime, at least there wouldn't be any error with this PR.

I still don't think we merge as-is, based on my comments above. That line is for exporting marker labels, and your change would include all point labels, including non-marker data label (if my understanding is correct). We still need changes to filter out the additional labels.