robot_trajectory::RobotTrajectory as standard container and tests#1162
robot_trajectory::RobotTrajectory as standard container and tests#1162bhomberg wants to merge 2 commits intomoveit:kinetic-develfrom
Conversation
| for (auto pair : traj) | ||
| { | ||
| // Compare waypoints | ||
| EXPECT_EQ(*pair.first, traj_robot_state); |
There was a problem hiding this comment.
Could you compare pointers instead of values here, so you don't need to implement an equality operator for class?
| "<group name=\"base_with_bad_subgroups\">" | ||
| "<group name=\"error\"/>" | ||
| "</group>" | ||
| "</robot>"; |
There was a problem hiding this comment.
Is this URDF taken from another moveit file? Alternatively, you could use one of the URDFs from moveit_resources and load it from the file, to keep this test file concise.
There was a problem hiding this comment.
Yes -- it's taken from https://github.com/ros-planning/moveit/blob/melodic-devel/moveit_core/robot_state/test/robot_state_test.cpp
I'll look into the moveit_resources URDFs
|
@bhomberg what's the current state of this PR? I agree with @jonbinney that you don't need to implement an equality operator just for the test. Since pointers won't work (copy-construct) just compare if distance is 0.0 or smaller than |
|
@henningkayser -- I haven't had time to look at this since world MoveIt day. I don't know that I'll have a chance to work on it for a while. |
|
@bhomberg its the next WMD - think you could finish this PR up? |
|
@davetcoleman -- I only had time for a couple hours at WMD today, I don't think I'll get to this until maybe next year's WMD... |
|
That's too bad - this looks like such an improvement of test coverage that is close to being ready for merging! |
|
|
||
| std::size_t generateTestTraj(robot_trajectory::RobotTrajectory& traj, | ||
| const moveit::core::RobotModelConstPtr& robot_model, | ||
| const robot_model::JointModelGroup* joint_model_group) |
There was a problem hiding this comment.
The last parameter is unused.
|
@bhomberg this PR is 3 years old, seems we should close it. Or could you finish it? Would be a shame to not add this test coverage! |
|
I don't have time to work on this right now, but I'll close + reopen if I have a chance to pick it back up! |
* Based on initial size/iterator implementations from moveit/moveit#1162
Description
This is a WIP -- still working on completing the tests.
-Adds iterator to RobotTrajectory class.
-Adds .size() method to RobotTrajectory class.
-Adds basic tests for RobotTrajectory class.
-Adds (in progress) equality operator for RobotState class in order to complete RobotTrajectory tests.
Addresses #1121
Checklist