-
Notifications
You must be signed in to change notification settings - Fork 328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Undo binding of model getCoordinatesInMultibodyTreeOrder and introduc… #3666
Conversation
#3656) * Undo binding of model getCoordinatesInMultibodyTreeOrder and introduce getCoordinateNamesInMultibodyTreeOrder for scripting users * Fix dereferencing the ReferencePtr and exercise in test case * Fix compilation warnings/errors on linux and call site to index the SimTKArray * Fix int/size_t mixup that causes compilation errors on linux * Update CHANGELOG.md Use updated function name * Address feedback on PR in test, changelog language
* Add missing types * Add tests to check BufferedOrientationReference * Update CHANGELOG * Add test case for appending data to oRefs Co-authored-by: Zach Strout <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No biggies, just suggesions/comments
@@ -79,19 +79,19 @@ class OSIMSIMULATION_API BufferedOrientationsReference | |||
SimTK::Array_<SimTK::Rotation_<double>>& values) const override; | |||
|
|||
/** add passed in values to data procesing Queue */ | |||
void putValues(double time, const SimTK::RowVector_<SimTK::Rotation>& dataRow); | |||
void putValues(double time, const SimTK::RowVector_<SimTK::Rotation_<double>>& dataRow); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't SimTK::Rotation
an alias for SimTK::Rotation_<SimTK::Real>
, with SimTK::Real
usually being an alias for double
(i.e. the before and after are equivalent for OpenSim)?
@@ -943,6 +943,14 @@ OpenSim_OBJECT_NONTEMPLATE_DEFS(Model, ModelComponent); | |||
std::vector<SimTK::ReferencePtr<const OpenSim::Coordinate>> | |||
getCoordinatesInMultibodyTreeOrder() const; | |||
|
|||
/** A variant of getCoordinatesInMultibodyTreeOrder that returns names for Scripting users */ | |||
SimTK::Array_<std::string> getCoordinateNamesInMultibodyTreeOrder() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imo, should be in .cpp
file - there's no upside to inlining it (it's not templated, and it's doing a fairly large amount of work)
/** A variant of getCoordinatesInMultibodyTreeOrder that returns names for Scripting users */ | ||
SimTK::Array_<std::string> getCoordinateNamesInMultibodyTreeOrder() { | ||
SimTK::Array_<std::string> namesArray; | ||
auto coords = getCoordinatesInMultibodyTreeOrder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coords
is a std::vector<T>
, so this can be rewritten as:
const auto coords = getCoordinatesInMultibodyTreeOrder();
SimTK::Array_<std::string> namesArray;
namesArray.reserve(coords.size()); // prevent a reallocation
for (const auto& coord : coords) {
namesArray.push_back(coord.getName());
}
@aymanhab, are the changes here included in the OpenSim 4.5 release? If so, we should get them merged in soon. |
Yes, changes were made in 4.5 and backported to main so closing. Thanks @nickbianco |
…… (#3656)
Undo binding of model getCoordinatesInMultibodyTreeOrder and introduce getCoordinateNamesInMultibodyTreeOrder for scripting users
Fix dereferencing the ReferencePtr and exercise in test case
Fix compilation warnings/errors on linux and call site to index the SimTKArray
Fix int/size_t mixup that causes compilation errors on linux
Update CHANGELOG.md
Use updated function name
Fixes issue #<issue_number>
Brief summary of changes
Testing I've completed
Looking for feedback on...
CHANGELOG.md (choose one)
This change is