Skip to content
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

[pull] master from ros-planning:master #459

Open
wants to merge 42 commits into
base: master
Choose a base branch
from

Conversation

pull[bot]
Copy link

@pull pull bot commented Apr 17, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

Standalone applications used the convenience function moveit_warehouse::loadDatabase()
to load a database plugin. Internally, this uses a static plugin loader.
As this static variable is released too late during app shutdown, we get an exception:

```
terminate called after throwing an instance of 'class_loader::LibraryUnloadException'
what():  Attempt to unload library that class_loader is unaware of.
```

Explicitly instantiating the loader ensures correct destruction order and avoids the error.
@pull pull bot requested a review from dg-shadow as a code owner April 17, 2024 09:37
@pull pull bot added the ⤵️ pull label Apr 17, 2024
rhaschke and others added 27 commits April 18, 2024 10:36
The old implementation used a RobotState instance for considered_start_state_, which was transformed into a RobotState message, when filling the actual MotionPlanRequest.
This turned a provided diff message into a full message, which has performance drawbacks when there are many large attached object meshes.
The new implementation uses the message format directly, avoiding unnecessary conversions.

Co-authored-by: Robert Haschke <rhaschke@users.noreply.github.com>
…cific timestamps (#3580)

This commit introduces:

1. per-group current state time retrieval
2. per-group complete robot state waiting

This fixes the following issue: 
I want to call waitForCompleteState on group_A, which the API already supports. There is also another joint out_of_group_A in the robot model that is not available until some time after bringup. 
I'm calling waitForCompleteState(group_A, <timeout>) way earlier than out_of_group_A is published by its source.
The problem with the current implementation is that it waits until <timeout> elapses, but returns true since all joints of group_A exist. I don't see a point in waiting for an out-of-group joint in this case.
…State (#3590)

CommandListManager::setStartState shall update the start state in the next MotionPlanRequest to match the reached robot pose after previous planning steps.
Copying attached collision objects is not necessary there, because the RobotState is initialized once in the beginning. In each iteration of CommandListManager::solveSequenceItems(), only joint states actually update.

Copying the attached bodies is not only a performance penalty, but also introduces a (numerical) drift of the attached object poses as pointed out in #3590 (comment) / #3569.

Co-authored-by: Robert Haschke <rhaschke@users.noreply.github.com>
* github.com/ros-planning -> github.com/moveit
* codecov.io/gh/ros-planning -> codecov.io/gh/moveit
* ros-planning.github.io -> moveit.github.io
Fixes #3538/#3609
If the message is not a diff and parent_scene is not set either,
the message should be handled as a full planning scene message as before #3538.
After Plan+Execute, the joints tab switched back to the start state model because onFinishedExecution()
updates this state to the current one usually. Hence, it was the last model modified...
Ensure that the goal state gets updated in any case.
* Fix constrained-based planning

Constrained-based planning enables the PoseModelStateSpace by default, meaning that state samples are created by sampling in SE(3) and then performing IK. This state space frequently caused issues, eventually leading to the introduction of the ROS parameter enforce_joint_model_state_space in PR #541 as a work-around.

A more detailed analysis shows that the state space's interpolation approach is the culprit - performing interpolation in Cartesian space. If the two interpolation ends are close in Cartesian but distant in joint space, the interpolated state (obtained from IK) can be far away from both start and end (particularly for redundant arms), while its Cartesian pose is naturally close to both interpolation states. This will lead to consecutive waypoints being close in Cartesian space but distant in joint space. If such a trajectory is executed, the controller will interpolate in joint space, leading to a large and unchecked motion.

To avoid this issue, interpolation should be performed in joint space during planning as well.
The only drawback of this change is that trajectories are less linear in Cartesian space.
However, as the trajectory is random-sampled anyway, this shouldn't be a big issue.

* PoseModelStateSpace: Use joint-space distance

Configurations may be similar in Cartesian EE pose, but very far apart in joint space.
What actually matters when connecting states is the joint-space distance.
... when no IK solver is defined for the planning group
* Prevent segfault when getParentLinkModel() is NULL
* Simpliy code
* Add unit test operating the panda gripper

---------

Co-authored-by: Captain Yoshi <captain.yoshisaur@gmail.com>
As the CHOMP planner only considers active joints, it needs to use setJointGroupActivePositions()
instead of setJointGroupPositions().
Don't recreate the FCL object when moving shapes, but just update its transforms.
The Pilz planner generated trajectories with identical subsequent time stamps, which were rejected by the ROS controllers.

Pilz's appendWithStrictTimeIncrease() function had a single if-statement that encompassed two conditions:

1. trajectory is empty
2. the end state of one segment differs from the start state of the next segment

In both cases, the whole trajectory segment was appended with a dt=0. However, that's correct only for case 1.
In case 2, we needed to apply a non-zero time offset dictated by the actual dt of that segment's first waypoint.
* Deprecate JumpThreshold-based computeCartesianPath()
* Remove deprecated methods RobotState::computeCartesianPath()
* Replace deprecated functions
* cleanup after actionlib fix
  actionlib was fixed via ros/actionlib#156

* Reset status variables before calling sendGoal()
  This fixes a race condition, where the execution status is set to RUNNING after the doneCallback has been called,
  e.g. because the controller immediately reports success.
v4hn and others added 14 commits September 4, 2024 13:54
- do not copy acm from monitored scene (ignoring later updates)
- also do not lock scene twice for related checks
- do not cache shared_ptr, but actual RobotState, caching the ptr is
  useless
* populate joint trajectory with actual robot pose and not with last command

* Update moveit_ros/moveit_servo/src/servo_calcs.cpp

Co-authored-by: AndyZe <andyz@utexas.edu>

* pointer is necessary

---------

Co-authored-by: AndyZe <andyz@utexas.edu>
Cannot concatenate string and type.
* Reduce code duplication
* Fix Cartesian interpolation with multiple waypoints continue from latest RobotState
Add new ROS parameter planning_scene_monitor_options/ignored_frames to list all frame names that should not be considered in PSM's transform updates.
#3615 made PoseModelStateSpace identical to ModelBasedStateSpace, both using joint-space interpolation and distance measurements. This resulted in planning timeouts for code that previously worked. Obviously, using joint-space interpolation much more often results in the rejection of tight path constraints than Cartesian interpolation did.

The problem with potential jumps due to Cartesian interpolation in principle remains, which is a dilemma of the current design:

- We do need Cartesian interpolation to reduce the risk of state rejection due to a failing path constraint, which currently cannot be checked during interpolation.
- Ideally, we would do joint interpolation and only resort to Cartesian interpolation if the path constraint is violated, i.e. follow a similar approach as in #3618. However, this would require access to the path constraint...
The tolerance of orientation constraints needs to be interpreted w.r.t. the given frame_id of the constraint instead of the target frame. Not doing so, a large tolerance is interpreted about the wrong axis and resolving constraint frames fails.
Unfortunately, the proposed approach only works for the ROTATION_VECTOR representation.
)

The ResolveConstraintFrames adapter was only changing the planning request used within the planning pipeline,
while the subsequent trajectory validation was still using the original, unresolved request, causing at best a warning and at worst acceptance of paths not satisfying the path constraints.
We make the adapter obsolete and just call the resolver function once in the beginning of the planning pipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants