-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fix and add motion planner server tests #30
Fix and add motion planner server tests #30
Conversation
Signed-off-by: methylDragon <[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.
Looks good. Minor comments. Should we also update the integration test CI job to run colcon test
for this pkg?
"ros2_controllers.yaml", | ||
) | ||
ros2_control_node = Node( | ||
package="controller_manager", |
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.
Add <test_depend>
for ros2_control pkgs.
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.
@@ -318,6 +355,8 @@ def generate_test_description(): | |||
trigger_lifecycle_5, | |||
trigger_lifecycle_6, | |||
trigger_get_plan_irb1300_poses, | |||
ros2_control_node, | |||
*load_controllers, |
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.
What does the *
do? Is it a pythonic way to append to a list?
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.
The *
unpacks the list into its composite elements (you can think of it as an extend()
but in the middle of the list.
It was introduced to avoid "clunky syntax handling". https://peps.python.org/pep-3132/ , so I think it's acceptable.
The technically simpler way to do it would be:
return launch.LaunchDescription([
robot_state_publisher,
motion_planner,
move_group_node,
static_tf_node_1,
static_tf_node_2,
trigger_lifecycle_1,
trigger_lifecycle_2,
trigger_lifecycle_3,
trigger_lifecycle_4,
trigger_lifecycle_5,
trigger_lifecycle_6,
trigger_get_plan_irb1300_poses,
ros2_control_node]
+ load_controllers
+ [launch_testing.actions.ReadyToTest()
]), node_mapping
But I didn't like the triple addition.
Signed-off-by: methylDragon <[email protected]>
Signed-off-by: methylDragon <[email protected]>
I added the tests to CI b18e5b3 |
* Add motion plan cache Signed-off-by: methylDragon <[email protected]> * Switch to snake case for function names Signed-off-by: methylDragon <[email protected]> * Print cache fetch time and key plans on planned execution time Signed-off-by: methylDragon <[email protected]> * Add overwrite_worse_plans param Signed-off-by: methylDragon <[email protected]> * Fix plan fetching printout and don't recache fetched plans Signed-off-by: methylDragon <[email protected]> * Update only use cached plans parameter name Signed-off-by: methylDragon <[email protected]> * Add copyright headers and motion_planner namespace Signed-off-by: methylDragon <[email protected]> * Make motion_plan_cache a unique_ptr instead of shared_ptr Signed-off-by: methylDragon <[email protected]> * Set numerical precision Signed-off-by: methylDragon <[email protected]> * Use enum for planner database mode Signed-off-by: methylDragon <[email protected]> * Rename cache methods to prepare for cartesian caching Signed-off-by: methylDragon <[email protected]> * Add very important warning to cache class Signed-off-by: methylDragon <[email protected]> * Fix access after move bug Signed-off-by: methylDragon <[email protected]> * Remove internal cache node Signed-off-by: methylDragon <[email protected]> * Implement cartesian plan cache (#18) * Add cartesian plan caching interfaces Signed-off-by: methylDragon <[email protected]> * Add construct_get_cartesian_plan_request Signed-off-by: methylDragon <[email protected]> * Add goal query and metadata Signed-off-by: methylDragon <[email protected]> * Add start query and metadata Signed-off-by: methylDragon <[email protected]> * Implement top level cache ops Signed-off-by: methylDragon <[email protected]> * Use motion plan cache for cartesian plans Signed-off-by: methylDragon <[email protected]> * Allow mismatched plan frames since we coerce anyway Signed-off-by: methylDragon <[email protected]> * Fix move bug Signed-off-by: methylDragon <[email protected]> * Plan cache code review fixes (sans unit tests) (#26) * Remove query appending macro Signed-off-by: methylDragon <[email protected]> * Default to warehouse_ros plugin if warehouse plugin isn't set Signed-off-by: methylDragon <[email protected]> * Return and use init result Signed-off-by: methylDragon <[email protected]> * Add todo for catching exceptions Signed-off-by: methylDragon <[email protected]> * Implement plan fetching with configurable key Signed-off-by: methylDragon <[email protected]> * Add comments for exact match tolerance Signed-off-by: methylDragon <[email protected]> * Slightly refactor put plan Signed-off-by: methylDragon <[email protected]> * Rename overwrite to delete_worse_plans Signed-off-by: methylDragon <[email protected]> * Split out motion plan cache into its own library Signed-off-by: methylDragon <[email protected]> * Sort constraints for reduced cardinality Signed-off-by: methylDragon <[email protected]> * Rename util function Signed-off-by: methylDragon <[email protected]> * Add todo for is_diff Signed-off-by: methylDragon <[email protected]> * Add unit tests for motion plan cache (#28) * Add count methods Signed-off-by: methylDragon <[email protected]> * Enable shared from this for cache class Signed-off-by: methylDragon <[email protected]> * Add unit test build rules Signed-off-by: methylDragon <[email protected]> * Add tests for motion plan cache (but not cartesian) Signed-off-by: methylDragon <[email protected]> * Fix bugs in cartesian caching Signed-off-by: methylDragon <[email protected]> * Add tests for cartesian plan cache Signed-off-by: methylDragon <[email protected]> * Exit if a test fails Signed-off-by: methylDragon <[email protected]> * Remove gtest import Signed-off-by: methylDragon <[email protected]> * Remove enable_shared_from_this Signed-off-by: methylDragon <[email protected]> * Only check for failure Signed-off-by: methylDragon <[email protected]> * Test half in-tolerance Signed-off-by: methylDragon <[email protected]> * Test different robot cache Signed-off-by: methylDragon <[email protected]> * Add force_cache_mode_execute_read_only (#29) * Add force_cache_mode_execute_read_only Signed-off-by: methylDragon <[email protected]> * Add force_cache_mode_execute_read_only input port Signed-off-by: methylDragon <[email protected]> --------- Signed-off-by: methylDragon <[email protected]> --------- Signed-off-by: methylDragon <[email protected]> --------- Signed-off-by: methylDragon <[email protected]> --------- Signed-off-by: methylDragon <[email protected]> * Fix nullptr dereference Signed-off-by: methylDragon <[email protected]> * Fix and add motion planner server tests (#30) * Fix and add motion planner server tests Signed-off-by: methylDragon <[email protected]> * Add test deps Signed-off-by: methylDragon <[email protected]> * Add motion planner server test to CI Signed-off-by: methylDragon <[email protected]> --------- Signed-off-by: methylDragon <[email protected]> --------- Signed-off-by: methylDragon <[email protected]>
Builds on top of:
I uncommented the old launch test, and discovered it was passing.
Then I tried adding a test for the motion plan cache, and had it fail because we need access to the robot joint state.
Turns out the old tests weren't loading a ros2_control controller, so I fixed it (maybe that's what broke in the first place?)
After building, test with: