Obstacle rewrite by rishiso · Pull Request #2440 · RoboJackets/robocup-software

rishiso · 2025-10-28T20:04:46Z

Motivation

The previous obstacle system had several issues:

Use of dynamic obstacles (deprecated)
Use of globally published obstacles (deprecated)
Poor obstacle avoidance for moving obstacles (ie. enemy robots)
No clear separation between obstacle cores (hard physical collisions) and padding zones (safety margins)
Lack of extensibility for obstacle functionality

Changes

Did a complete rewrite of the current obstacles system:

New consolidated Obstacle/ObstacleSet
- Updated all use cases accordingly
- Better visualization functionality for obstacles
Improved creation of moving obstacles via stadium shapes
Removed deprecated features
Optimized EscapeObstaclesPathPlanner
General cleanup of obstacle-related code

Associated / Resolved Issue

ClickUp card

Testing

Did some basic testing and nothing seems broken. Going to do thorough testing on shop computer since Docker is lagging with all the drawing and stuff

Squid5678 · 2025-11-16T13:47:02Z

@rishiso what's the status of this PR?

automated style fixes Co-authored-by: rishiso <rishiso@users.noreply.github.com>

rishiso · 2025-11-16T15:18:17Z

Blocked thinking about some design issues @Squid5678. Going to try to finish during today's meeting

automated style fixes Co-authored-by: rishiso <rishiso@users.noreply.github.com>

Copilot

Pull request overview

This PR implements a complete rewrite of the obstacle system to address several architectural issues including deprecated dynamic obstacles, unclear separation between obstacle cores and padding zones, and poor handling of moving obstacles.

Key Changes

Introduced new Obstacle and ObstacleSet classes that distinguish between obstacle cores (hard physical collisions) and padding zones (safety margins)
Refactored StadiumShape to inherit from CompositeShape for better modularity and correct vertex ordering in polygons
Removed deprecated DynamicObstacle, StaticObstacle, and StadiumObstacle classes along with global obstacle publishing mechanism
Simplified API by consolidating static and dynamic obstacles into a single ObstacleSet parameter throughout the planning system

Reviewed changes

