Skip to content

Commit

Permalink
Merge pull request #3927 from opensim-org/fix_external-loads-should-k…
Browse files Browse the repository at this point in the history 
…eep-xmldoc

Fix external loads should keep xmldoc
  • Loading branch information
@adamkewley
adamkewley authored Oct 4, 2024
2 parents f874170 + 57dd758 commit b89164d
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 15 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ v4.6
- Made improvements to `MocoUtilities::createExternalLoadsTableForGait()`: center of pressure values are now set to zero, rather
than NaN, when vertical force is zero, and the vertical torque is returned in the torque columns (rather than the sum of the
sphere torques) to be consistent with the center of pressure GRF representation.
- Fixed an issue where a copy of an `OpenSim::Model` containing a `OpenSim::ExternalLoads` could not be
finalized (#3926)

v4.5.1
======
Expand Down
6 changes: 6 additions & 0 deletions OpenSim/Simulation/Model/ExternalLoads.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ ExternalLoads::ExternalLoads(const ExternalLoads &otherExternalLoads) :
ModelComponentSet<ExternalForce>(otherExternalLoads),
_dataFileName(_dataFileNameProp.getValueStr())
{
// copy the document over, because it's used during `extendFinalizeConnections`
// to figure out where the associated motion file (#3926)
if (auto* document = otherExternalLoads.getDocument()) {
setDocument(std::make_unique<XMLDocument>(*document).release());
}

setNull();

// Class Members
Expand Down
5 changes: 5 additions & 0 deletions OpenSim/Tools/Test/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@ OpenSimAddTests(
DATAFILES ${TEST_FILES}
LINKLIBS osimTools Catch2::Catch2WithMain
)


# edge-case: one test ensures that `ExternalLoads` still works - even when it isn't in
# the working directory of the test runner (#3926)
file(COPY "ExternalLoadsInSubdir" DESTINATION "${CMAKE_CURRENT_BINARY_DIR}")
27 changes: 27 additions & 0 deletions OpenSim/Tools/Test/ExternalLoadsInSubdir/external-loads-in-subdir.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="UTF-8" ?>
<OpenSimDocument Version="30000">
<ExternalLoads name="PendulumExternalLoads">
<objects>
<ExternalForce name="single">
<!--Flag indicating whether the force is disabled or not. Disabled means that the force is not active in subsequent dynamics realizations.-->
<isDisabled>false</isDisabled>
<!--Name of the body the force is applied to.-->
<applied_to_body>new_body</applied_to_body>
<!--Name of the body the force is expressed in (default is ground).-->
<force_expressed_in_body>new_body</force_expressed_in_body>
<!--Name of the body the point is expressed in (default is ground).-->
<point_expressed_in_body>new_body</point_expressed_in_body>
<!--Identifier (string) to locate the force to be applied in the data source.-->
<force_identifier>head_v</force_identifier>
<!--Identifier (string) to locate the point to be applied in the data source.-->
<point_identifier>head_p</point_identifier>
<!--Identifier (string) to locate the torque to be applied in the data source.-->
<!--Name of the data source (Storage) that will supply the force data.-->
<data_source_name>Unassigned</data_source_name>
</ExternalForce>
</objects>
<groups />
<!--Storage file (.sto) containing (3) components of force and/or torque and point of application.Note: this file overrides the data source specified by the individual external forces if specified.-->
<datafile>forces-in-subdir.mot</datafile>
</ExternalLoads>
</OpenSimDocument>
9 changes: 9 additions & 0 deletions OpenSim/Tools/Test/ExternalLoadsInSubdir/forces-in-subdir.mot
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
pendulum_swing
version=1
nRows=2
nColumns=7
inDegrees=yes
endheader
time head_vx head_vy head_vz head_px head_py head_pz
0 0 0 0 0 0 0
1 1000 1000 1000 0 0 0
115 changes: 115 additions & 0 deletions OpenSim/Tools/Test/ExternalLoadsInSubdir/model-in-subdir.osim
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<?xml version="1.0" encoding="UTF-8" ?>
<OpenSimDocument Version="40600">
<Model name="model">
<!--The model's ground reference frame.-->
<Ground name="ground">
<!--The geometry used to display the axes of this Frame.-->
<FrameGeometry name="frame_geometry">
<!--Path to a Component that satisfies the Socket 'frame' of type Frame.-->
<socket_frame>..</socket_frame>
<!--Scale factors in X, Y, Z directions respectively.-->
<scale_factors>0.20000000000000001 0.20000000000000001 0.20000000000000001</scale_factors>
</FrameGeometry>
</Ground>
<!--List of bodies that make up this model.-->
<BodySet name="bodyset">
<objects>
<Body name="new_body">
<!--The geometry used to display the axes of this Frame.-->
<FrameGeometry name="frame_geometry">
<!--Path to a Component that satisfies the Socket 'frame' of type Frame.-->
<socket_frame>..</socket_frame>
<!--Scale factors in X, Y, Z directions respectively.-->
<scale_factors>0.20000000000000001 0.20000000000000001 0.20000000000000001</scale_factors>
</FrameGeometry>
<!--List of geometry attached to this Frame. Note, the geometry are treated as fixed to the frame and they share the transform of the frame when visualized-->
<attached_geometry>
<Sphere name="new_body_geom_1">
<!--Path to a Component that satisfies the Socket 'frame' of type Frame.-->
<socket_frame>..</socket_frame>
<!--Radius of sphere, defaults to 1.0-->
<radius>0.10000000000000001</radius>
</Sphere>
</attached_geometry>
<!--The mass of the body (kg)-->
<mass>1</mass>
<!--The location (Vec3) of the mass center in the body frame.-->
<mass_center>0 0 0</mass_center>
<!--The elements of the inertia tensor (Vec6) as [Ixx Iyy Izz Ixy Ixz Iyz] measured about the mass_center and not the body origin.-->
<inertia>1 1 1 0 0 0</inertia>
</Body>
</objects>
<groups />
</BodySet>
<!--List of joints that connect the bodies.-->
<JointSet name="jointset">
<objects>
<PinJoint name="pinjoint">
<!--Path to a Component that satisfies the Socket 'parent_frame' of type PhysicalFrame (description: The parent frame for the joint.).-->
<socket_parent_frame>ground_offset</socket_parent_frame>
<!--Path to a Component that satisfies the Socket 'child_frame' of type PhysicalFrame (description: The child frame for the joint.).-->
<socket_child_frame>new_body_offset</socket_child_frame>
<!--List containing the generalized coordinates (q's) that parameterize this joint.-->
<coordinates>
<Coordinate name="rz">
<!--The value of this coordinate before any value has been set. Rotational coordinate value is in radians and Translational in meters.-->
<default_value>1.5707963705062866</default_value>
</Coordinate>
</coordinates>
<!--Physical offset frames owned by the Joint that are typically used to satisfy the owning Joint's parent and child frame connections (sockets). PhysicalOffsetFrames are often used to describe the fixed transformation from a Body's origin to another location of interest on the Body (e.g., the joint center). When the joint is deleted, so are the PhysicalOffsetFrame components in this list.-->
<frames>
<PhysicalOffsetFrame name="ground_offset">
<!--The geometry used to display the axes of this Frame.-->
<FrameGeometry name="frame_geometry">
<!--Path to a Component that satisfies the Socket 'frame' of type Frame.-->
<socket_frame>..</socket_frame>
<!--Scale factors in X, Y, Z directions respectively.-->
<scale_factors>0.20000000000000001 0.20000000000000001 0.20000000000000001</scale_factors>
</FrameGeometry>
<!--Path to a Component that satisfies the Socket 'parent' of type C (description: The parent frame to this frame.).-->
<socket_parent>/ground</socket_parent>
<!--Translational offset (in meters) of this frame's origin from the parent frame's origin, expressed in the parent frame.-->
<translation>0 1 0</translation>
<!--Orientation offset (in radians) of this frame in its parent frame, expressed as a frame-fixed x-y-z rotation sequence.-->
<orientation>-0 0 -0</orientation>
</PhysicalOffsetFrame>
<PhysicalOffsetFrame name="new_body_offset">
<!--The geometry used to display the axes of this Frame.-->
<FrameGeometry name="frame_geometry">
<!--Path to a Component that satisfies the Socket 'frame' of type Frame.-->
<socket_frame>..</socket_frame>
<!--Scale factors in X, Y, Z directions respectively.-->
<scale_factors>0.20000000000000001 0.20000000000000001 0.20000000000000001</scale_factors>
</FrameGeometry>
<!--Path to a Component that satisfies the Socket 'parent' of type C (description: The parent frame to this frame.).-->
<socket_parent>/bodyset/new_body</socket_parent>
<!--Translational offset (in meters) of this frame's origin from the parent frame's origin, expressed in the parent frame.-->
<translation>-0 0.46722877025604248 -0</translation>
<!--Orientation offset (in radians) of this frame in its parent frame, expressed as a frame-fixed x-y-z rotation sequence.-->
<orientation>-0 0 -0</orientation>
</PhysicalOffsetFrame>
</frames>
</PinJoint>
</objects>
<groups />
</JointSet>
<!--Controllers that provide the control inputs for Actuators.-->
<ControllerSet name="controllerset">
<objects />
<groups />
</ControllerSet>
<!--Forces in the model (includes Actuators).-->
<ForceSet name="forceset">
<objects />
<groups />
</ForceSet>
<!--Visual preferences for this model.-->
<ModelVisualPreferences name="modelvisualpreferences">
<!--Model display preferences-->
<ModelDisplayHints>
<!--Flag to indicate whether or not to show frames, default to false.-->
<show_frames>true</show_frames>
</ModelDisplayHints>
</ModelVisualPreferences>
</Model>
</OpenSimDocument>
37 changes: 22 additions & 15 deletions OpenSim/Tools/Test/testExternalLoads.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,15 @@
* limitations under the License. *
* -------------------------------------------------------------------------- */

#include <iostream>
#include <OpenSim/OpenSim.h>
#include <OpenSim/Auxiliary/auxiliaryTestFunctions.h>

using namespace OpenSim;
using namespace std;

void testExternalLoad();
void testExternalLoadDefaultProperties();
#include <catch2/catch_all.hpp>

int main()
{
SimTK_START_TEST("testExternalLoads");
SimTK_SUBTEST(testExternalLoad);
SimTK_SUBTEST(testExternalLoadDefaultProperties);
SimTK_END_TEST();
}
#include <iostream>

using namespace OpenSim;
using namespace std;

void addLoadToStorage(Storage &forceStore, SimTK::Vec3 force, SimTK::Vec3 point, SimTK::Vec3 torque)
{
Expand Down Expand Up @@ -85,7 +76,7 @@ void addLoadToStorage(Storage &forceStore, SimTK::Vec3 force, SimTK::Vec3 point,
forces->addToRdStorage(forceStore, 0.0, 1.0);
}

void testExternalLoad()
TEST_CASE("ExternalLoads")
{
using namespace SimTK;

Expand Down Expand Up @@ -249,7 +240,8 @@ void testExternalLoad()
}

// Ensure the default values for the ExternalForce properties work as expected.
void testExternalLoadDefaultProperties() {
TEST_CASE("ExternalLoads Default Properties")
{
using namespace SimTK;

Model model("Pendulum.osim");
Expand Down Expand Up @@ -303,3 +295,18 @@ void testExternalLoadDefaultProperties() {
xf->set_force_expressed_in_body("nonexistent");
model.initSystem();
}

// related: #3926
//
// adding a valid `OpenSim::ExternalLoads` to a model shouldn't result in a model
// that cannot be copied.
TEST_CASE("ExternalLoads Can Be Copied")
{
OpenSim::Model model{"ExternalLoadsInSubdir/model-in-subdir.osim"};
model.finalizeConnections(); // should work
model.addModelComponent(&dynamic_cast<OpenSim::ModelComponent&>(*Object::makeObjectFromFile("ExternalLoadsInSubdir/external-loads-in-subdir.xml")));
model.finalizeConnections(); // should also work

OpenSim::Model copy{model}; // create an independent copy containing the `OpenSim::ExternalLoads`
copy.finalizeConnections(); // should work (wasn't when this test was written)
}

0 comments on commit b89164d

Please sign in to comment.