From e565abfc88a1c3c3031cae9fc0866924460b687d Mon Sep 17 00:00:00 2001 From: Zach Strout <43109787+RTnhN@users.noreply.github.com> Date: Tue, 12 Dec 2023 12:33:08 -0600 Subject: [PATCH 1/5] Fix types for methods in BufferedOrientationReference (#3644) * Add missing types * Add tests to check BufferedOrientationReference * Update CHANGELOG * Add test case for appending data to oRefs --- Bindings/Python/tests/test_opensense.py | 20 +++++++++++++++++++ CHANGELOG.md | 1 + .../BufferedOrientationsReference.h | 8 ++++---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Bindings/Python/tests/test_opensense.py b/Bindings/Python/tests/test_opensense.py index ec92ebaac7..d5452fca16 100644 --- a/Bindings/Python/tests/test_opensense.py +++ b/Bindings/Python/tests/test_opensense.py @@ -33,6 +33,10 @@ def test_createObjects(self): constraint_var = .0001 ikSolver = osim.InverseKinematicsSolver(model, mRefs, oRefs, coordinateReferences, constraint_var) print("Created InverseKinematicsSolver object..") + oRefs = osim.BufferedOrientationsReference() + print("Created BufferedOrientationsReference object..") + ikSolver = osim.InverseKinematicsSolver(model, mRefs, oRefs, coordinateReferences, constraint_var) + print("Created InverseKinematicsSolver object with BufferedOrientationsReference..") def test_vector_rowvector(self): print() @@ -43,3 +47,19 @@ def test_vector_rowvector(self): col[1] == row[1] and col[2] == row[2] and col[3] == row[3]) + + def test_BufferedOrientationReferencePut(self): + # Make sure that we can append new data to the BufferedOrientationReference object + quatTable = osim.TimeSeriesTableQuaternion(os.path.join(test_dir,'orientation_quats.sto')) + print("Created TimeSeriesTableQuaternion object..") + orientationsData = osim.OpenSenseUtilities.convertQuaternionsToRotations(quatTable) + print("Convert Quaternions to orientationsData") + time = 0 + rowVecView = orientationsData.getNearestRow(time) + print("Sliced orientationData") + rowVec = osim.RowVectorRotation(rowVecView) + print("Converted slice to row vector") + oRefs = osim.BufferedOrientationsReference() + print("Created BufferedOrienationReference object") + oRefs.putValues(time, rowVec) + print("Added row vector to BufferedOrientationReference object") diff --git a/CHANGELOG.md b/CHANGELOG.md index 95fbd56d1b..fb55200544 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ and `Blankevoort1991Ligament`, usages of `get_GeometryPath`, `upd_GeometryPath`, - Exposed simbody methods to obtain GravityForces, MobilityForces and BodyForces (#3490) - Simbody was updated such that the headers it transitively exposes to downstream projects are compatible with C++20 (#3619) - Moved speed computation from `computeForce` in children of `ScalarActuator` to dedicated `getSpeed` function. +- Fix type problem with BufferedOrientationReference (Issue #3415, PR #3644) v4.4.1 diff --git a/OpenSim/Simulation/BufferedOrientationsReference.h b/OpenSim/Simulation/BufferedOrientationsReference.h index e39009e177..a8e874fece 100644 --- a/OpenSim/Simulation/BufferedOrientationsReference.h +++ b/OpenSim/Simulation/BufferedOrientationsReference.h @@ -33,7 +33,7 @@ namespace OpenSim { //============================================================================= /** * Subclass of OrientationsReference that handles live data by providing a DataQueue - * that allows clients to push data into and allows the InverseKinematicsSolver to + * that allows clients to push data into and allows the InverseKinematicsSolver to * draw data from for solving. * Ideally this would be templatized, allowing for all Reference classes to leverage it. * @@ -79,19 +79,19 @@ class OSIMSIMULATION_API BufferedOrientationsReference SimTK::Array_>& values) const override; /** add passed in values to data procesing Queue */ - void putValues(double time, const SimTK::RowVector_& dataRow); + void putValues(double time, const SimTK::RowVector_>& dataRow); double getNextValuesAndTime( SimTK::Array_>& values) override; virtual bool hasNext() const override { return !_finished; }; - void setFinished(bool finished) { + void setFinished(bool finished) { _finished = finished; }; private: // Use a specialized data structure for holding the orientation data - mutable DataQueue_ _orientationDataQueue; + mutable DataQueue_> _orientationDataQueue; bool _finished{false}; //============================================================================= }; // END of class BufferedOrientationsReference From d17c20e336ffdd646f2b7a6fd7e711e12962fa0f Mon Sep 17 00:00:00 2001 From: Mohammadreza Rezaie Date: Wed, 13 Dec 2023 03:32:14 +0330 Subject: [PATCH 2/5] Fix a minor typo in test_DataTable.py (#3647) Fix typo --- Bindings/Python/tests/test_DataTable.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Bindings/Python/tests/test_DataTable.py b/Bindings/Python/tests/test_DataTable.py index 4ece2fa8d5..8bbe9214bc 100644 --- a/Bindings/Python/tests/test_DataTable.py +++ b/Bindings/Python/tests/test_DataTable.py @@ -49,7 +49,7 @@ def test_vector_rowvector(self): assert (str(col[0]) == str(row[0]) and str(col[1]) == str(row[1]) and str(col[2]) == str(row[2])) - print('Test transpose VectorOVec3 to RowVectorVec3.') + print('Test transpose VectorVec3 to RowVectorVec3.') row_copy = col.transpose() assert (str(row_copy[0]) == str(row[0]) and str(row_copy[1]) == str(row[1]) and From 2754d05af265de2d732c2b349b53db0c4738b53f Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Wed, 13 Dec 2023 08:53:00 +0100 Subject: [PATCH 3/5] Change simbody to 589e931f --- dependencies/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies/CMakeLists.txt b/dependencies/CMakeLists.txt index a399e68a68..f904aada5b 100644 --- a/dependencies/CMakeLists.txt +++ b/dependencies/CMakeLists.txt @@ -183,7 +183,7 @@ AddDependency(NAME ezc3d AddDependency(NAME simbody DEFAULT ON GIT_URL https://github.com/simbody/simbody.git - GIT_TAG 930ae0feff0adb5aec184af62d14f9d138cacd48 + GIT_TAG 589e931f4a127830a5876ee4dd8f327b47361504 CMAKE_ARGS -DBUILD_EXAMPLES:BOOL=OFF -DBUILD_TESTING:BOOL=OFF ${SIMBODY_EXTRA_CMAKE_ARGS}) From ecdfba104965785077ca33ba4e1361943ae9b576 Mon Sep 17 00:00:00 2001 From: Nicholas Bianco Date: Tue, 19 Dec 2023 16:50:56 -0600 Subject: [PATCH 4/5] Add rule-of-five to IKMarkerTask and OrientationWeightSet (#3657) --- OpenSim/Tools/IKMarkerTask.h | 3 +++ OpenSim/Tools/IMUInverseKinematicsTool.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/OpenSim/Tools/IKMarkerTask.h b/OpenSim/Tools/IKMarkerTask.h index 3035434a5a..986745cf44 100644 --- a/OpenSim/Tools/IKMarkerTask.h +++ b/OpenSim/Tools/IKMarkerTask.h @@ -41,6 +41,9 @@ OpenSim_DECLARE_CONCRETE_OBJECT(IKMarkerTask, IKTask); public: IKMarkerTask() = default; IKMarkerTask(const IKMarkerTask&) = default; + IKMarkerTask(IKMarkerTask&&) = default; + IKMarkerTask& operator=(const IKMarkerTask&) = default; + IKMarkerTask& operator=(IKMarkerTask&&) = default; //============================================================================= }; // END of class IKMarkerTask //============================================================================= diff --git a/OpenSim/Tools/IMUInverseKinematicsTool.h b/OpenSim/Tools/IMUInverseKinematicsTool.h index d0aa2dbdc8..130fd3c666 100644 --- a/OpenSim/Tools/IMUInverseKinematicsTool.h +++ b/OpenSim/Tools/IMUInverseKinematicsTool.h @@ -58,6 +58,9 @@ class OSIMTOOLS_API OrientationWeightSet : public Set { // default copy, assignment operator, and destructor OrientationWeightSet() = default; OrientationWeightSet(const OrientationWeightSet&) = default; + OrientationWeightSet(OrientationWeightSet&&) = default; + OrientationWeightSet& operator=(const OrientationWeightSet&) = default; + OrientationWeightSet& operator=(OrientationWeightSet&&) = default; //============================================================================= }; //============================================================================= From 88da27ec855f6f017dbc62c736e3b959ea382ea5 Mon Sep 17 00:00:00 2001 From: Nicholas Bianco Date: Tue, 19 Dec 2023 18:04:34 -0600 Subject: [PATCH 5/5] Replace usages of `sprintf` with `snprintf` (#3658) * Replace usages of sprintf with snprintf * Add constexpr for _DoubleFormat length --- OpenSim/Analyses/PointKinematics.cpp | 4 ++-- OpenSim/Common/About.cpp | 2 +- OpenSim/Common/IO.cpp | 16 ++++++++++------ OpenSim/Common/IO.h | 5 +++-- OpenSim/Common/PropertyDbl.cpp | 2 +- OpenSim/Common/PropertyDblArray.cpp | 2 +- OpenSim/Common/PropertyInt.cpp | 2 +- OpenSim/Common/PropertyIntArray.cpp | 2 +- OpenSim/Common/StateVector.cpp | 4 ++-- OpenSim/Common/Storage.cpp | 2 +- OpenSim/Common/XMLDocument.cpp | 2 +- OpenSim/Simulation/Model/AbstractTool.cpp | 3 ++- 12 files changed, 26 insertions(+), 20 deletions(-) diff --git a/OpenSim/Analyses/PointKinematics.cpp b/OpenSim/Analyses/PointKinematics.cpp index e2864a73a9..511406835c 100644 --- a/OpenSim/Analyses/PointKinematics.cpp +++ b/OpenSim/Analyses/PointKinematics.cpp @@ -223,12 +223,12 @@ constructDescription() strcat(descrip,"(position, velocity, or acceleration) of\n"); if(_relativeToBody){ - sprintf(tmp,"point (%lf, %lf, %lf) on body %s relative to body %s of model %s.\n", + snprintf(tmp, BUFFER_LENGTH, "point (%lf, %lf, %lf) on body %s relative to body %s of model %s.\n", _point[0],_point[1],_point[2],_body->getName().c_str(), _relativeToBody->getName().c_str(), _model->getName().c_str()); } else{ - sprintf(tmp,"point (%lf, %lf, %lf) on the %s of model %s.\n", + snprintf(tmp, BUFFER_LENGTH, "point (%lf, %lf, %lf) on the %s of model %s.\n", _point[0],_point[1],_point[2],_body->getName().c_str(), _model->getName().c_str()); } diff --git a/OpenSim/Common/About.cpp b/OpenSim/Common/About.cpp index ec3dfe8df7..91594d5581 100644 --- a/OpenSim/Common/About.cpp +++ b/OpenSim/Common/About.cpp @@ -128,7 +128,7 @@ static const char* OpenSimVersion = GET_OSIM_VERSION; std::string GetVersionAndDate() { char buffer[256]; - sprintf(buffer,"version %s, build date %s %s", + snprintf(buffer, 256, "version %s, build date %s %s", OpenSimVersion, __TIME__, __DATE__); return std::string(buffer); } diff --git a/OpenSim/Common/IO.cpp b/OpenSim/Common/IO.cpp index ca4ad37429..66828b49b1 100644 --- a/OpenSim/Common/IO.cpp +++ b/OpenSim/Common/IO.cpp @@ -90,7 +90,7 @@ ConstructDateAndTimeStamp() // CONSTRUCT STAMP char *stamp = new char[64]; - sprintf(stamp,"%d%02d%02d_%02d%02d%02d", + snprintf(stamp, 64, "%d%02d%02d_%02d%02d%02d", timeStruct->tm_year+1900,timeStruct->tm_mon+1,timeStruct->tm_mday, timeStruct->tm_hour,timeStruct->tm_min,timeStruct->tm_sec); @@ -282,18 +282,22 @@ void IO:: ConstructDoubleOutputFormat() { if(_GFormatForDoubleOutput) { - sprintf(_DoubleFormat,"%%g"); + snprintf(_DoubleFormat, IO_DBLFMTLEN, "%%g"); } else if(_Scientific) { if(_Pad<0) { - sprintf(_DoubleFormat,"%%.%dle",_Precision); + snprintf(_DoubleFormat, IO_DBLFMTLEN, + "%%.%dle", _Precision); } else { - sprintf(_DoubleFormat,"%%%d.%dle",_Pad+_Precision,_Precision); + snprintf(_DoubleFormat, IO_DBLFMTLEN, + "%%%d.%dle", _Pad+_Precision, _Precision); } } else { if(_Pad<0) { - sprintf(_DoubleFormat,"%%.%dlf",_Precision); + snprintf(_DoubleFormat, IO_DBLFMTLEN, + "%%.%dlf", _Precision); } else { - sprintf(_DoubleFormat,"%%%d.%dlf",_Pad+_Precision,_Precision); + snprintf(_DoubleFormat, IO_DBLFMTLEN, + "%%%d.%dlf", _Pad+_Precision, _Precision); } } } diff --git a/OpenSim/Common/IO.h b/OpenSim/Common/IO.h index 6954a7b250..2b403b959d 100644 --- a/OpenSim/Common/IO.h +++ b/OpenSim/Common/IO.h @@ -34,7 +34,8 @@ #include // DEFINES -const int IO_STRLEN = 2048; +constexpr int IO_STRLEN = 2048; +constexpr int IO_DBLFMTLEN = 256; namespace OpenSim { @@ -62,7 +63,7 @@ class OSIMCOMMON_API IO { /** Specifies the precision of number output. */ static int _Precision; /** The output format string. */ - static char _DoubleFormat[256]; + static char _DoubleFormat[IO_DBLFMTLEN]; /** Whether offline documents should also be printed when Object::print is called. */ static bool _PrintOfflineDocuments; diff --git a/OpenSim/Common/PropertyDbl.cpp b/OpenSim/Common/PropertyDbl.cpp index 44c84f473d..0a31f98f22 100644 --- a/OpenSim/Common/PropertyDbl.cpp +++ b/OpenSim/Common/PropertyDbl.cpp @@ -187,7 +187,7 @@ toString() const { if (SimTK::isFinite(_value)) { char dbl[256]; - sprintf(dbl, "%g", _value); + snprintf(dbl, 256, "%g", _value); return dbl; } diff --git a/OpenSim/Common/PropertyDblArray.cpp b/OpenSim/Common/PropertyDblArray.cpp index 4f840ab3b4..fbe78015dc 100644 --- a/OpenSim/Common/PropertyDblArray.cpp +++ b/OpenSim/Common/PropertyDblArray.cpp @@ -216,7 +216,7 @@ toString() const string str = "("; char dbl[256]; for(int i=0; i < _array.getSize(); i++){ - sprintf(dbl, "%g", _array[i]); + snprintf(dbl, 256, "%g", _array[i]); str += (i>0?" ":"") + string(dbl); } str += ")"; diff --git a/OpenSim/Common/PropertyInt.cpp b/OpenSim/Common/PropertyInt.cpp index 140e67e18c..2ae6ecbc9c 100644 --- a/OpenSim/Common/PropertyInt.cpp +++ b/OpenSim/Common/PropertyInt.cpp @@ -185,6 +185,6 @@ string PropertyInt:: toString() const { char intString[32]; - sprintf(intString, "%d", _value); + snprintf(intString, 32, "%d", _value); return intString; } diff --git a/OpenSim/Common/PropertyIntArray.cpp b/OpenSim/Common/PropertyIntArray.cpp index 99e8d64ccb..6c7f8918f4 100644 --- a/OpenSim/Common/PropertyIntArray.cpp +++ b/OpenSim/Common/PropertyIntArray.cpp @@ -214,7 +214,7 @@ toString() const string str = "("; char intString[256]; for(int i=0; i < _array.getSize(); i++){ - sprintf(intString, "%d", _array[i]); + snprintf(intString, 256, "%d", _array[i]); str += (i>0?" ":"") + string(intString); } str += ")"; diff --git a/OpenSim/Common/StateVector.cpp b/OpenSim/Common/StateVector.cpp index 3ef3c0816c..e62ec5a3d0 100644 --- a/OpenSim/Common/StateVector.cpp +++ b/OpenSim/Common/StateVector.cpp @@ -543,7 +543,7 @@ print(FILE *fp) const // TIME char format[IO_STRLEN]; - sprintf(format,"%s",IO::GetDoubleOutputFormat()); + snprintf(format, IO_STRLEN, "%s", IO::GetDoubleOutputFormat()); int n=0,nTotal=0; n = fprintf(fp,format,_t); if(n<0) { @@ -553,7 +553,7 @@ print(FILE *fp) const nTotal += n; // STATES - sprintf(format,"\t%s",IO::GetDoubleOutputFormat()); + snprintf(format, IO_STRLEN, "\t%s", IO::GetDoubleOutputFormat()); for(int i=0;i<_data.getSize();i++) { n = fprintf(fp,format,_data[i]); if(n<0) { diff --git a/OpenSim/Common/Storage.cpp b/OpenSim/Common/Storage.cpp index 798f0da652..039d96d34b 100644 --- a/OpenSim/Common/Storage.cpp +++ b/OpenSim/Common/Storage.cpp @@ -3404,7 +3404,7 @@ bool Storage::makeStorageLabelsUnique() { int c = 1; while (exist) { char cString[20]; - sprintf(cString,"%d", c); + snprintf(cString, 20, "%d", c); newName = std::string(cString) + "_" + offending; exist = (lbls.findIndex(newName) != -1); c++; diff --git a/OpenSim/Common/XMLDocument.cpp b/OpenSim/Common/XMLDocument.cpp index 9bee9a7c4e..3514bb3dda 100644 --- a/OpenSim/Common/XMLDocument.cpp +++ b/OpenSim/Common/XMLDocument.cpp @@ -220,7 +220,7 @@ getVersionAsString(const int aVersion, std::string& aString) for(int i=0; i<3; i++) { int digits = ver / div; - sprintf(pad, "%02d",digits); + snprintf(pad, 3, "%02d", digits); ver -= div*(ver / div); div /=100; aString += string(pad); diff --git a/OpenSim/Simulation/Model/AbstractTool.cpp b/OpenSim/Simulation/Model/AbstractTool.cpp index ff7046edd7..dcac598069 100644 --- a/OpenSim/Simulation/Model/AbstractTool.cpp +++ b/OpenSim/Simulation/Model/AbstractTool.cpp @@ -822,7 +822,8 @@ std::string AbstractTool::createExternalLoadsFile(const std::string& oldFile, ExternalForce* xf = new ExternalForce(); xf->setAppliedToBodyName((f==0)?body1:body2); char pad[3]; - sprintf(pad,"%d", f+1); + snprintf(pad, 3, "%d", f+1); + std::string suffix = "ExternalForce_"+string(pad); xf->setName(suffix); _externalLoads.adoptAndAppend(xf);