Repository navigation
[params 6/13] rj_planning: remove rj_param_utils - #2593
Open
yuvrajdhadwal wants to merge 3 commits into
Open
yuvrajdhadwal wants to merge 3 commits into
yuvrajdhadwal wants to merge 3 commits into
Conversation
This was referenced Oct 12, 2026
yuvrajdhadwal
force-pushed
the
params/06-planning
branch
from
October 12, 2026 00:12
dc6d6a8 to
6340671
Compare
yuvrajdhadwal
added this pull request to stack #2598
October 12, 2026 00:12
yuvrajdhadwal
changed the title
yuvrajdhadwal
changed the title
yuvrajdhadwal
changed the title
yuvrajdhadwal
changed the title
yuvrajdhadwal
force-pushed
the
params/06-planning
branch
from
October 12, 2026 01:28
f9babf7 to
f301e7b
Compare
yuvrajdhadwal
force-pushed
the
params/06-planning
branch
from
October 12, 2026 02:00
f301e7b to
c16c373
Compare
yuvrajdhadwal
marked this pull request as ready for review
October 12, 2026 14:44
jvogt23
force-pushed
the
params/06-planning
branch
from
October 14, 2026 00:23
c16c373 to
4fafbc5
Compare
jvogt23
force-pushed
the
params/06-planning
branch
from
October 14, 2026 01:23
4fafbc5 to
61f960e
Compare
jvogt23
force-pushed
the
params/06-planning
branch
from
October 14, 2026 01:38
61f960e to
88236b9
Compare
jvogt23
force-pushed
the
params/06-planning
branch
from
October 18, 2026 23:21
88236b9 to
ad9cd34
Compare
jvogt23
force-pushed
the
params/06-planning
branch
from
October 19, 2026 01:34
3c5d4d9 to
a331636
Compare
shourikb
requested changes
Oct 21, 2026
shourikb
left a comment
shourikb
left a comment
Contributor
There was a problem hiding this comment.
Few dependencies on rj_utils which is gone now and a minor comment, other than that looks good.
| }; | ||
|
|
||
| struct PivotParams { | ||
| double radius_multiplier; |
Contributor
There was a problem hiding this comment.
can we have a comment of what this is used for? Do we really need a struct for one variable?
| find_package(rj_convert REQUIRED) | ||
| find_package(rj_msgs REQUIRED) | ||
| find_package(rj_param_utils REQUIRED) | ||
| find_package(rj_utils REQUIRED) |
Contributor
There was a problem hiding this comment.
rj_utils is gone now. why do we depend on rj_utils here?
| <depend>rj_convert</depend> | ||
| <depend>rj_msgs</depend> | ||
| <depend>rj_param_utils</depend> | ||
| <depend>rj_utils</depend> |
jvogt23
force-pushed
the
params/06-planning
branch
from
October 21, 2026 03:06
a331636 to
1e1a7dc
Compare
Replaces every planning PARAM_* global and deletes planning_params from rj_param_utils. - PlannerNode declares every /planner param once at startup (load_planning_params) into a PlanningParams struct and shares it read-only with each PlannerForRobot. Each PlanRequest carries it and planners read request.params, so planners no longer touch the node. - Replanner::PlanParams takes ReplannerParams + IntermediateParams. - MotionConstraints, RotationConstraints and RobotConstraints move from rj_common to rj_planning. - Removes dead params: escape.* (unused since main removed RRT) and six collect.* values nothing has read since before this migration. - SettlePathPlanner::invalid() passes intermediate params like the other call sites. - settle_path_planner.hpp includes what it uses (it relied on unity-build batching). - planner_test loads the params from sim_params.yaml exactly like PlannerNode, so it also catches a missing key. The play-state speed cap still replaces max_translational_speed, as it always has, so that param has no effect today (see make_request). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jvogt23
force-pushed
the
params/06-planning
branch
from
October 21, 2026 04:01
1e1a7dc to
9ab8223
Compare
This branch has not been deployed
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.
Description
Removes
rj_param_utilsfromrj_planning.escape.*(unused sincemainremoved RRT).collect.*values nothing has read since before this migration:control_accel_scale,dist_cutoff_to_approach,stop_dist_scale,target_point_lowpass_gain,vel_cutoff_to_controlandvelocity_control_scale.constraints.max_translational_speedhas no effectThe per-play-state speed cap in
make_requestreplaces the max speed rather than capping it, as it always has. So this param is ignored:real_params.yamlsets it to 1.0, but robots actually run at 1.4. This PR keeps that behavior and adds a comment. Switching tomin(configured, cap)would slow real robots to 1.0, so that's a team decision.