Skip to content

Commit a7326b3

Browse files
XuRoboticsclaude
XuRobotics
and
claude
committed
cleanup: fix 5 real bugs flagged by tests/static/check_ros2_port.sh
Run the new static check suite and fix every failure it surfaces. After this commit: 82 checks, 82 passed, 0 failed, 0 skipped. The 5 failures were caught across sections K, L, M, and N of the new bash runner: K. Hardcoded user-specific absolute paths - 6 launch files in autonomy_real/real_experiment_launch hardcoded DeclareLaunchArgument('dir', default_value='/home/dcist/bags/...') as the rosbag record output directory. Replaced with '/tmp/kr_bags/...' defaults — still user-overrideable via the ros2 launch CLI but at least the default exists on any machine. Files: record_bag.launch.py, record_bag_ca_trip.launch.py, record_bag_for_spencer.launch.py, record_bag_max_seam.launch.py, record_bag_atl.launch.py. - real_experiment_launch/util/convert_multicam_calib_to_msckf.py hardcoded /home/dcist/multicam_calibration/calib/example/ as the input folder. Rewrote to accept the path via argparse (--in-folder) or SLIDESLAM_MULTICAM_CALIB_DIR env var, with a ~/multicam_calibration/... default. - fake_sloam/fake_sloam/fake_sloam_node.py hardcoded /home/sam/LRS-SLAM/.../treeposition_nn_radius_5.txt as the `input_file` parameter default. Changed to an empty string default with a prominent comment documenting the required override path. L. Node(executable=...) unresolved in 4 entries - estimation_launch/launch/estimation.launch.py referenced Node(package='fla_ukf', executable='fla_ukf_node') but fla_ukf builds a rclcpp_components composable node, not a standalone executable. Rewrote the launch to use ComposableNodeContainer + ComposableNode with plugin='fla_ukf::FLAUKFNodelet', hosted inside rclcpp_components' component_container executable. - px4_interface_launch/launch/SO3_command_to_mavros.launch.py had the same issue for mavros_interface. Same rewrite: ComposableNodeContainer + ComposableNode with plugin='mavros_interface::SO3CmdToMavros'. - state_machine_launch/launch/replanner.launch.py referenced Node(package='state_machine', executable='path_replanner'). The path_replanner.cpp source was deleted earlier in the port (dead code commented out on master, dependencies no longer exist). Stub the launch file to return an empty LaunchDescription with a docstring pointing at how to reinstate it if someone ports path_replanner. - sim/full_sim_fake_semantic_lidar.launch.py referenced Node(package='fake_sloam', executable='fake_sloam_node'). The executable IS installed via fake_sloam's setup.py console_scripts entry — the static check was blind to ament_python packages. Teach check L to also search setup.py console_scripts as a fallback after the CMakeLists resolution fails. The check's new resolves_in_ament_python_pkg() helper finds the package's setup.py by matching `name='<pkg>'` and then greps the entry_points block for an entry `'<exe> = module:main'`. M. 6 duplicate raw declare_parameter keys - action_trackers/src/{land_tracker, take_off_tracker, trajectory_tracker_upgraded}.cpp each declared the same 6 gain parameters: gains.pos.{x,y,z}, gains.vel.{x,y,z}. All three tracker plugins load into the SAME parent trackers_manager rclcpp::Node, so the second tracker's raw declare_parameter would throw ParameterAlreadyDeclared at runtime. - Fix: new inline header action_trackers/include/action_trackers/gain_params.hpp defining action_trackers::declare_shared_gain_params<NodeLike>(node) that wraps each declare_parameter call in a has_parameter guard. The first tracker to Initialize() declares the parameters; the other trackers' calls become no-ops. Each tracker .cpp now includes the header and calls the helper instead of the 6 raw declare_parameter lines. The templated helper accepts any NodeLike (SharedPtr or raw pointer). N. Shell script ROS1 leftovers - roscore string in autonomy_sim/unity_sim/dcist_utils/script/ unity_sim.sh line 28 — was a false positive inside a tmux send-keys nested-quote string (`echo "ROS2: no roscore needed"` inside `send-keys "..."`), but the lib.sh echo-stub carve-out couldn't see the nested quote. Reworded the echo payload to not contain `roscore` at all. - devel/setup.bash in 9 entrypoint.sh files total — the earlier sweep fixed `source /opt/ros/noetic/setup.bash` -> `source /opt/ros/jazzy/setup.bash` but missed the workspace-level `source /root/<ws>/devel/setup.bash` references. Sweep them all now in autonomy_core/{base,client,control,estimation,interface, map_plan,state_machine}/docker/entrypoint.sh plus autonomy_sim/docker/entrypoint.sh and autonomy_sim/unity_sim/docker/entrypoint.sh. devel/setup.bash -> install/setup.bash in all 9 files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5446d27 commit a7326b3

