feat: roboplan integration#2478
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #2478 +/- ##
==========================================
+ Coverage 71.01% 71.09% +0.08%
==========================================
Files 892 897 +5
Lines 79232 80151 +919
Branches 7077 7214 +137
==========================================
+ Hits 56267 56987 +720
- Misses 21123 21295 +172
- Partials 1842 1869 +27
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
TomCC7
requested review from
leshy,
mustafab0,
paul-nechifor and
spomichter
as code owners
Contributor
Greptile SummaryThis PR integrates RoboPlan as an optional manipulation planning backend, addressing all issues flagged in the previous review rounds.
Confidence Score: 5/5Safe to merge; all previously flagged issues have been resolved in this version. Every issue raised in prior review rounds — the missing
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant MM as ManipulationModule
participant F as factory.py
participant RPW as RoboPlanWorld
participant RP as roboplan C++ binding
MM->>F: "create_world(backend="roboplan")"
F->>RPW: RoboPlanWorld()
MM->>RPW: add_robot(config)
RPW->>RP: Scene(urdf, srdf, pkg_paths)
RPW-->>MM: robot_id
MM->>RPW: finalize()
alt "planner_name="roboplan""
MM->>F: "create_planner(name="roboplan", world=RPW)"
F-->>MM: returns RPW (PlannerSpec)
MM->>RPW: "plan_joint_path(world=self, ...)"
RPW->>RP: RRT.plan(start, goal)
RP-->>RPW: JointPath
RPW-->>MM: PlanningResult(SUCCESS)
else "planner_name="rrt_connect""
MM->>F: "create_planner(name="rrt_connect")"
F-->>MM: RRTConnectPlanner
MM->>RRTConnectPlanner: "plan_joint_path(world=RPW, ...)"
RRTConnectPlanner->>RPW: check_config_collision_free(...)
RPW->>RP: hasCollisions(q)
RP-->>RPW: bool
RPW-->>RRTConnectPlanner: bool
RRTConnectPlanner-->>MM: PlanningResult(SUCCESS)
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant MM as ManipulationModule
participant F as factory.py
participant RPW as RoboPlanWorld
participant RP as roboplan C++ binding
MM->>F: "create_world(backend="roboplan")"
F->>RPW: RoboPlanWorld()
MM->>RPW: add_robot(config)
RPW->>RP: Scene(urdf, srdf, pkg_paths)
RPW-->>MM: robot_id
MM->>RPW: finalize()
alt "planner_name="roboplan""
MM->>F: "create_planner(name="roboplan", world=RPW)"
F-->>MM: returns RPW (PlannerSpec)
MM->>RPW: "plan_joint_path(world=self, ...)"
RPW->>RP: RRT.plan(start, goal)
RP-->>RPW: JointPath
RPW-->>MM: PlanningResult(SUCCESS)
else "planner_name="rrt_connect""
MM->>F: "create_planner(name="rrt_connect")"
F-->>MM: RRTConnectPlanner
MM->>RRTConnectPlanner: "plan_joint_path(world=RPW, ...)"
RRTConnectPlanner->>RPW: check_config_collision_free(...)
RPW->>RP: hasCollisions(q)
RP-->>RPW: bool
RPW-->>RRTConnectPlanner: bool
RRTConnectPlanner-->>MM: PlanningResult(SUCCESS)
end
Reviews (26): Last reviewed commit: "Merge branch 'main' into cc/feat/robopla..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
github-actions
Bot
added
the
ready-to-merge
github-actions
Bot
added
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
github-actions
Bot
added
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
Replace the str-plus-comment config fields with Literal type aliases (WorldBackend / PlannerName / KinematicsName) so Pydantic validates the values. The SUPPORTED_* tuples are now derived from the aliases via get_args, removing the chance of the two drifting.
plan_joint_path wrapped the whole _run_native_rrt call in except Exception and stringified the error, losing the traceback for unexpected failures. Catch the ValueError that _run_native_rrt raises for the known no-path case and let anything else propagate, matching the sibling RRTConnectPlanner.
A rejected collision-exclusion pair is safety-relevant, so log it at warning rather than debug. Detect a missing setCollisions API once up-front and warn instead of silently returning out of the loop on the first AttributeError.
PlannerSpec is a @runtime_checkable Protocol declaring plan_joint_path and get_name, so isinstance both validates the world and narrows the type for the return -- replacing the hasattr check and the type: ignore[return-value]. The test double now uses spec=PlannerSpec so it actually satisfies the protocol under Python 3.12's getattr_static check.
validate_backend_combination and create_planner raised the identical 'planner_name="roboplan" requires world_backend="roboplan"' message; hoist it to a module constant so the two can't drift.
Move ManipulationModule construction into a make_module factory fixture that stops every built module on teardown, so cleanup runs even when the test body fails -- replacing the hand-rolled try/finally: module.stop().
The manipulation extra depends on the roboplan package (pyproject.toml), not roboplan-dimos, which exists nowhere in the repo.
_has_collisions took a ctx it never read, and model_handle (config.name) was threaded through _extract_joint_limits and _query_scene_joint_limits without being used. Remove the dead parameters and the now-unused _RoboPlanRobotData.model_handle field.
_run_native_rrt already raises when planner.plan returns None before calling _extract_native_path, so the identical check inside _extract_native_path is dead.
Add the missing -> None return annotations (and a pytest.MonkeyPatch hint) so the suite matches the rest of the repo, and assert the factory return types with isinstance instead of comparing type(...).__name__ to a string.
The test parametrized a single base_pose case, with its type: ignore[call-arg] stranded on the tuple paren. Inline it as one direct test and move the ignore onto the PoseStamped call it applies to.
Drop the stray double blank line after the manipulation extra and add the missing space before the closing brace of exclude-newer-package.
create_kinematics's default and the create_planning_specs fallback both hardcoded "pink"; route both through a DEFAULT_KINEMATICS_NAME constant so they can't drift.
github-actions
Bot
added
the
ready-to-merge
…ion-autofixes Auto-fixes for cc/feat/roboplan-integration
github-actions
Bot
added
ready-to-merge
github-actions
Bot
removed
the
ready-to-merge
github-actions
Bot
added
the
ready-to-merge
mustafab0
approved these changes
Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integrate roboplan into manipulation module. One class implements roboplan's worldspec+plannerspec, achieving composition of
roboplan world+roboplan planner/generic planner.Better merge after #2413 since we resurfaced creation of world/planner specs.
Design choices:
How to Test
launch coordinator and pass argument for roboplan world/planner.
Contributor License Agreement