Integrate SANEHAL-2 Robot-side bringup - #48
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2082287c40
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR consolidates SANEHAL-2 robot-side bringup so the Raspberry Pi can start drive/TF, JT16, PointCloud2→LaserScan, and slam_toolbox from a single launch file, with explicit support for both real Dynamixel hardware and a mock-hardware validation mode. It also standardizes TF frame ownership by introducing base_footprint as the diff-drive base frame and adds a launch test to validate the mock bringup contract.
Changes:
- Integrated Robot-side launch flow in
sanehal.launch.py, adding component switches, device preflight waits, and RViz default-off on the robot. - Added mock-hardware mode support through ros2_control xacro plumbing and updated TF contract to
odom -> base_footprint -> base_link. - Added/updated tests and documentation to validate and describe the new bringup + 5 Hz LaserScan metadata.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sanehal_vehicle_description/urdf/sanehal.urdf.xacro | Adds base_footprint and mock-hardware/dynamixel args into the robot description. |
| sanehal_vehicle_description/urdf/sanehal_display.urdf.xacro | Mirrors the base_footprint TF structure for display URDF. |
| sanehal_vehicle_description/ros2_control/sanehal.ros2_control.xacro | Switches ros2_control hardware plugin between Dynamixel and mock_components/GenericSystem. |
| sanehal_vehicle_description/controllers/sanehal_controllers.yaml | Updates diff-drive base_frame_id to base_footprint to establish TF ownership. |
| sanehal_vehicle_description/package.xml | Updates runtime dependencies for the description/control stack. |
| sanehal_vehicle_description/README.md | Documents TF ownership responsibilities across SLAM, controller, and state publisher. |
| sanehal_bringup/launch/sanehal.launch.py | Makes sanehal.launch.py the integrated robot entry point with component switches and remaps. |
| sanehal_bringup/launch/sanehal_on_pi.launch.py | Adds device preflight and grouped controller activation; wires mock-hardware args into xacro. |
| sanehal_bringup/launch/jt16.launch.py | Adds serial-device preflight and sim-time propagation to the Hesai driver launch. |
| sanehal_bringup/launch/slam.launch.py | Replaces the prior multi-component SLAM launch with an include of the integrated bringup. |
| sanehal_bringup/config/slam_toolbox_jt16.yaml | Updates slam_toolbox base frame to base_footprint and documents TF ownership. |
| sanehal_bringup/config/pointcloud_to_laserscan_jt16.yaml | Updates LaserScan metadata for measured 5 Hz JT16 stream and TF commentary. |
| sanehal_bringup/test/test_robot_bringup.py | Adds a mock integration launch test verifying controllers, joint states, odom, and TF. |
| sanehal_bringup/test/test_pointcloud_to_laserscan.py | Updates the test contract to match scan_time: 0.2. |
| sanehal_bringup/package.xml | Adds deps for tests and for bringup-to-description package linkage. |
| sanehal_bringup/CMakeLists.txt | Registers the new launch test. |
| sanehal_bringup/README.md | Updates usage docs and the Robot/Operator interface contract. |
| README.md | Updates top-level quickstart to reflect integrated robot bringup and mock mode. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7d2bd367e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
sanehal.launch.pythe single Robot-side entry point for drive/TF, JT16, PointCloud2-to-LaserScan, andslam_toolboxdynamixel_hardware_interfacefor production and add an explicit mock-hardware mode for device-free validationmap -> odom -> base_footprint -> base_link -> hesai_lidarValidation
Automated
colcon build --packages-select sanehal_vehicle_description sanehal_bringupcheck_urdffor production and mock descriptionsros2 launch sanehal_bringup sanehal.launch.py --show-argscolcon test --packages-select sanehal_vehicle_description sanehal_bringupcolcon test-result --verbose: 25 tests, 0 errors, 0 failuresHardware integration
/joint_statesat about 100 Hz and odometry at about 50 Hzslam_toolboxactive and publishing/mapmap -> odom,odom -> base_footprint, and wheel joints; static TF has one publisherMotion and mapping
With the robot first raised off the floor:
cmd_vel_timeout: wheels settled to zero about 0.75 s after command publication stoppedOn the floor at 0.08 m/s and 0.25 rad/s:
Follow-up
Long-duration Raspberry Pi stability, temperature, controller-overrun, and packet-loss testing is tracked in #47.
Closes #27