24 files changed

Lines changed: 238 additions & 131 deletions

File tree

‎autonomy_core/client/docker/entrypoint.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#!/usr/bin/env bash
22

3-
if [ -f /root/client_ws/devel/setup.bash ]; then
3+
if [ -f /root/client_ws/install/setup.bash ]; then
44
echo "Setting up client_ws"
5-
echo "source /root/client_ws/devel/setup.bash" >> /$HOME/.bashrc
5+
echo "source /root/client_ws/install/setup.bash" >> /$HOME/.bashrc
66
source /$HOME/.bashrc
77
fi
88

9-
source /root/client_ws/devel/setup.bash
9+
source /root/client_ws/install/setup.bash
1010

1111
if [ "$#" -eq 0 ]; then
1212
exec bash

‎autonomy_core/control/docker/entrypoint.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#!/bin/bash
22

3-
if [ -f /root/control_ws/devel/setup.bash ]; then
3+
if [ -f /root/control_ws/install/setup.bash ]; then
44
echo "Setting up control_ws"
5-
echo "source /root/control_ws/devel/setup.bash" >> /$HOME/.bashrc
5+
echo "source /root/control_ws/install/setup.bash" >> /$HOME/.bashrc
66
source /$HOME/.bashrc
77
fi
88

9-
source /root/control_ws/devel/setup.bash
9+
source /root/control_ws/install/setup.bash
1010

1111
if [ "$#" -eq 0 ]; then
1212
exec bash

‎autonomy_core/estimation/docker/entrypoint.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#!/bin/bash
22

3-
if [ -f /root/estimation_ws/devel/setup.bash ]; then
3+
if [ -f /root/estimation_ws/install/setup.bash ]; then
44
echo "Setting up estimation_ws"
5-
echo "source /root/estimation_ws/devel/setup.bash" >> /$HOME/.bashrc
5+
echo "source /root/estimation_ws/install/setup.bash" >> /$HOME/.bashrc
66
source /$HOME/.bashrc
77
fi
88

9-
source /root/estimation_ws/devel/setup.bash
9+
source /root/estimation_ws/install/setup.bash
1010

1111
if [ "$#" -eq 0 ]; then
1212
exec bash

‎autonomy_core/estimation/estimation_launch/launch/estimation.launch.py‎

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
from launch.actions import DeclareLaunchArgument, GroupAction
33
from launch.conditions import IfCondition
44
from launch.substitutions import LaunchConfiguration, PathJoinSubstitution, EnvironmentVariable
5-
from launch_ros.actions import Node, PushRosNamespace
5+
from launch_ros.actions import ComposableNodeContainer, Node, PushRosNamespace
6+
from launch_ros.descriptions import ComposableNode
67
from launch_ros.substitutions import FindPackageShare
78

89

@@ -55,37 +56,47 @@ def generate_launch_description():
5556