Copilot reviewed 39 out of 41 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/rj_planning/include/rj_planning/obstacle.hpp	New `Obstacle` class with separate obstacle core and padding shape, providing hit detection methods for both
src/rj_planning/include/rj_planning/obstacle_set.hpp	New `ObstacleSet` container class with hit detection, conversion utilities, and visualization methods
src/rj_planning/include/rj_planning/plan_request.hpp	Updated to remove `virtual_obstacles` field and add inline helper functions for creating robot and ball obstacles with velocity-based padding
src/rj_planning/src/plan_request.cpp	Rewrote `fill_obstacles` to populate `ObstacleSet` with moving obstacles using stadium-shaped padding
src/rj_planning/src/trajectory_utils.cpp	Removed `trajectory_hits_dynamic` function, simplified `trajectory_hits_static` to work with `ObstacleSet`
src/rj_planning/src/primitives/create_path.cpp	Updated `rrt` and `intermediate` functions to use single `ObstacleSet` parameter, removed dynamic obstacle retry logic
src/rj_planning/src/primitives/replanner.cpp	Updated to use unified `ObstacleSet`, removed separate dynamic obstacle collision checking
src/rj_planning/src/planners/*.cpp	Updated all planners (collect, settle, escape_obstacles, goalie_idle, path_target) to use `ObstacleSet`
src/rj_planning/src/planner_for_robot.cpp	Removed global obstacles handling, consolidated into single field_obstacles
src/rj_planning/src/global_state.cpp	Removed global obstacles subscription and renamed `set_static_obstacles` to `set_field_obstacles`
src/rj_geometry/src/stadium_shape.cpp	Refactored to use `CompositeShape` base class and fixed polygon vertex ordering
src/rj_geometry/include/rj_geometry/stadium_shape.hpp	Updated to inherit from `CompositeShape` instead of `Shape`
src/rj_planning/tests/*.cpp	Updated test files to construct obstacles with the new `Obstacle` wrapper
src/rj_planning/CMakeLists.txt	Removed obsolete source files (obstacle.cpp, static_obstacle.cpp, stadium_obstacle.cpp)
src/rj_constants/include/rj_constants/topic_names.hpp	Removed deprecated `kGlobalObstaclesTopic` constant
src/rj_common/include/rj_common/context.hpp	Removed `global_obstacles` field from Context struct
install/setup.bash	Generated build artifact that should not be in PR
install/setup.zsh	Generated build artifact that should not be in PR

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-03T02:50:08Z

install/setup.bash


 unset COLCON_CURRENT_PREFIX
-unset _colcon_prefix_chain_bash_source_script
+unset _colcon_prefix_chain_bash_source_script


The install/ directory should not be included in the pull request. According to the .gitignore file, this directory should be excluded from version control. These are generated build artifacts that should not be committed to the repository.

Copilot · 2026-01-03T02:50:09Z

install/setup.zsh


 unset COLCON_CURRENT_PREFIX
-unset _colcon_prefix_chain_zsh_source_script
+unset _colcon_prefix_chain_zsh_source_script


The install/ directory should not be included in the pull request. According to the .gitignore file, this directory should be excluded from version control. These are generated build artifacts that should not be committed to the repository.

src/rj_planning/include/rj_planning/plan_request.hpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jvogt23

Looks pretty good, just a few questions/comments

jvogt23 · 2026-01-03T19:19:19Z

src/rj_planning/src/trajectory_utils.cpp

 namespace planning {

-bool trajectory_hits_static(const Trajectory& trajectory, const rj_geometry::ShapeSet& obstacles,
+bool trajectory_hits_static(const Trajectory& trajectory, const ObstacleSet& obstacles,


Can this be renamed to trajectory_hits_obstacle or are we still differentiating between obstacle types?

jvogt23 · 2026-01-03T19:30:01Z

src/rj_planning/include/rj_planning/obstacle.hpp

+    }
+
+    // Convenience methods: hit() delegates to padding_hit() for compatibility
+    virtual bool hit(rj_geometry::Point pt) { return padding_hit(pt); }


If we consider the "obstacle" shape to be the obstacle itself and the padding to be an unsafe zone/an area where you are likely to hit the obstacle, would it be better to have hit() delegate to obstacle_hit()?

Or are we considering a "hit" to the obstacle to be a collision with either the obstacle or the unsafe zone?

jvogt23 · 2026-01-03T19:32:23Z

src/rj_planning/include/rj_planning/obstacle.hpp

 protected:
    std::shared_ptr<rj_geometry::Shape> obstacle;
    std::shared_ptr<rj_geometry::Shape> padding;
-    std::shared_ptr<rj_geometry::Point> velocity;


Is an obstacle that scales by velocity something we plan to implement in a subclass, or are we getting rid of that idea altogether?

If neither of those are the case, I would think we would still need line 25.

jvogt23 · 2026-01-03T19:38:04Z

src/rj_planning/include/rj_planning/plan_request.hpp

@@ -30,7 +29,6 @@ namespace planning {
 struct PlanRequest {
    PlanRequest(RobotInstant start, MotionCommand command,  // NOLINT


Do we still want PlanRequest to expect a ShapeSet for field_obstacles instead of an ObstacleSet?

EDIT: Is this a ShapeSet for compatibility with ROS messages?

jvogt23 · 2026-01-03T19:45:56Z

src/rj_planning/include/rj_planning/trajectory_utils.hpp

-                           RJ::Time start_time,
-                           rj_geometry::Circle* out_hit_obstacle,
-                           RJ::Time* out_hit_time);
+bool trajectory_hits_static(const Trajectory& trajectory, const ObstacleSet& obstacles,


May make sense to rename to "trajectory_hits_obstacle"

rishiso added 2 commits October 28, 2025 13:14

Removing dynamic obstacles

110bf36

Saving changes

353ec7b

github-actions bot mentioned this pull request Oct 28, 2025

Fix Code Style On obstacle-rewrite #2441

Merged

Fix Code Style On obstacle-rewrite (#2441)

02bb409

automated style fixes Co-authored-by: rishiso <rishiso@users.noreply.github.com>

Saving progress

4c2d0ca

github-actions bot mentioned this pull request Dec 31, 2025

Fix Code Style On obstacle-rewrite #2477

Merged

Done?

2fdd670

rishiso force-pushed the obstacle-rewrite branch from 8c2646c to 2fdd670 Compare January 1, 2026 22:58

github-actions bot mentioned this pull request Jan 1, 2026

Fix Code Style On obstacle-rewrite #2478

Closed

Build works

c09df1d

github-actions bot mentioned this pull request Jan 2, 2026

Fix Code Style On obstacle-rewrite #2479

Merged

github-actions bot and others added 3 commits January 1, 2026 20:51

Fix Code Style On obstacle-rewrite (#2479)

0465ac2

automated style fixes Co-authored-by: rishiso <rishiso@users.noreply.github.com>

Improvements

8229a64

More improvements

ee5ebc9

rishiso marked this pull request as ready for review January 2, 2026 20:56

More

9071d5f

github-actions bot mentioned this pull request Jan 3, 2026

Fix Code Style On obstacle-rewrite #2480

Merged

One more

3a5d40d

rishiso force-pushed the obstacle-rewrite branch from 47f5f6b to 3a5d40d Compare January 3, 2026 02:42

Fix Code Style On obstacle-rewrite (#2480)

269fcb9

automated style fixes Co-authored-by: rishiso <rishiso@users.noreply.github.com>

rishiso requested review from Squid5678, Copilot, jvogt23 and sanatd33 January 3, 2026 02:46

Copilot started reviewing on behalf of rishiso January 3, 2026 02:47 View session

Copilot AI reviewed Jan 3, 2026

View reviewed changes

Update src/rj_planning/include/rj_planning/plan_request.hpp

e87cb16

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jvogt23 requested changes Jan 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Obstacle rewrite#2440

Obstacle rewrite#2440
rishiso wants to merge 13 commits intoros2from
obstacle-rewrite

rishiso commented Oct 28, 2025 •

edited

Loading

Uh oh!

Squid5678 commented Nov 16, 2025

Uh oh!

rishiso commented Nov 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Uh oh!

jvogt23 left a comment

Uh oh!

jvogt23 Jan 3, 2026

Uh oh!

jvogt23 Jan 3, 2026

Uh oh!

jvogt23 Jan 3, 2026

Uh oh!

jvogt23 Jan 3, 2026

Uh oh!

jvogt23 Jan 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -30,7 +29,6 @@ namespace planning {
		struct PlanRequest {
		PlanRequest(RobotInstant start, MotionCommand command, // NOLINT

Conversation

rishiso commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Associated / Resolved Issue

Testing

Uh oh!

Squid5678 commented Nov 16, 2025

Uh oh!

rishiso commented Nov 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jvogt23 left a comment

Choose a reason for hiding this comment

Uh oh!

jvogt23 Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

jvogt23 Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

jvogt23 Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

jvogt23 Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

jvogt23 Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rishiso commented Oct 28, 2025 •

edited

Loading