Skip to content

Commit 3ec69fb

Browse files
authored
Merge PR #460: improvements to ModifyPlanningScene stage
2 parents 78da3e4 + 2728b3c commit 3ec69fb

File tree

13 files changed

+196
-67
lines changed

13 files changed

+196
-67
lines changed

.github/workflows/ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ jobs:
4343
CLANG_TIDY_ARGS: -quiet -export-fixes ${{ github.workspace }}/.work/clang-tidy-fixes.yaml
4444
DOCKER_IMAGE: moveit/moveit:${{ matrix.env.IMAGE }}
4545
UNDERLAY: /root/ws_moveit/install
46-
DOWNSTREAM_WORKSPACE: "github:ubi-agni/mtc_demos#master github:TAMS-Group/mtc_pour#master"
46+
DOWNSTREAM_WORKSPACE: "github:ubi-agni/mtc_demos#master"
4747
CCACHE_DIR: ${{ github.workspace }}/.ccache
4848
BASEDIR: ${{ github.workspace }}/.work
4949
CACHE_PREFIX: "${{ matrix.env.IMAGE }}${{ contains(matrix.env.TARGET_CMAKE_ARGS, '--coverage') && '-ccov' || '' }}"

core/include/moveit/task_constructor/stages/modify_planning_scene.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ class ModifyPlanningScene : public PropagatingEitherWay
149149

150150
protected:
151151
// apply stored modifications to scene
152-
InterfaceState apply(const InterfaceState& from, bool invert);
153-
void processCollisionObject(planning_scene::PlanningScene& scene, const moveit_msgs::CollisionObject& object);
152+
std::pair<InterfaceState, SubTrajectory> apply(const InterfaceState& from, bool invert);
153+
void processCollisionObject(planning_scene::PlanningScene& scene, moveit_msgs::CollisionObject& object, bool invert);
154154
void attachObjects(planning_scene::PlanningScene& scene, const std::pair<std::string, std::pair<Names, bool>>& pair,
155155
bool invert);
156156
void allowCollisions(planning_scene::PlanningScene& scene, const CollisionMatrixPairs& pairs, bool invert);

core/include/moveit/task_constructor/storage.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,11 @@ class SolutionBase
311311
auto& markers() { return markers_; }
312312
const auto& markers() const { return markers_; }
313313

314+
/// convert solution to message
315+
void toMsg(moveit_task_constructor_msgs::Solution& solution, Introspection* introspection = nullptr) const;
314316
/// append this solution to Solution msg
315-
virtual void fillMessage(moveit_task_constructor_msgs::Solution& solution,
316-
Introspection* introspection = nullptr) const = 0;
317+
virtual void appendTo(moveit_task_constructor_msgs::Solution& solution,
318+
Introspection* introspection = nullptr) const = 0;
317319
void fillInfo(moveit_task_constructor_msgs::SolutionInfo& info, Introspection* introspection = nullptr) const;
318320

319321
/// required to dispatch to type-specific CostTerm methods via vtable
@@ -354,7 +356,7 @@ class SubTrajectory : public SolutionBase
354356
robot_trajectory::RobotTrajectoryConstPtr trajectory() const { return trajectory_; }
355357
void setTrajectory(const robot_trajectory::RobotTrajectoryPtr& t) { trajectory_ = t; }
356358

357-
void fillMessage(moveit_task_constructor_msgs::Solution& msg, Introspection* introspection = nullptr) const override;
359+
void appendTo(moveit_task_constructor_msgs::Solution& msg, Introspection* introspection = nullptr) const override;
358360

359361
double computeCost(const CostTerm& cost, std::string& comment) const override;
360362

@@ -387,7 +389,7 @@ class SolutionSequence : public SolutionBase
387389
void push_back(const SolutionBase& solution);
388390

389391
/// append all subsolutions to solution
390-
void fillMessage(moveit_task_constructor_msgs::Solution& msg, Introspection* introspection) const override;
392+
void appendTo(moveit_task_constructor_msgs::Solution& msg, Introspection* introspection) const override;
391393