5657
GroupAction(actions=[
5758
PushRosNamespace(robot),
58-
Node(
59-
package='fla_ukf',
60-
executable='fla_ukf_node',
61-
name='fla_ukf',
62-
output='screen',
63-
parameters=[
64-
ukf_params,
65-
{
66-
'world_frame_id': ukf_ref_frame_id,
67-
'robot_frame_id': robot_frame_id,
68-
'cam_frame_id': vio_imu_frame_id,
69-
'lidar_frame_id': lidar_frame_id,
70-
'enable_vio_odom': enable_vio_odom,
71-
'enable_lidar': enable_lidar,
72-
'vision_frame_id': vio_ref_frame_id,
73-
'enable_laser': False,
74-
'enable_gps': False,
75-
'enable_height': False,
76-
'enable_mag': False,
77-
'enable_yaw': False,
78-
'publish_tf': publish_odom_tf,
79-
},
80-
],
81-
remappings=[
82-
('~/imu', imu),
83-
('~/mag', mag),
84-
('~/height', 'mavros/distance_sensor/lidarlite_pub'),
85-
('~/vio_odom', '/Odometry'),
86-
('~/pose_lidar', '/quadrotor/llol_odom/pose_cov'),
87-
('~/odom_out', output_odom),
59+
# fla_ukf is a rclcpp_components composable node, not a
60+
# standalone executable. Host it in a component_container.
61+
ComposableNodeContainer(
62+
name='fla_ukf_container',
63+
namespace='',
64+
package='rclcpp_components',
65+
executable='component_container',
66+
composable_node_descriptions=[
67+
ComposableNode(
68+
package='fla_ukf',
69+
plugin='fla_ukf::FLAUKFNodelet',
70+
name='fla_ukf',
71+
parameters=[
72+
ukf_params,
73+
{
74+
'world_frame_id': ukf_ref_frame_id,
75+
'robot_frame_id': robot_frame_id,
76+
'cam_frame_id': vio_imu_frame_id,
77+
'lidar_frame_id': lidar_frame_id,
78+
'enable_vio_odom': enable_vio_odom,
79+
'enable_lidar': enable_lidar,
80+
'vision_frame_id': vio_ref_frame_id,
81+
'enable_laser': False,
82+
'enable_gps': False,
83+
'enable_height': False,
84+
'enable_mag': False,
85+
'enable_yaw': False,
86+
'publish_tf': publish_odom_tf,
87+
},
88+
],
89+
remappings=[
90+
('~/imu', imu),
91+
('~/mag', mag),
92+
('~/height', 'mavros/distance_sensor/lidarlite_pub'),
93+
('~/vio_odom', '/Odometry'),
94+
('~/pose_lidar', '/quadrotor/llol_odom/pose_cov'),
95+
('~/odom_out', output_odom),
96+
],
97+
),
8898
],
99+
output='screen',
89100
),
90101
]),
91102

‎autonomy_core/interface/docker/entrypoint.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#!/usr/bin/env bash
22

3-
if [ -f /root/px4_interface_ws/devel/setup.bash ]; then
3+
if [ -f /root/px4_interface_ws/install/setup.bash ]; then
44
echo "Setting up px4_interface_ws"
5-
echo "source /root/px4_interface_ws/devel/setup.bash" >> /$HOME/.bashrc
5+
echo "source /root/px4_interface_ws/install/setup.bash" >> /$HOME/.bashrc
66
source /$HOME/.bashrc
77
fi
88

9-
source /root/px4_interface_ws/devel/setup.bash
9+
source /root/px4_interface_ws/install/setup.bash
1010

1111
if [ "$#" -eq 0 ]; then
1212
exec bash

‎autonomy_core/interface/px4_interface_launch/launch/SO3_command_to_mavros.launch.py‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
from launch import LaunchDescription
22
from launch.actions import DeclareLaunchArgument, GroupAction
33
from launch.substitutions import LaunchConfiguration, EnvironmentVariable
4-
from launch_ros.actions import Node, PushRosNamespace
4+
from launch_ros.actions import ComposableNodeContainer, PushRosNamespace
5+
from launch_ros.descriptions import ComposableNode
56

67

78
def generate_launch_description():
@@ -36,26 +37,37 @@ def generate_launch_description():
3637

3738
GroupAction(actions=[
3839
PushRosNamespace(robot),
39-
Node(
40-
package='mavros_interface',
41-
executable='so3cmd_to_mavros_node',
42-
name='so3cmd_to_mavros',
43-
output='screen',
44-
parameters=[{
45-
'num_props': num_props,
46-
'thrust_vs_rpm_coeff_a': thrust_vs_rpm_coeff_a,
47-
'thrust_vs_rpm_coeff_b': thrust_vs_rpm_coeff_b,
48-
'thrust_vs_rpm_coeff_c': thrust_vs_rpm_coeff_c,
49-
'rpm_vs_throttle_coeff_a': rpm_vs_throttle_coeff_a,
50-
'rpm_vs_throttle_coeff_b': rpm_vs_throttle_coeff_b,
51-
}],
52-
remappings=[
53-
('~/odom', odom),
54-
('~/so3_cmd', so3_cmd),
55-
('~/imu', 'mavros/imu/data'),
56-
('~/attitude_raw', 'mavros/setpoint_raw/attitude'),
57-
('~/odom_pose', 'mavros/vision_pose/pose'),
40+
# mavros_interface's SO3CmdToMavros is a rclcpp_components
41+
# composable node, not a standalone executable. Host it in a
42+
# component_container.
43+
ComposableNodeContainer(
44+
name='so3cmd_to_mavros_container',
45+
namespace='',
46+
package='rclcpp_components',
47+
executable='component_container',
48+
composable_node_descriptions=[
49+
ComposableNode(
50+
package='mavros_interface',
51+
plugin='mavros_interface::SO3CmdToMavros',
52+
name='so3cmd_to_mavros',
53+
parameters=[{
54+
'num_props': num_props,
55+
'thrust_vs_rpm_coeff_a': thrust_vs_rpm_coeff_a,
56+
'thrust_vs_rpm_coeff_b': thrust_vs_rpm_coeff_b,
57+
'thrust_vs_rpm_coeff_c': thrust_vs_rpm_coeff_c,
58+
'rpm_vs_throttle_coeff_a': rpm_vs_throttle_coeff_a,
59+
'rpm_vs_throttle_coeff_b': rpm_vs_throttle_coeff_b,
60+
}],
61+
remappings=[
62+
('~/odom', odom),
63+
('~/so3_cmd', so3_cmd),
64+
('~/imu', 'mavros/imu/data'),
65+
('~/attitude_raw', 'mavros/setpoint_raw/attitude'),
66+
('~/odom_pose', 'mavros/vision_pose/pose'),
67+
],
68+
),
5869
],
70+
output='screen',
5971
),
6072
]),
6173
])

‎autonomy_core/map_plan/docker/entrypoint.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#!/usr/bin/env bash
22

3-
if [ -f /root/map_plan_ws/devel/setup.bash ]; then
3+
if [ -f /root/map_plan_ws/install/setup.bash ]; then
44
echo "Setting up map_plan_ws"
5-
echo "source /root/map_plan_ws/devel/setup.bash" >> /$HOME/.bashrc
5+
echo "source /root/map_plan_ws/install/setup.bash" >> /$HOME/.bashrc
66
source /$HOME/.bashrc
77
fi
88

9-
source /root/map_plan_ws/devel/setup.bash
9+
source /root/map_plan_ws/install/setup.bash
1010

1111
if [ "$#" -eq 0 ]; then
1212
exec bash

‎autonomy_core/state_machine/action_trackers/include/action_trackers/gain_params.hpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Shared position / velocity gain parameter declarations for the
2+
// action_trackers pluginlib tracker plugins.
3+
//
4+
// All tracker plugins (LandTracker, TakeOffTracker, ActionTrajectoryTracker,
5+
// ...) load into the SAME parent trackers_manager rclcpp::Node. In ROS1 the
6+
// individual plugins each did their own `nh_.param("gains/pos/x", ...)` calls
7+
// — safe because nh_.param is idempotent. In ROS2 `declare_parameter()` on
8+
// the same node throws `ParameterAlreadyDeclared` the second time, so this
9+
// helper centralizes the declaration and wraps every call in a
10+
// `has_parameter()` guard. The first tracker to Initialize() declares the
11+
// parameters; subsequent trackers' calls become no-ops and then
12+
// `get_parameter()` returns the value the first tracker (or the yaml
13+
// override) set.
14+
//
15+
// If you are adding a new tracker plugin and you need the gains parameters,
16+
// `#include <action_trackers/gain_params.hpp>` and call
17+
// `action_trackers::declare_shared_gain_params(nh_)` from your Initialize().
18+
19+
#pragma once
20+
21+
namespace action_trackers {
22+
23+
// Templated so we don't care whether `nh_` is a rclcpp::Node::SharedPtr, a
24+
// rclcpp::Node*, or anything else that has -> has_parameter / declare_parameter.
25+
template <typename NodeLike>
26+
inline void declare_shared_gain_params(NodeLike& node) {
27+
if (!node->has_parameter("gains.pos.x")) {
28+
node->declare_parameter("gains.pos.x", 2.5);
29+
}
30+
if (!node->has_parameter("gains.pos.y")) {
31+
node->declare_parameter("gains.pos.y", 2.5);
32+
}
33+
if (!node->has_parameter("gains.pos.z")) {
34+
node->declare_parameter("gains.pos.z", 5.0);
35+
}
36+
if (!node->has_parameter("gains.vel.x")) {
37+
node->declare_parameter("gains.vel.x", 2.2);
38+
}
39+
if (!node->has_parameter("gains.vel.y")) {
40+
node->declare_parameter("gains.vel.y", 2.2);
41+
}
42+
if (!node->has_parameter("gains.vel.z")) {
43+
node->declare_parameter("gains.vel.z", 4.0);
44+
}
45+
}
46+
47+
} // namespace action_trackers

‎autonomy_core/state_machine/action_trackers/src/land_tracker.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <action_trackers/gain_params.hpp>
12
#include <action_trackers/action/land.hpp>
23
#include <rclcpp/rclcpp.hpp>
34
#include <rclcpp_action/rclcpp_action.hpp>
@@ -66,12 +67,7 @@ class LandTracker : public kr_trackers_manager::Tracker {
6667

6768
void LandTracker::Initialize(const rclcpp::Node::SharedPtr &parent_nh) {
6869
nh_ = parent_nh;
69-
nh_->declare_parameter("gains.pos.x", 2.5);
70-
nh_->declare_parameter("gains.pos.y", 2.5);
71-
nh_->declare_parameter("gains.pos.z", 5.0);
72-
nh_->declare_parameter("gains.vel.x", 2.2);
73-
nh_->declare_parameter("gains.vel.y", 2.2);
74-
nh_->declare_parameter("gains.vel.z", 4.0);
70+
action_trackers::declare_shared_gain_params(nh_);
7571
nh_->get_parameter("gains.pos.x", kx_[0]);
7672
nh_->get_parameter("gains.pos.y", kx_[1]);
7773
nh_->get_parameter("gains.pos.z", kx_[2]);

‎autonomy_core/state_machine/action_trackers/src/take_off_tracker.cpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <action_trackers/gain_params.hpp>
12
#include <action_trackers/action/take_off.hpp>
23
#include <rclcpp/rclcpp.hpp>
34
#include <rclcpp_action/rclcpp_action.hpp>
@@ -110,12 +111,7 @@ void TakeOffTracker::halfGains(PositionCommand& cmd) {
110111

111112
void TakeOffTracker::Initialize(const rclcpp::Node::SharedPtr& parent_nh) {
112113
nh_ = parent_nh;
113-
nh_->declare_parameter("gains.pos.x", 2.5);
114-
nh_->declare_parameter("gains.pos.y", 2.5);
115-
nh_->declare_parameter("gains.pos.z", 5.0);
116-
nh_->declare_parameter("gains.vel.x", 2.2);
117-
nh_->declare_parameter("gains.vel.y", 2.2);
118-
nh_->declare_parameter("gains.vel.z", 4.0);
114+
action_trackers::declare_shared_gain_params(nh_);
119115
nh_->get_parameter("gains.pos.x", kx_[0]);
120116
nh_->get_parameter("gains.pos.y", kx_[1]);
121117
nh_->get_parameter("gains.pos.z", kx_[2]);

0 commit comments

Comments
 (0)