Uniform Time Sampling in XsenseDataReader and ADPMDataReader#3977
Uniform Time Sampling in XsenseDataReader and ADPMDataReader#3977nickbianco merged 1 commit intoopensim-org:mainfrom
Conversation
|
@alexbeattie42, would the existing CommonUtilities::createVectorLinspace be sufficient here? |
|
@nickbianco Thanks for the suggestion! I updated the the code to use the std::vector<double> times(times_vec.getContiguousScalarData(),times_vec.getContiguousScalarData() + times_vec.size());I looked at modifying the |
|
I suppose we could add another linspace function to CommonUtilities that returns a |
dd91edd to
7002ba9
Compare
|
@nickbianco could you take another look? |
nickbianco
left a comment
There was a problem hiding this comment.
Nice work @alexbeattie42!
Just a few nitpicks to address.
Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @alexbeattie42)
OpenSim/Common/XsensDataReader.cpp line 165 at r3 (raw file):
} } const double timeIncrement = 1 / dataRate;
nit: use 1.0 to avoid implicit conversion:
Suggestion:
const double timeIncrement = 1.0 / dataRate;OpenSim/Common/APDMDataReader.cpp line 192 at r3 (raw file):
} // Trim Matrices in use to actual data and move into tables const double timeIncrement = 1 / dataRate;
nit: use 1.0 to avoid implicit conversion:
Suggestion:
const double timeIncrement = 1.0 / dataRate;OpenSim/Common/Test/testCommonUtilities.cpp line 2 at r3 (raw file):
/* --------------------------------------------------------------------------* * OpenSim: testCommonUtilities.cpp *
nit: misaligned comment box (this line and the ones below):
Suggestion:
/* --------------------------------------------------------------------------*
* OpenSim: testCommonUtilities.cpp *
alexbeattie42
left a comment
There was a problem hiding this comment.
@nickbianco Thanks! These are fixed now
Reviewable status: 1 of 4 files reviewed, all discussions resolved (waiting on @nickbianco)
abe56ec to
ea659cc
Compare
…r` type, add unit tests, and fix the related bug (opensim-org#3976) in IMU DataReaders
ea659cc to
544810c
Compare
nickbianco
left a comment
There was a problem hiding this comment.
Reviewed 5 of 5 files at r4, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @alexbeattie42)
|
Thanks @alexbeattie42! |
Fixes issue #3976
Brief summary of changes
Implementation of the method described in this issue
Testing I've completed
Tested locally. Will add tests
Looking for feedback on...
@aymanhab and @nickbianco
TableUtilities?CHANGELOG.md (choose one)
This change is