392394
double computeCost(const CostTerm& cost, std::string& comment) const override;
393395

@@ -418,8 +420,8 @@ class WrappedSolution : public SolutionBase
418420
: SolutionBase(creator, cost), wrapped_(wrapped) {}
419421
explicit WrappedSolution(Stage* creator, const SolutionBase* wrapped)
420422
: WrappedSolution(creator, wrapped, wrapped->cost()) {}
421-
void fillMessage(moveit_task_constructor_msgs::Solution& solution,
422-
Introspection* introspection = nullptr) const override;
423+
void appendTo(moveit_task_constructor_msgs::Solution& solution,
424+
Introspection* introspection = nullptr) const override;
423425

424426
double computeCost(const CostTerm& cost, std::string& comment) const override;
425427

core/python/bindings/src/core.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ void export_core(pybind11::module& m) {
124124
":visualization_msgs:`Marker`: Markers to visualize important aspects of the trajectory (read-only)")
125125
.def(
126126
"toMsg",
127-
[](const SolutionBasePtr& s) {
127+
[](const SolutionBase& self) {
128128
moveit_task_constructor_msgs::Solution msg;
129-
s->fillMessage(msg);
129+
self.toMsg(msg);
130130
return msg;
131131
},
132132
"Convert to the ROS message ``Solution``");
@@ -440,6 +440,7 @@ void export_core(pybind11::module& m) {
440440
t.add(it->cast<Stage::pointer>());
441441
},
442442
"Append stage(s) to the task's top-level container")
443+
.def("insert", &Task::insert, "stage"_a, "before"_a = -1, "Insert stage before given index")
443444
.def("__len__", [](const Task& t) { t.stages()->numChildren(); })
444445
.def(
445446
"__getitem__",
@@ -494,7 +495,7 @@ void export_core(pybind11::module& m) {
494495

495496
MoveGroupInterface::Plan plan;
496497
moveit_task_constructor_msgs::Solution serialized;
497-
solution->fillMessage(serialized);
498+
solution->toMsg(serialized);
498499

499500
for (const moveit_task_constructor_msgs::SubTrajectory& traj : serialized.sub_trajectory) {
500501
if (!traj.trajectory.joint_trajectory.points.empty()) {

core/python/bindings/src/stages.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ void export_stages(pybind11::module& m) {
105105
const std::string& attach_link) {
106106
self.attachObjects(elementOrList<std::string>(names), attach_link, false);
107107
}, "Detach multiple objects from a robot link", "names"_a, "attach_link"_a)
108+
.def("allowCollisions", [](ModifyPlanningScene& self, const std::string& object, bool allow) {self.allowCollisions(object, allow);},
109+
"Allow or disable all collisions involving the given object", "object"_a, "enable_collision"_a = true)
108110
.def("allowCollisions", [](ModifyPlanningScene& self,
109111
const py::object& first, const py::object& second, bool enable_collision) {
110112
self.allowCollisions(elementOrList<std::string>(first), elementOrList<std::string>(second), enable_collision);
@@ -114,7 +116,10 @@ void export_stages(pybind11::module& m) {
114116
115117
.. _CollisionObject: https://docs.ros.org/en/melodic/api/moveit_msgs/html/msg/CollisionObject.html
116118
117-
)", "collision_object"_a);
119+
)", "collision_object"_a)
120+
.def("removeObject", &ModifyPlanningScene::removeObject,
121+
"Remove a CollisionObject_ from the planning scene", "name"_a)
122+
;
118123

119124
properties::class_<CurrentState, Stage>(m, "CurrentState", R"(
120125
Fetch the current PlanningScene / real robot state via the ``get_planning_scene`` service.

core/python/test/rostest_mps.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
#! /usr/bin/env python
2+
3+
from __future__ import print_function
4+
import unittest
5+
import rostest
6+
from moveit_commander.roscpp_initializer import roscpp_initialize
7+
from moveit_commander import PlanningSceneInterface
8+
from moveit.task_constructor import core, stages
9+
from geometry_msgs.msg import PoseStamped
10+
11+
12+
def make_pose(x, y, z):
13+
pose = PoseStamped()
14+
pose.header.frame_id = "base_link"
15+
pose.pose.position.x = x
16+
pose.pose.position.y = y
17+
pose.pose.position.z = z
18+
pose.pose.orientation.w = 1.0
19+
return pose
20+
21+
22+
class TestModifyPlanningScene(unittest.TestCase):
23+
PLANNING_GROUP = "manipulator"
24+
25+
def setUp(self):
26+
super(TestModifyPlanningScene, self).setUp()
27+
self.psi = PlanningSceneInterface(synchronous=True)
28+
self.make_box = self.psi._PlanningSceneInterface__make_box
29+
# insert a box to collide with
30+
self.psi.add_box("box", make_pose(0.8, 0.25, 1.25), [0.2, 0.2, 0.2])
31+
self.task = task = core.Task()
32+
task.add(stages.CurrentState("current"))
33+
34+
def insertMove(self, position=-1):
35+
# colliding motion
36+
move = stages.MoveRelative("move", core.JointInterpolationPlanner())
37+
move.group = self.PLANNING_GROUP
38+
move.setDirection({"joint_1": 0.3})
39+
self.task.insert(move, position)
40+
41+
def test_collision(self):
42+
self.insertMove()
43+
self.assertFalse(self.task.plan())
44+
45+
def test_allow_collision_list(self):
46+
mps = stages.ModifyPlanningScene("allow specific collisions for box")
47+
mps.allowCollisions("box", ["link_4", "link_5", "link_6"], True)
48+
self.task.add(mps)
49+
self.insertMove()
50+
self.assertTrue(self.task.plan())
51+
52+
def test_allow_collision_all(self):
53+
# insert an extra collision object that is unknown to ACM
54+
self.psi.add_box("block", make_pose(0.8, 0.55, 1.25), [0.2, 0.2, 0.2])
55+
# attach box to end effector
56+
self.psi.attach_box("link_6", "box")
57+
self.insertMove()
58+
self.assertFalse(self.task.plan())
59+
60+
# allow all collisions for attached "box" object
61+
mps = stages.ModifyPlanningScene("allow all collisions for box")
62+
mps.allowCollisions("box", True)
63+
self.task.insert(mps, 1)
64+
self.assertTrue(self.task.plan())
65+
# restore original state
66+
self.psi.remove_attached_object("link_6", "box")
67+
self.psi.remove_world_object("block")
68+
69+
def test_fw_add_object(self):
70+
mps = stages.ModifyPlanningScene("addObject(block)")
71+
mps.addObject(self.make_box("block", make_pose(0.8, 0.55, 1.25), [0.2, 0.2, 0.2]))
72+
self.task.add(mps)
73+
self.insertMove()
74+
self.assertFalse(self.task.plan())
75+
76+
def test_fw_remove_object(self):
77+
mps = stages.ModifyPlanningScene("removeObject(box)")
78+
mps.removeObject("box")
79+
self.task.insert(mps, 1)
80+
self.assertTrue(self.task.plan())
81+
s = self.task.solutions[0].toMsg()
82+
self.assertEqual(s.sub_trajectory[1].scene_diff.world.collision_objects[0].id, "box")
83+
84+
def DISABLED_test_bw_add_object(self):
85+
# add object to move_group's planning scene
86+
self.psi.add_box("block", make_pose(0.8, 0.55, 1.25), [0.2, 0.2, 0.2])
87+
88+
# backward operation will actually remove the object
89+
mps = stages.ModifyPlanningScene("addObject(block) backwards")
90+
mps.addObject(self.make_box("block", make_pose(0.8, 0.55, 1.25), [0.2, 0.2, 0.2]))
91+
self.task.insert(mps, 0)
92+
self.insertMove(0)
93+
self.assertTrue(self.task.plan())
94+
95+
self.psi.remove_world_object("block") # restore original state
96+
97+
s = self.task.solutions[0].toMsg()
98+
for ps in [s.start_scene] + [s.sub_trajectory[i].scene_diff for i in [0, 1]]:
99+
objects = [o.id for o in ps.world.collision_objects]
100+
self.assertTrue(objects == ["block", "box"])
101+
102+
def DISABLED_test_bw_remove_object(self):
103+
mps = stages.ModifyPlanningScene("removeObject(box) backwards")
104+
mps.removeObject("box")
105+
self.task.insert(mps, 0)
106+
self.insertMove(0)
107+
self.assertFalse(self.task.plan())
108+
109+
110+
if __name__ == "__main__":
111+
roscpp_initialize("test_mtc")
112+
rostest.rosrun("mtc", "mps", TestModifyPlanningScene)

core/python/test/rostest_mtc.py

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,31 +14,21 @@
1414
class Test(unittest.TestCase):
1515
PLANNING_GROUP = "manipulator"
1616

17-
@classmethod
18-
def setUpClass(self):
19-
pass
17+
def test_MoveAndExecute(self):
18+
moveRel = stages.MoveRelative("moveRel", core.JointInterpolationPlanner())
19+
moveRel.group = self.PLANNING_GROUP
20+
moveRel.setDirection({"joint_1": 0.2, "joint_2": 0.4})
2021

21-
@classmethod
22-
def tearDown(self):
23-
pass
22+
moveTo = stages.MoveTo("moveTo", core.JointInterpolationPlanner())
23+
moveTo.group = self.PLANNING_GROUP
24+
moveTo.setGoal("all-zeros")
2425

25-
def test_MoveRelative(self):
2626
task = core.Task()
27-
task.add(stages.CurrentState("current"))
28-
move = stages.MoveRelative("move", core.JointInterpolationPlanner())
29-
move.group = self.PLANNING_GROUP
30-
move.setDirection({"joint_1": 0.2, "joint_2": 0.4})
31-
task.add(move)
32-
33-
task.enableIntrospection()
34-
task.init()
35-
task.plan()
27+
task.add(stages.CurrentState("current"), moveRel, moveTo)
3628

29+
self.assertTrue(task.plan())
3730
self.assertEqual(len(task.solutions), 1)
38-
for s in task.solutions:
39-
print(s)
40-
s = task.solutions[0]
41-
task.execute(s)
31+
task.execute(task.solutions[0])
4232

4333
def test_Merger(self):
4434
cartesian = core.CartesianPath()

core/python/test/rostest_mtc.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<launch>
22
<include file="$(find moveit_resources_fanuc_moveit_config)/launch/test_environment.launch" />
33
<test pkg="moveit_task_constructor_core" type="rostest_mtc.py" test-name="rostest_mtc" time-limit="60" args="" />
4+
<test pkg="moveit_task_constructor_core" type="rostest_mps.py" test-name="rostest_mps" time-limit="60" args="" />
45
<test pkg="moveit_task_constructor_core" type="rostest_trampoline.py" test-name="rostest_trampoline" time-limit="60" args="" />
56
</launch>

core/src/introspection.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,7 @@ void Introspection::registerSolution(const SolutionBase& s) {
154154
}
155155

156156
void Introspection::fillSolution(moveit_task_constructor_msgs::Solution& msg, const SolutionBase& s) {
157-
s.fillMessage(msg, this);
158-
s.start()->scene()->getPlanningSceneMsg(msg.start_scene);
159-
157+
s.toMsg(msg, this);
160158
msg.task_id = impl->task_id_;
161159
}
162160

core/src/stages/modify_planning_scene.cpp

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,13 @@ void ModifyPlanningScene::allowCollisions(const std::string& first, const moveit
8484
}
8585

8686
void ModifyPlanningScene::computeForward(const InterfaceState& from) {
87-
sendForward(from, apply(from, false), SubTrajectory());
87+
auto result = apply(from, false);
88+
sendForward(from, std::move(result.first), std::move(result.second));
8889
}
8990

9091
void ModifyPlanningScene::computeBackward(const InterfaceState& to) {
91-
sendBackward(apply(to, true), to, SubTrajectory());
92+
auto result = apply(to, true);
93+
sendBackward(std::move(result.first), to, std::move(result.second));
9294
}
9395

9496
void ModifyPlanningScene::attachObjects(planning_scene::PlanningScene& scene,
@@ -111,38 +113,53 @@ void ModifyPlanningScene::allowCollisions(planning_scene::PlanningScene& scene,
111113
collision_detection::AllowedCollisionMatrix& acm = scene.getAllowedCollisionMatrixNonConst();
112114
bool allow = invert ? !pairs.allow : pairs.allow;
113115
if (pairs.second.empty()) {
114-
for (const auto& name : pairs.first)
116+
for (const auto& name : pairs.first) {
117+
acm.setDefaultEntry(name, allow);
115118
acm.setEntry(name, allow);
119+
}
116120
} else
117121
acm.setEntry(pairs.first, pairs.second, allow);
118122
}
119123

120124
// invert indicates, whether to detach instead of attach (and vice versa)
121125
// as well as to forbid instead of allow collision (and vice versa)
122-
InterfaceState ModifyPlanningScene::apply(const InterfaceState& from, bool invert) {
126+
std::pair<InterfaceState, SubTrajectory> ModifyPlanningScene::apply(const InterfaceState& from, bool invert) {
123127
planning_scene::PlanningScenePtr scene = from.scene()->diff();
124-
InterfaceState result(scene);
125-
// add/remove objects
126-
for (const auto& collision_object : collision_objects_)
127-
processCollisionObject(*scene, collision_object);
128-
129-
// attach/detach objects
130-
for (const auto& pair : attach_objects_)
131-
attachObjects(*scene, pair, invert);
132-
133-
// allow/forbid collisions
134-
for (const auto& pairs : collision_matrix_edits_)
135-
allowCollisions(*scene, pairs, invert);
136-
137-
if (callback_)
138-
callback_(scene, properties());
139-
140-
return result;
128+
InterfaceState state(scene);
129+
SubTrajectory traj;
130+
try {
131+
// add/remove objects
132+
for (auto& collision_object : collision_objects_)
133+
processCollisionObject(*scene, collision_object, invert);
134+
135+
// attach/detach objects
136+
for (const auto& pair : attach_objects_)
137+
attachObjects(*scene, pair, invert);
138+
139+
// allow/forbid collisions
140+
for (const auto& pairs : collision_matrix_edits_)
141+
allowCollisions(*scene, pairs, invert);
142+
143+
if (callback_)
144+
callback_(scene, properties());
145+
} catch (const std::exception& e) {
146+
traj.markAsFailure(e.what());
147+
}
148+
return std::make_pair(state, traj);
141149
}
142150

143151
void ModifyPlanningScene::processCollisionObject(planning_scene::PlanningScene& scene,
144-
const moveit_msgs::CollisionObject& object) {
152+
moveit_msgs::CollisionObject& object, bool invert) {
153+
auto op = object.operation;
154+
if (invert) {
155+
if (op == moveit_msgs::CollisionObject::ADD)
156+
op = moveit_msgs::CollisionObject::REMOVE;
157+
else if (op == moveit_msgs::CollisionObject::REMOVE)
158+
throw std::runtime_error("cannot apply removeObject() backwards");
159+
}
160+
145161
scene.processCollisionObjectMsg(object);
162+
object.operation = op;
146163
}
147164
} // namespace stages
148165
} // namespace task_constructor

0 commit comments

Comments
 (0)