Add realsense docker module and include examples#99
Add realsense docker module and include examples#99yuhsiang1117 wants to merge 14 commits intoj3soon:mainfrom
Conversation
WalkthroughAdds an optional RealSense Docker module and integrates new ROS2 packages, Gazebo models/plugins, launch files, and documentation to provide simulated and hardware RealSense D435 support in the workspace and container build. Changes
Sequence Diagram(s)sequenceDiagram
participant Dockerfile
participant install_script as /tmp/install_realsense.sh
participant apt as apt-get
Note over Dockerfile,install_script: Build-time flow
Dockerfile->>install_script: COPY & RUN (with ARG REALSENSE)
install_script-->>Dockerfile: if REALSENSE != "YES": exit 0
install_script->>apt: apt-get update && apt-get install realsense & ROS packages
apt-->>install_script: packages installed
install_script-->>Dockerfile: cleanup & exit
sequenceDiagram
participant Gazebo
participant Plugin as GazeboRosRealsense
participant ROS as rclcpp/node
participant RViz
participant PointCloudTransport as pct
Note over Gazebo,ROS: Runtime sensor flow
Gazebo->>Plugin: rendering frames (depth/color/ir)
Plugin->>ROS: publish sensor_msgs::Image + CameraInfo
Plugin->>PointCloudTransport: publish PointCloud2 (if enabled)
ROS->>RViz: RViz subscribes to images/pointclouds and visualizes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
YuZhong-Chen
left a comment
YuZhong-Chen
left a comment
There was a problem hiding this comment.
This is my first time doing a full PR review. If anything is unclear, please don't hesitate to let me know!
Also, please follow the PR guidelines when submitting changes:
Attribution: If you're copying code from elsewhere, please include the source link in the commit message or comments (or both).Atomic Commits: If you need to modify copied code, please split the copy and the modifications into separate commits. This makes the review process much easier for us.Credit: If this commit was done collaboratively or you want to give credit, remember to add aCo-authored-bytag.
template_ws/src/realsense_example/realsense_desp/models/turtlebot3_burger/model.config
Show resolved
Hide resolved
template_ws/src/realsense_example/realsense_desp/urdf/turtlebot3_burger.urdf
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo/launch/realsense_gazebo.launch.py
Show resolved
Hide resolved
...ate_ws/src/realsense_examples/realsense_description/models/turtlebot3_burger/meshes/d435.dae
Show resolved
Hide resolved
|
Self-note: Isaac Sim actually provides a USD asset for RealSense, so we can plan to support Isaac Sim in the future. I've used it in other repo before, so it should be pretty straightforward. |
|
Even though the cache should already be created (since I've launched |
|
If the package is available via user@yuzhong-desktop:/home/ros2-essentials/template_ws$ colcon build --symlink-install
[0.153s] WARNING:colcon.colcon_core.package_selection:Some selected packages are already built in one or more underlay workspaces:
'realsense2_camera' is in: /home/ros2-essentials/template_ws/install/realsense2_camera, /opt/ros/humble
If a package in a merged underlay workspace is overridden and it installs headers, then all packages in the overlay must sort their include directories by workspace order. Failure to do so may result in build failures or undefined behavior at run time.
If the overridden package is used by another package in any underlay, then the overriding package in the overlay must be API and ABI compatible or undefined behavior at run time may occur.
If you understand the risks and want to override a package anyways, add the following to the command line:
--allow-overriding realsense2_camera |
|
I haven't had a chance to run this on a physical RealSense camera yet. I'll probably wait until these comments are fixed to do a full test, but I might jump in earlier if time permits. |
yuhsiang1117
force-pushed
the
feat/realsense
branch
3 times, most recently
from
b83a48f to
80ebee7
Compare
- add install_realsense.sh under docker_modules/
- update Dockerfile and compose.yaml to let user decide installation via ARG REALSENSE
- add device_cgroup_rule in compose.yaml for realsense camera
- ref: https://github.com/2b-t/realsense-ros2-docker
yuhsiang1117
force-pushed
the
feat/realsense
branch
from
80ebee7 to
f88ac49
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In `@docker_modules/install_realsense.sh`:
- Around line 20-21: Remove the trailing whitespace after the line-continuation
backslash in the apt install line that includes "ros-${ROS_DISTRO}-launch-pytest
\", ensuring the backslash is the last character so the next "&& sudo rm -rf
/var/lib/apt/lists/*" is properly continued; also verify whether the package
"ros-${ROS_DISTRO}-launch-pytest" actually exists in your ROS distro and either
update/remove that package token if it's not available (so the apt install
command succeeds) — look for the apt install command containing
"ros-${ROS_DISTRO}-launch-pytest" to apply the fix.
In `@docs/docker-modules/realsense.md`:
- Line 3: Update the badge hyperlink so it points to the RealSense installer
script instead of the Cartographer script: replace the current URL
"https://github.com/j3soon/ros2-essentials/blob/main/docker_modules/install_cartographer.sh"
in the badge markdown with
"https://github.com/j3soon/ros2-essentials/blob/main/docker_modules/install_realsense.sh"
so the badge links to install_realsense.sh.
- Around line 47-48: The docs reference a non-existent package name
realsense_desp while the actual directory/package is named
realsense_description; update the documentation text to use
realsense_description (or rename the directory/package to realsense_desp where
appropriate) so names are consistent across the project, ensuring both the
listing and any other mentions use the same identifier (realsense_description)
and update related entries like realsense_gazebo if needed.
In `@template_ws/src/realsense_examples/realsense_description/CMakeLists.txt`:
- Around line 1-2: The package name in CMakeLists.txt is wrong and mismatches
other packages: change project(realsense_desp) to
project(realsense_description); then update package.xml by changing the <name>
element from realsense_desp to realsense_description; rename the launch file
launch/realsense_desp.launch.py to launch/realsense_description.launch.py; and
inside that launch file replace any occurrences of the package identifier
realsense_desp (e.g., node/package references at lines referenced in the review)
with realsense_description so all external references (like from
realsense_gazebo) resolve correctly.
In
`@template_ws/src/realsense_examples/realsense_description/launch/realsense_desp.launch.py`:
- Around line 25-35: The package name is inconsistent: change the hardcoded
package identifier from "realsense_desp" to the correct package name
"realsense_description" wherever it's used (e.g., the FindPackageShare call that
constructs burger_model_path and the get_package_share_directory call used in
urdf_path) so both burger_model_path and urdf_path reference the same, actual
package name; update both occurrences to "realsense_description".
In `@template_ws/src/realsense_examples/realsense_description/package.xml`:
- Line 4: The package.xml currently sets the package name to "realsense_desp"
which does not match the directory; update the <name> element to
"realsense_description" to match the directory and mirror the same fix applied
in CMakeLists.txt (ensure any references to package name inside package.xml and
related manifest metadata use "realsense_description"). Verify consistency
across package.xml and CMakeLists.txt so the package name is identical in both
places.
In `@template_ws/src/realsense_examples/realsense_gazebo_plugin/README.md`:
- Around line 16-18: Update the README to reflect the correct ROS distro used by
the project (replace "ROS Foxy" with the Dockerfile default "Humble") and add a
clear note that any references to external robot packages like "chiconybot" and
"chiconybot_description" are placeholders; instruct users to replace those paths
in their model.sdf with their own robot package names or provide instructions
how to point to external assets. Also add a short example or sentence showing
how to change the model.sdf plugin/link/sensor paths to a user-provided package
name so readers know these assets are not bundled.
In
`@template_ws/src/realsense_examples/realsense_gazebo_plugin/src/gazebo_ros_realsense.cpp`:
- Around line 149-152: The iterators are being constructed from the class member
pointcloud_msg_ instead of the function parameter point_cloud_msg; update the
PointCloud2Iterator<> constructions (iter_x, iter_y, iter_z, iter_rgb) to use
point_cloud_msg so they operate on the same message as the PointCloud2Modifier,
keeping the field names ("x","y","z","rgb") and template types unchanged and
ensuring no other references still use pointcloud_msg_.
- Around line 45-57: The ROS initialization check is done after creating the
node (node_ via rclcpp::Node::make_shared), which can fail; move the
rclcpp::ok() check to occur before calling rclcpp::Node::make_shared(node_name).
Specifically, call rclcpp::ok() first, and if it returns false use RCLCPP_ERROR
(with the same message) and return without constructing node_; only then create
node_ with rclcpp::Node::make_shared. Keep using node_->get_logger() replaced by
a static/global logger or defer logger usage until after node_ is created to
avoid calling get_logger() on a null/uninitialized node.
In
`@template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp`:
- Line 121: Fix the typo in the thrown exception message in RealSensePlugin.cpp:
update the std::runtime_error call that currently reads "Ivalid parameter for
RealSensePlugin" to the correct spelling "Invalid parameter for RealSensePlugin"
so the error text is accurate when thrown from the RealSensePlugin
constructor/initializer path.
- Around line 31-39: The constructor RealSensePlugin() leaves several members
uninitialized; initialize rangeMinDepth_, rangeMaxDepth_, pointCloudCutOff_,
depthUpdateRate_, colorUpdateRate_, and infraredUpdateRate_ with safe defaults
(e.g., rangeMinDepth_ = 0.1, rangeMaxDepth_ = 5.0, pointCloudCutOff_ = 0.0 or
matching pointCloudCutOffMax_, and reasonable rates like 30.0 or 0.0 to indicate
disabled) inside RealSensePlugin() so OnNewDepthFrame() and publisher setup
never read garbage values; update the constructor that currently sets depthCam,
ired1Cam, ired2Cam, colorCam, prefix, pointCloudCutOffMax_ to also set these
members to the chosen defaults.
- Around line 279-285: The step is incorrectly using depthCam->ImageDepth()
(bytes-per-pixel for the original camera) while depthMap actually holds uint16_t
values; update the image step to use depthCam->ImageWidth() *
sizeof(this->depthMap[0]) (or sizeof(uint16_t)) so it matches the actual buffer
element size, and likewise ensure the set_data size uses
sizeof(this->depthMap[0]) * imageSize rather than relying on ImageDepth();
modify the calls around msg.mutable_image()->set_step(...) and
msg.mutable_image()->set_data(...) to use sizeof(this->depthMap[0]) to fix the
mismatch.
- Around line 137-149: The code is dereferencing the result of
std::dynamic_pointer_cast and calling DepthCamera()/Camera() without checking
the cast succeeded; introduce temporary sensor pointers (e.g., auto depthSensor
= smanager->GetSensor(prefix + DEPTH_CAMERA_NAME); auto depthCamPtr =
std::dynamic_pointer_cast<sensors::DepthCameraSensor>(depthSensor)) and only
call depthCamPtr->DepthCamera() if depthCamPtr is non-null (similarly for
ired1/ired2/color using CameraSensor and Camera()); if any cast returns null,
log an error/handle gracefully (return or set the member to nullptr) before the
later null-checks to avoid dereferencing null.
In
`@template_ws/src/realsense_examples/realsense_gazebo/launch/realsense_gazebo.launch.py`:
- Around line 33-37: The include path for the robot description is incorrect:
update the PathJoinSubstitution used to build realsense_description_path so it
references the actual file name realsense_desp.launch.py instead of
realsense_description.launch.py; locate the realsense_description_path variable
(the PathJoinSubstitution/FindPackageShare expression) and change the file
component to "realsense_desp.launch.py" so the launch includes the correct file.
In `@template_ws/src/realsense_examples/realsense_gazebo/package.xml`:
- Around line 10-17: The package.xml is missing runtime exec_depend entries for
packages referenced by the launch file; add exec_depend entries for
turtlebot3_gazebo, gazebo_ros, realsense_description, and rviz2 so they are
installed at runtime. Update package.xml to include <exec_depend> entries for
each of those four packages (turtlebot3_gazebo, gazebo_ros,
realsense_description, rviz2) alongside the existing buildtool/test deps and
export block so FindPackageShare/get_package_share_directory and rviz2 usage
resolve at runtime.
🧹 Nitpick comments (10)
template_ws/src/realsense_examples/realsense_gazebo_plugin/CONTRIBUTING.md (1)
5-12: Consider adding a language specifier to the fenced code block.Static analysis flagged this block as missing a language identifier. Since this is license text, you could use
textor leave it as-is since it's not actual code.Proposed fix
-~~~ +~~~text 5. Submission of Contributions. Unless You explicitly state otherwise,docker_modules/install_realsense.sh (1)
4-11: Redundant condition check.The script checks
if [ -z "$REALSENSE" ]at line 4, then checksif [ "$REALSENSE" = "YES" ]at line 11. IfREALSENSEis set to something other than "YES" (e.g., "yes" or "1"), the script will skip both installation and the early exit, then print "installation completed successfully" without actually installing anything.Consider simplifying to a single check or using case-insensitive matching.
Proposed fix using case-insensitive check
#!/bin/bash set -e -if [ -z "$REALSENSE" ]; then +# Convert to uppercase for case-insensitive comparison +REALSENSE_UPPER=$(echo "${REALSENSE:-}" | tr '[:lower:]' '[:upper:]') + +if [ "$REALSENSE_UPPER" != "YES" ]; then echo "Skipping RealSense installation as REALSENSE is not set" exit 0 fi # Install RealSense, TurtleBot3 packages and rqt steering # This script is intended to be run inside the Dockerfile during build. -if [ "$REALSENSE" = "YES" ]; then - echo "Installing RealSense ros packages for ROS distro: ${ROS_DISTRO:-humble}" +echo "Installing RealSense ros packages for ROS distro: ${ROS_DISTRO:-humble}" - sudo apt-get update && sudo apt-get install -y \ - ... - && sudo rm -rf /var/lib/apt/lists/* -fi +sudo apt-get update && sudo apt-get install -y \ + ... + && sudo rm -rf /var/lib/apt/lists/* echo "RealSense installation completed successfully!"template_ws/docker/Dockerfile (1)
164-169: Gate RealSense install on the build arg.
Running the script unconditionally can add build time when REALSENSE is empty; make the intent explicit by guarding the step.♻️ Proposed update
RUN --mount=type=cache,target=/var/cache/apt,sharing=private \ - /tmp/install_realsense.sh && rm /tmp/install_realsense.sh + if [ "$REALSENSE" = "YES" ]; then /tmp/install_realsense.sh; fi \ + && rm /tmp/install_realsense.shtemplate_ws/docker/compose.yaml (1)
114-115: Scope USB access instead of mounting full/dev.
Mounting/dev:/devwhileprivileged: truegrants broad device access and makesdevice_cgroup_rulesineffective. If RealSense is the main requirement, bind‑mount only/dev/bus/usb(and add explicit/dev/videoNif needed) so the rules remain meaningful.♻️ Proposed update
- - /dev:/dev + - /dev/bus/usb:/dev/bus/usbAlso applies to: 184-187
template_ws/src/realsense_examples/realsense_gazebo_plugin/CMakeLists.txt (1)
14-16: Remove unused FastRTPS variables with hardcoded Foxy paths.These variables are never referenced in the CMakeLists.txt and the hardcoded
/opt/ros/foxypaths are incorrect for the Humble environment (as configured in the Dockerfile). Remove them to eliminate dead code and confusion.♻️ Cleanup
-set(FastRTPS_INCLUDE_DIR /opt/ros/foxy/include) -set(FastRTPS_LIBRARY_RELEASE /opt/ros/foxy/lib/libfastrtps.so)template_ws/src/realsense_examples/realsense_gazebo_plugin/include/realsense_gazebo_plugin/RealSensePlugin.hpp (2)
32-35: Consider using constexpr instead of macros for camera name constants.Macros defined inside a namespace still have global scope. Using
constexprprovides type safety and proper scoping.♻️ Suggested refactor
-#define DEPTH_CAMERA_NAME "depth" -#define COLOR_CAMERA_NAME "color" -#define IRED1_CAMERA_NAME "ired1" -#define IRED2_CAMERA_NAME "ired2" +constexpr const char* DEPTH_CAMERA_NAME = "depth"; +constexpr const char* COLOR_CAMERA_NAME = "color"; +constexpr const char* IRED1_CAMERA_NAME = "ired1"; +constexpr const char* IRED2_CAMERA_NAME = "ired2";
133-142: Consider adding default initializers for all numeric members.
pointCloud_has an inline initializer, but the other numeric members do not. Adding default values here would prevent undefined behavior if the constructor or SDF parsing misses them.♻️ Suggested inline initializers
bool pointCloud_ = false; std::string pointCloudTopic_; - double pointCloudCutOff_, pointCloudCutOffMax_; + double pointCloudCutOff_ = 0.0; + double pointCloudCutOffMax_ = 5.0; - double colorUpdateRate_; - double infraredUpdateRate_; - double depthUpdateRate_; + double colorUpdateRate_ = 30.0; + double infraredUpdateRate_ = 30.0; + double depthUpdateRate_ = 30.0; - float rangeMinDepth_; - float rangeMaxDepth_; + float rangeMinDepth_ = 0.0f; + float rangeMaxDepth_ = 10.0f;template_ws/src/realsense_examples/realsense_gazebo_plugin/include/realsense_gazebo_plugin/gazebo_ros_realsense.hpp (2)
37-92: Redundant access specifier declarations throughout the class.Multiple
public:andprotected:specifiers are scattered throughout. This is syntactically valid but unconventional and reduces readability. Consider grouping all public members together, followed by protected, then private.
70-72: Consider usingstd::shared_ptrinstead ofboost::shared_ptr.ROS 2 prefers
std::shared_ptr. Usingboost::shared_ptrhere is inconsistent with thestd::unique_ptron line 81 andrclcpp::Node::SharedPtron line 78.♻️ Suggested change
- boost::shared_ptr<camera_info_manager::CameraInfoManager> - camera_info_manager_; + std::shared_ptr<camera_info_manager::CameraInfoManager> + camera_info_manager_;template_ws/src/realsense_examples/realsense_gazebo_plugin/src/gazebo_ros_realsense.cpp (1)
279-294: Consider throwing or returning an optional instead of silently defaulting.When an unknown camera name is encountered, the function logs an error but returns
COLOR_CAMERA_NAMEas a fallback. This could mask configuration errors and lead to confusing behavior where one camera's data appears on another's topic.♻️ Alternative: throw on unknown camera
if (name.find(IRED2_CAMERA_NAME) != std::string::npos) { return IRED2_CAMERA_NAME; } RCLCPP_ERROR(rclcpp::get_logger("realsense_camera"), "Unknown camera name"); - - return COLOR_CAMERA_NAME; + throw std::runtime_error("Unknown camera name: " + name); }
template_ws/src/realsense_examples/realsense_description/CMakeLists.txt
Outdated
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_description/launch/realsense_description.launch.py
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo/launch/realsense_gazebo.launch.py
Show resolved
Hide resolved
yuhsiang1117
force-pushed
the
feat/realsense
branch
from
f88ac49 to
1b55a90
Compare
…space
- add turtlebot3_burger.urdf and model.sdf
- ref: https://github.com/ROBOTIS-GIT/turtlebot3_simulations/tree/humble
yuhsiang1117
force-pushed
the
feat/realsense
branch
3 times, most recently
from
b700e2d to
11add30
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docker_modules/install_realsense.sh`:
- Line 24: The success message "RealSense installation completed successfully!"
is printed unconditionally; update install_realsense.sh to only print that
message when the REALSENSE installation branch actually runs by moving the echo
into the block guarded by the REALSENSE check (the conditional that tests
REALSENSE == "YES") or by adding an explicit conditional around the echo that
checks REALSENSE == "YES"; ensure you reference the REALSENSE variable and the
success message string so the message is emitted only after the installation
steps complete.
In `@docs/docker-modules/realsense.md`:
- Line 59: Fix the typo in the section heading by changing the heading text
"Realsense test in realworld" to "Realsense test in real world" in the
docs/realsense markdown (update the heading string exactly where "Realsense test
in realworld" appears).
In
`@template_ws/src/realsense_examples/realsense_description/urdf/turtlebot3_burger.urdf`:
- Line 59: The package.xml is missing runtime/build dependencies referenced by
the URDF: add explicit <depend> entries for the packages used by the URDF
(turtlebot3_gazebo and realsense2_description) so ROS tools can resolve
package:// URIs; open
template_ws/src/realsense_examples/realsense_description/package.xml and add
<depend>turtlebot3_gazebo</depend> and <depend>realsense2_description</depend>
in the existing <depend> section to declare those dependencies for the
realsense_description package.
♻️ Duplicate comments (8)
template_ws/src/realsense_examples/realsense_description/package.xml (1)
4-4: Package name mismatch with directory name.The package name
realsense_despdoesn't match the directory namerealsense_description. This inconsistency will cause issues when referencing the package in launch files, dependencies, orros2 pkgcommands.Proposed fix
- <name>realsense_desp</name> + <name>realsense_description</name>docker_modules/install_realsense.sh (1)
20-21: Critical: Trailing whitespace after backslash breaks shell parsing.Line 20 has a trailing space after the backslash, which breaks line continuation and causes shell parsing errors. The
fion line 22 won't be recognized, failing the Docker build.Proposed fix
sudo apt-get update && sudo apt-get install -y \ ros-${ROS_DISTRO}-point-cloud-transport \ ros-${ROS_DISTRO}-librealsense2* \ ros-${ROS_DISTRO}-realsense2-* \ ros-${ROS_DISTRO}-turtlebot3* \ ros-${ROS_DISTRO}-rqt-robot-steering \ - ros-${ROS_DISTRO}-launch-pytest \ - && sudo rm -rf /var/lib/apt/lists/* + && sudo rm -rf /var/lib/apt/lists/*Or if
launch-pytestis required, simply remove the trailing space:- ros-${ROS_DISTRO}-launch-pytest \ + ros-${ROS_DISTRO}-launch-pytest \template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp (4)
31-39: Initialize all configuration members to safe defaults.Several config fields are read even when SDF keys are absent; leaving them uninitialized can cause undefined behavior.
🐛 Proposed fix
RealSensePlugin::RealSensePlugin() { this->depthCam = nullptr; this->ired1Cam = nullptr; this->ired2Cam = nullptr; this->colorCam = nullptr; this->prefix = ""; this->pointCloudCutOffMax_ = 5.0; + this->pointCloudCutOff_ = 0.0; + this->rangeMinDepth_ = 0.0f; + this->rangeMaxDepth_ = 10.0f; + this->depthUpdateRate_ = 30.0; + this->colorUpdateRate_ = 30.0; + this->infraredUpdateRate_ = 30.0; }
121-121: Fix typo in exception message.✏️ Proposed fix
- throw std::runtime_error("Ivalid parameter for RealSensePlugin"); + throw std::runtime_error("Invalid parameter for RealSensePlugin");
137-149: Guard sensor casts before dereference.
dynamic_pointer_castcan return null; dereferencing before checks can crash.🐛 Proposed fix
- this->depthCam = std::dynamic_pointer_cast<sensors::DepthCameraSensor>( - smanager->GetSensor(prefix + DEPTH_CAMERA_NAME)) - ->DepthCamera(); - - this->ired1Cam = std::dynamic_pointer_cast<sensors::CameraSensor>( - smanager->GetSensor(prefix + IRED1_CAMERA_NAME)) - ->Camera(); - this->ired2Cam = std::dynamic_pointer_cast<sensors::CameraSensor>( - smanager->GetSensor(prefix + IRED2_CAMERA_NAME)) - ->Camera(); - this->colorCam = std::dynamic_pointer_cast<sensors::CameraSensor>( - smanager->GetSensor(prefix + COLOR_CAMERA_NAME)) - ->Camera(); + auto depthSensor = std::dynamic_pointer_cast<sensors::DepthCameraSensor>( + smanager->GetSensor(prefix + DEPTH_CAMERA_NAME)); + if (depthSensor) { + this->depthCam = depthSensor->DepthCamera(); + } + + auto ired1Sensor = std::dynamic_pointer_cast<sensors::CameraSensor>( + smanager->GetSensor(prefix + IRED1_CAMERA_NAME)); + if (ired1Sensor) { + this->ired1Cam = ired1Sensor->Camera(); + } + + auto ired2Sensor = std::dynamic_pointer_cast<sensors::CameraSensor>( + smanager->GetSensor(prefix + IRED2_CAMERA_NAME)); + if (ired2Sensor) { + this->ired2Cam = ired2Sensor->Camera(); + } + + auto colorSensor = std::dynamic_pointer_cast<sensors::CameraSensor>( + smanager->GetSensor(prefix + COLOR_CAMERA_NAME)); + if (colorSensor) { + this->colorCam = colorSensor->Camera(); + }
279-285: Depth image step must matchuint16_tdepthMap.
ImageDepth()reflects the float depth stream; the published buffer isuint16_t.🐛 Proposed fix
msg.mutable_image()->set_step( - this->depthCam->ImageWidth() * - this->depthCam->ImageDepth()); + this->depthCam->ImageWidth() * + sizeof(*this->depthMap.data()));template_ws/src/realsense_examples/realsense_gazebo_plugin/src/gazebo_ros_realsense.cpp (2)
40-57: Checkrclcpp::ok()before creating the node.Node creation can throw if ROS isn’t initialized; guard first.
🐛 Proposed fix
void GazeboRosRealsense::Load(physics::ModelPtr _model, sdf::ElementPtr _sdf) { + // Make sure the ROS node for Gazebo has already been initialized + if (!rclcpp::ok()) { + RCLCPP_ERROR( + rclcpp::get_logger("realsense_gazebo_plugin"), + "A ROS node for Gazebo has not been initialized, unable " + "to load plugin. " + "Load the Gazebo system plugin " + "'libgazebo_ros_api_plugin.so' in the gazebo_ros " + "package"); + return; + } + RealSensePlugin::Load(_model, _sdf); std::string node_name = "gazebo_realsense"; node_name += this->prefix.empty() ? "" : "_" + this->prefix; this->node_ = rclcpp::Node::make_shared(node_name); - - // Make sure the ROS node for Gazebo has already been initialized - if (!rclcpp::ok()) { - RCLCPP_ERROR( - node_->get_logger(), - "A ROS node for Gazebo has not been initialized, unable " - "to load plugin. " - "Load the Gazebo system plugin " - "'libgazebo_ros_api_plugin.so' in the gazebo_ros " - "package"); - return; - } RCLCPP_INFO(node_->get_logger(), "Realsense Gazebo ROS plugin loading.");
149-152: Use the function parameter for PointCloud2 iterators.Iterators currently bind to the member, which may diverge from the argument.
🐛 Proposed fix
- sensor_msgs::PointCloud2Iterator<float> iter_x(pointcloud_msg_, "x"); - sensor_msgs::PointCloud2Iterator<float> iter_y(pointcloud_msg_, "y"); - sensor_msgs::PointCloud2Iterator<float> iter_z(pointcloud_msg_, "z"); - sensor_msgs::PointCloud2Iterator<uint8_t> iter_rgb(pointcloud_msg_, "rgb"); + sensor_msgs::PointCloud2Iterator<float> iter_x(point_cloud_msg, "x"); + sensor_msgs::PointCloud2Iterator<float> iter_y(point_cloud_msg, "y"); + sensor_msgs::PointCloud2Iterator<float> iter_z(point_cloud_msg, "z"); + sensor_msgs::PointCloud2Iterator<uint8_t> iter_rgb(point_cloud_msg, "rgb");
🧹 Nitpick comments (6)
template_ws/src/realsense_examples/realsense_gazebo/rviz/realsense.rviz (1)
242-261: Window geometry contains machine-specific values.The window position (
X: 255,Y: 133) and dimensions (Width: 1200,Height: 846) are specific to the development machine. This is typical for RViz configs and generally works fine, but users on different screen resolutions may see suboptimal initial layouts. This is a minor ergonomic consideration and not a blocking issue.template_ws/src/realsense_examples/realsense_description/models/turtlebot3_burger/model.sdf (3)
200-200: Minor typo in comment."pamareter" should be "parameter".
Proposed fix
- <!-- This friction pamareter don't contain reliable data!! --> + <!-- This friction parameter doesn't contain reliable data!! -->
262-262: Same typo repeated.Proposed fix
- <!-- This friction pamareter don't contain reliable data!! --> + <!-- This friction parameter doesn't contain reliable data!! -->
445-456: Extremely small mass value may cause physics instability.The mass of
0.000001kg for the RealSense link is unrealistically small and could cause numerical instability in Gazebo's physics simulation. The actual Intel RealSense D435 weighs approximately 72 grams (0.072 kg).Proposed fix
<inertial> <pose>0 0 0 0 0 0</pose> <inertia> <ixx>0.001</ixx> <ixy>0.000</ixy> <ixz>0.000</ixz> <iyy>0.001</iyy> <iyz>0.000</iyz> <izz>0.001</izz> </inertia> - <mass>0.000001</mass> + <mass>0.072</mass> </inertial>template_ws/src/realsense_examples/realsense_description/urdf/turtlebot3_burger.urdf (2)
2-4: Consider removing unused xacro namespace.The file declares an
xmlns:xacronamespace but doesn't use any xacro features (macros, includes, properties). The commented-out include on line 4 suggests this may have been intentional during development. Consider either:
- Removing the xacro namespace declaration for a cleaner plain URDF, or
- Uncommenting and utilizing the include if the common properties are needed
Option 1: Plain URDF
<?xml version="1.0" ?> -<robot name="turtlebot3_burger" - xmlns:xacro="http://ros.org/wiki/xacro"> - <!-- <xacro:include filename="$(find turtlebot3_description)/urdf/common_properties.urdf"/> --> +<robot name="turtlebot3_burger">
196-209: Using mesh for collision geometry may impact performance.Using the detailed
.daemesh for collision geometry (line 203-207) can significantly slow down physics simulation. Consider using a simplified box or primitive shape for the collision geometry instead.Proposed fix
<link name="camera_bottom_screw_frame"> <visual> <origin xyz="0 0 0" rpy="1.57 0 1.57"/> <geometry> <mesh filename="package://realsense2_description/meshes/d435.dae" /> </geometry> </visual> <collision> <origin xyz="0 0 0" rpy="1.57 0 1.57"/> <geometry> - <mesh filename="package://realsense2_description/meshes/d435.dae" /> + <box size="0.025 0.090 0.025"/> </geometry> </collision> </link>
| && sudo rm -rf /var/lib/apt/lists/* | ||
| fi | ||
|
|
||
| echo "RealSense installation completed successfully!" |
There was a problem hiding this comment.
Success message prints even when installation was skipped.
The "RealSense installation completed successfully!" message will print even when REALSENSE is set but not equal to "YES" (e.g., REALSENSE=NO). Consider moving this message inside the installation block or adjusting the logic.
Proposed fix
if [ "$REALSENSE" = "YES" ]; then
echo "Installing RealSense ros packages for ROS distro: ${ROS_DISTRO:-humble}"
sudo apt-get update && sudo apt-get install -y \
ros-${ROS_DISTRO}-point-cloud-transport \
ros-${ROS_DISTRO}-librealsense2* \
ros-${ROS_DISTRO}-realsense2-* \
ros-${ROS_DISTRO}-turtlebot3* \
ros-${ROS_DISTRO}-rqt-robot-steering \
&& sudo rm -rf /var/lib/apt/lists/*
+
+ echo "RealSense installation completed successfully!"
fi
-
-echo "RealSense installation completed successfully!"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "RealSense installation completed successfully!" | |
| if [ "$REALSENSE" = "YES" ]; then | |
| echo "Installing RealSense ros packages for ROS distro: ${ROS_DISTRO:-humble}" | |
| sudo apt-get update && sudo apt-get install -y \ | |
| ros-${ROS_DISTRO}-point-cloud-transport \ | |
| ros-${ROS_DISTRO}-librealsense2* \ | |
| ros-${ROS_DISTRO}-realsense2-* \ | |
| ros-${ROS_DISTRO}-turtlebot3* \ | |
| ros-${ROS_DISTRO}-rqt-robot-steering \ | |
| && sudo rm -rf /var/lib/apt/lists/* | |
| echo "RealSense installation completed successfully!" | |
| fi |
🤖 Prompt for AI Agents
In `@docker_modules/install_realsense.sh` at line 24, The success message
"RealSense installation completed successfully!" is printed unconditionally;
update install_realsense.sh to only print that message when the REALSENSE
installation branch actually runs by moving the echo into the block guarded by
the REALSENSE check (the conditional that tests REALSENSE == "YES") or by adding
an explicit conditional around the echo that checks REALSENSE == "YES"; ensure
you reference the REALSENSE variable and the success message string so the
message is emitted only after the installation steps complete.
There was a problem hiding this comment.
Minor: This could be improved/fixed as well. It doesn't affect the functionality, but it would be nice to have.
There was a problem hiding this comment.
@YuZhong-Chen, thanks for acknowledging this! I agree it's a minor improvement that would make the output more accurate. The fix is straightforward—just moving the success message inside the if [ "$REALSENSE" = "YES" ] block as suggested in the diff above. This way, the message will only appear when the installation actually runs. Feel free to apply it when convenient! 👍
🧠 Learnings used
Learnt from: CR
Repo: j3soon/ros2-essentials PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T17:35:09.442Z
Learning: `docker_modules/` holds shared install scripts that are hard-linked into each workspace via `./scripts/post_install.sh`
Learnt from: CR
Repo: j3soon/ros2-essentials PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-29T17:34:59.425Z
Learning: Docker module scripts in `docker_modules/` must be designed as reusable install scripts that can be hard-linked into workspace `docker/modules/` directories
Learnt from: CR
Repo: j3soon/ros2-essentials PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-29T17:34:59.425Z
Learning: Applies to **/docker/Dockerfile : Dockerfile definitions must support multi-architecture builds (amd64 for simulation+real, arm64 for real only) and reference `modules/` scripts from docker_modules
Learnt from: CR
Repo: j3soon/ros2-essentials PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-29T17:34:59.425Z
Learning: Always run `./scripts/post_install.sh` after cloning, pulling, or switching branches to set up docker modules
| ros2 run rqt_robot_steering rqt_robot_steering | ||
| ``` | ||
|
|
||
| ## Realsense test in realworld |
There was a problem hiding this comment.
Minor typo in section heading.
"realworld" should be "real world" (two words).
Proposed fix
-## Realsense test in realworld
+## Realsense test in real world🤖 Prompt for AI Agents
In `@docs/docker-modules/realsense.md` at line 59, Fix the typo in the section
heading by changing the heading text "Realsense test in realworld" to "Realsense
test in real world" in the docs/realsense markdown (update the heading string
exactly where "Realsense test in realworld" appears).
There was a problem hiding this comment.
Minor: Please fix it.
template_ws/src/realsense_examples/realsense_description/urdf/turtlebot3_burger.urdf
Show resolved
Hide resolved
- modify turtlebot3_burger.urdf and model.sdf for realsense camera simulation in gazebo
- add realsense_desp.launch.py
- add realsense gazebo.launch.py to spawn turtlebot3 burger with realsense in gazebo
- add rviz configuration for realsense visualization
- add realsense_gazebo_plugin package for Gazebo simulation of RealSense cameras.
- add realsense_ros package for ROS integration with RealSense cameras.
- add realsense.md in docs/docker-modules/
- fix install_realsense.sh permissions to be executable
- add rviz_config variable to specify the path to the rviz config file
- update rviz node arguments to use the rviz_config variable instead of hardcoded path
Co-authored-by: YuZhong-Chen <yuzhong1214@gmail.com>
- adjusted the pose of the Realsense D435 camera in both the SDF and URDF files to ensure correct orientation and positioning on the TurtleBot3 Burger model.
- source: https://github.com/issaiass/realsense2_description
yuhsiang1117
force-pushed
the
feat/realsense
branch
from
9307dc9 to
dc9ba0b
Compare
- added installation of ros-${ROS_DISTRO}-launch-pytest for realsense2_camera package
- fix the issue of realsense not showing in gazebo simulation
- rename package from realsense_desp to realsense_description for clarity
…workspace structure
- removed the local copy of realsense_ros package to avoid conflicts with the package installed via apt
yuhsiang1117
force-pushed
the
feat/realsense
branch
from
dc9ba0b to
35a73e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@template_ws/src/realsense_examples/realsense_gazebo_plugin/CONTRIBUTING.md`:
- Around line 5-12: The fenced code block that begins with ``` before the
"Submission of Contributions..." text is missing a language identifier
(markdownlint MD040); open the CONTRIBUTING.md, locate that fenced block (the
opening ``` marker) and add an explicit language identifier such as ```text or
```none immediately after the backticks so the block becomes ```text, then save
the file to satisfy the lint rule.
♻️ Duplicate comments (8)
docs/docker-modules/realsense.md (1)
59-59: Fix heading typo (“realworld” → “real world”).Line 59 should be “real world” (two words).
🛠️ Proposed fix
-## Realsense test in realworld +## Realsense test in real worlddocker_modules/install_realsense.sh (1)
24-24: Move the success message inside the REALSENSE=YES block.Line 24 prints “completed successfully” even when install is skipped.
🛠️ Proposed fix
if [ "$REALSENSE" = "YES" ]; then echo "Installing RealSense ros packages for ROS distro: ${ROS_DISTRO:-humble}" sudo apt-get update && sudo apt-get install -y \ ros-${ROS_DISTRO}-point-cloud-transport \ ros-${ROS_DISTRO}-librealsense2* \ ros-${ROS_DISTRO}-realsense2-* \ ros-${ROS_DISTRO}-turtlebot3* \ ros-${ROS_DISTRO}-rqt-robot-steering \ ros-${ROS_DISTRO}-launch-pytest \ && sudo rm -rf /var/lib/apt/lists/* + + echo "RealSense installation completed successfully!" fi - -echo "RealSense installation completed successfully!"template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp (4)
31-39: Uninitialized member variables in constructor.Several members used in
OnNewDepthFrame()and publisher setup are not initialized:rangeMinDepth_,rangeMaxDepth_,pointCloudCutOff_,depthUpdateRate_,colorUpdateRate_, andinfraredUpdateRate_. If SDF parameters are missing, these will contain garbage values leading to undefined behavior.🐛 Proposed fix
RealSensePlugin::RealSensePlugin() { this->depthCam = nullptr; this->ired1Cam = nullptr; this->ired2Cam = nullptr; this->colorCam = nullptr; this->prefix = ""; this->pointCloudCutOffMax_ = 5.0; + this->pointCloudCutOff_ = 0.0; + this->rangeMinDepth_ = 0.1f; + this->rangeMaxDepth_ = 10.0f; + this->depthUpdateRate_ = 30.0; + this->colorUpdateRate_ = 30.0; + this->infraredUpdateRate_ = 30.0; }
121-121: Typo: "Ivalid" should be "Invalid".✏️ Proposed fix
- throw std::runtime_error("Ivalid parameter for RealSensePlugin"); + throw std::runtime_error("Invalid parameter for RealSensePlugin");
137-149: Null pointer dereference before safety checks.The code calls
->DepthCamera()and->Camera()directly on the result ofdynamic_pointer_castwithout checking if the cast succeeded. IfGetSensor()returnsnullptror the cast fails, this will crash before the null checks on lines 152-171.🐛 Proposed fix
- this->depthCam = std::dynamic_pointer_cast<sensors::DepthCameraSensor>( - smanager->GetSensor(prefix + DEPTH_CAMERA_NAME)) - ->DepthCamera(); - - this->ired1Cam = std::dynamic_pointer_cast<sensors::CameraSensor>( - smanager->GetSensor(prefix + IRED1_CAMERA_NAME)) - ->Camera(); - this->ired2Cam = std::dynamic_pointer_cast<sensors::CameraSensor>( - smanager->GetSensor(prefix + IRED2_CAMERA_NAME)) - ->Camera(); - this->colorCam = std::dynamic_pointer_cast<sensors::CameraSensor>( - smanager->GetSensor(prefix + COLOR_CAMERA_NAME)) - ->Camera(); + auto depthSensor = std::dynamic_pointer_cast<sensors::DepthCameraSensor>( + smanager->GetSensor(prefix + DEPTH_CAMERA_NAME)); + if (depthSensor) { + this->depthCam = depthSensor->DepthCamera(); + } + + auto ired1Sensor = std::dynamic_pointer_cast<sensors::CameraSensor>( + smanager->GetSensor(prefix + IRED1_CAMERA_NAME)); + if (ired1Sensor) { + this->ired1Cam = ired1Sensor->Camera(); + } + + auto ired2Sensor = std::dynamic_pointer_cast<sensors::CameraSensor>( + smanager->GetSensor(prefix + IRED2_CAMERA_NAME)); + if (ired2Sensor) { + this->ired2Cam = ired2Sensor->Camera(); + } + + auto colorSensor = std::dynamic_pointer_cast<sensors::CameraSensor>( + smanager->GetSensor(prefix + COLOR_CAMERA_NAME)); + if (colorSensor) { + this->colorCam = colorSensor->Camera(); + }
280-282: Incorrect step calculation for depth map.
ImageDepth()returns the original camera's bytes-per-pixel (likely 4 for float), butdepthMapcontainsuint16_tvalues (2 bytes per pixel). This mismatch will cause incorrect image interpretation downstream.🐛 Proposed fix
msg.mutable_image()->set_step( - this->depthCam->ImageWidth() * - this->depthCam->ImageDepth()); + this->depthCam->ImageWidth() * sizeof(uint16_t));template_ws/src/realsense_examples/realsense_gazebo_plugin/src/gazebo_ros_realsense.cpp (2)
40-57: ROS initialization check happens after node creation.
rclcpp::ok()is checked on line 48 afterrclcpp::Node::make_shared()on line 45. If ROS is not initialized,make_sharedwill throw or fail before reaching the check. Move the check before node creation.🐛 Proposed fix
void GazeboRosRealsense::Load(physics::ModelPtr _model, sdf::ElementPtr _sdf) { + // Make sure the ROS node for Gazebo has already been initialized + if (!rclcpp::ok()) { + RCLCPP_ERROR( + rclcpp::get_logger("realsense_gazebo_plugin"), + "A ROS node for Gazebo has not been initialized, unable " + "to load plugin. " + "Load the Gazebo system plugin " + "'libgazebo_ros_api_plugin.so' in the gazebo_ros " + "package"); + return; + } + RealSensePlugin::Load(_model, _sdf); std::string node_name = "gazebo_realsense"; node_name += this->prefix.empty() ? "" : "_" + this->prefix; this->node_ = rclcpp::Node::make_shared(node_name); - // Make sure the ROS node for Gazebo has already been initialized - if (!rclcpp::ok()) { - RCLCPP_ERROR( - node_->get_logger(), - "A ROS node for Gazebo has not been initialized, unable " - "to load plugin. " - "Load the Gazebo system plugin " - "'libgazebo_ros_api_plugin.so' in the gazebo_ros " - "package"); - return; - } RCLCPP_INFO(node_->get_logger(), "Realsense Gazebo ROS plugin loading.");
149-152: Iterators use memberpointcloud_msg_instead of parameterpoint_cloud_msg.The
PointCloud2Modifiercorrectly operates onpoint_cloud_msg, but the iterators are created on the class memberpointcloud_msg_. This causes the iterators to write to a different message than the one being modified.🐛 Proposed fix
- sensor_msgs::PointCloud2Iterator<float> iter_x(pointcloud_msg_, "x"); - sensor_msgs::PointCloud2Iterator<float> iter_y(pointcloud_msg_, "y"); - sensor_msgs::PointCloud2Iterator<float> iter_z(pointcloud_msg_, "z"); - sensor_msgs::PointCloud2Iterator<uint8_t> iter_rgb(pointcloud_msg_, "rgb"); + sensor_msgs::PointCloud2Iterator<float> iter_x(point_cloud_msg, "x"); + sensor_msgs::PointCloud2Iterator<float> iter_y(point_cloud_msg, "y"); + sensor_msgs::PointCloud2Iterator<float> iter_z(point_cloud_msg, "z"); + sensor_msgs::PointCloud2Iterator<uint8_t> iter_rgb(point_cloud_msg, "rgb");
🧹 Nitpick comments (7)
template_ws/src/realsense_examples/realsense_gazebo_plugin/src/gazebo_ros_realsense.cpp (1)
279-294: Consider throwing or returning an error indicator for unknown camera names.
extractCameraNamelogs an error but returnsCOLOR_CAMERA_NAMEas a fallback for unknown cameras. This could silently cause incorrect behavior. Consider throwing an exception or returning an empty string to fail visibly.♻️ Suggested improvement
RCLCPP_ERROR(rclcpp::get_logger("realsense_camera"), "Unknown camera name"); - return COLOR_CAMERA_NAME; + throw std::runtime_error("Unknown camera name: " + name); }template_ws/src/realsense_examples/realsense_gazebo_plugin/include/realsense_gazebo_plugin/gazebo_ros_realsense.hpp (3)
37-68: Consider consolidating access specifiers.Multiple
public:access specifiers are scattered throughout the class declaration (lines 37, 42, 47, 53, 65). While functional, consolidating them improves readability.♻️ Suggested cleanup
class GazeboRosRealsense : public RealSensePlugin { - /// \brief Constructor. - public: + /// \brief Constructor. GazeboRosRealsense(); /// \brief Destructor. - -public: ~GazeboRosRealsense(); // Documentation Inherited. - -public: virtual void Load(physics::ModelPtr _model, sdf::ElementPtr _sdf); /// \brief Callback that publishes a received Depth Camera Frame as an /// ImageStamped message. - -public: virtual void OnNewDepthFrame(); /// \brief Helper function to fill the pointcloud information bool FillPointCloudHelper( sensor_msgs::msg::PointCloud2 & point_cloud_msg, uint32_t rows_arg, uint32_t cols_arg, uint32_t step_arg, const void * data_arg); /// \brief Callback that publishes a received Camera Frame as an /// ImageStamped message. - -public: virtual void OnNewFrame( const rendering::CameraPtr cam, const transport::PublisherPtr pub);
56-60: Unused parameterstep_argin FillPointCloudHelper.The
step_argparameter is declared in the function signature but appears unused in the implementation (see gazebo_ros_realsense.cpp lines 139-232). Consider removing it or documenting why it's reserved.
70-72: Consider usingstd::shared_ptrinstead ofboost::shared_ptr.Modern ROS 2 code typically uses
std::shared_ptr. Whileboost::shared_ptrworks, usingstd::shared_ptraligns better with ROS 2 conventions.template_ws/src/realsense_examples/realsense_gazebo_plugin/include/realsense_gazebo_plugin/RealSensePlugin.hpp (3)
32-35: Consider usingconstexprstrings instead of#definemacros.Macros don't respect namespace scope. Using
constexprorconststrings would be safer and more idiomatic for C++.♻️ Suggested improvement
namespace gazebo { -#define DEPTH_CAMERA_NAME "depth" -#define COLOR_CAMERA_NAME "color" -#define IRED1_CAMERA_NAME "ired1" -#define IRED2_CAMERA_NAME "ired2" +constexpr const char* DEPTH_CAMERA_NAME = "depth"; +constexpr const char* COLOR_CAMERA_NAME = "color"; +constexpr const char* IRED1_CAMERA_NAME = "ired1"; +constexpr const char* IRED2_CAMERA_NAME = "ired2";
37-46: Initialize CameraParams members with default values.The struct has an empty constructor but members are uninitialized. Consider using in-class initializers or a defaulted constructor for clarity.
♻️ Suggested improvement
struct CameraParams { - CameraParams() - { - } + CameraParams() = default; - std::string topic_name; - std::string camera_info_topic_name; - std::string optical_frame; + std::string topic_name{}; + std::string camera_info_topic_name{}; + std::string optical_frame{}; };
133-142: Member variables lack default initialization.Several members (
pointCloudCutOff_,pointCloudCutOffMax_,colorUpdateRate_,infraredUpdateRate_,depthUpdateRate_,rangeMinDepth_,rangeMaxDepth_) are declared without default values. This relates to the constructor initialization issue flagged in RealSensePlugin.cpp.♻️ Suggested in-class initialization
- bool pointCloud_ = false; - std::string pointCloudTopic_; - double pointCloudCutOff_, pointCloudCutOffMax_; + bool pointCloud_{false}; + std::string pointCloudTopic_{}; + double pointCloudCutOff_{0.0}; + double pointCloudCutOffMax_{5.0}; - double colorUpdateRate_; - double infraredUpdateRate_; - double depthUpdateRate_; + double colorUpdateRate_{30.0}; + double infraredUpdateRate_{30.0}; + double depthUpdateRate_{30.0}; - float rangeMinDepth_; - float rangeMaxDepth_; + float rangeMinDepth_{0.1f}; + float rangeMaxDepth_{10.0f};
template_ws/src/realsense_examples/realsense_gazebo_plugin/CONTRIBUTING.md
Show resolved
Hide resolved
| - Launch Realsense camera node in `tmux` | ||
| ```bash | ||
| # add pointcloud.enable:=true to enable pointcloud | ||
| ros2 launch realsense2_camera rs_launch.py | ||
| ``` |
There was a problem hiding this comment.
Please ensure the tutorial in the Markdown is comprehensive enough for a beginner to start testing RealSense without needing to refer to external links or resources.
For instance, consider adding some examples to help others better understand which parameters are adjustable.
# Check the param (Optional)
ros2 param get /camera/camera depth_module.depth_profile
# Launch the node
ros2 launch realsense2_camera rs_launch.py \
initial_reset:=true \
enable_color:=true \
rgb_camera.color_profile:=640x480x30 \
enable_depth:=true \
depth_module.depth_profile:=640x480x30 \
enable_sync:=true \
enable_rgbd:=true \
align_depth.enable:=true \
pointcloud.enable:=trueTo get a full list of adjustable parameters, check here.
There was a problem hiding this comment.
If you feel this approach isn't ideal, feel free to refer to my launch file. It might be a simpler and more straightforward way to handle this.
| if [ -z "$REALSENSE" ]; then | ||
| echo "Skipping RealSense installation as REALSENSE is not set" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
fix: If REALSENSE is not set to YES, exit.
| && sudo rm -rf /var/lib/apt/lists/* | ||
| fi | ||
|
|
||
| echo "RealSense installation completed successfully!" |
There was a problem hiding this comment.
Minor: This could be improved/fixed as well. It doesn't affect the functionality, but it would be nice to have.
| ros2 run rqt_robot_steering rqt_robot_steering | ||
| ``` | ||
|
|
||
| ## Realsense test in realworld |
There was a problem hiding this comment.
Minor: Please fix it.
| ROS2 Realsense. (Only installed by default for the template workspace.) | ||
|
|
||
| To enable Realsense, set the `REALSENSE` argument to `YES` in the `compose.yaml` file of your desired workspace (e.g., `husky_ws/docker/compose.yaml`). After making these changes, rebuild the Docker image. |
There was a problem hiding this comment.
Consider adding a note stating that these scripts have only been tested on the RealSense D435i.
template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo_plugin/src/RealSensePlugin.cpp
Show resolved
Hide resolved
template_ws/src/realsense_examples/realsense_gazebo_plugin/CONTRIBUTING.md
Show resolved
Hide resolved
|
Please include the scope of your changes at the beginning of your git commit messages. For example, in this PR, it should be Also, please leave a blank line between the commit message header and the body. The same applies to separating paragraphs within the body, and consider capitalizing the first letter of your commit messages for consistency. |
|
Some coderabbitai's feedback is worth implementing. If you decide not to fix something, please leave a comment explaining why instead of just resolving the thread without any action or follow-up. Thanks. |
|
Please add a reference to |
2 participants
Summary of PR
install_realsense.shindocker_modulesrealsense_examplesintemplate_wsrealsense_descriptionfor adding realsense d435 camera model on turtlebotrealsense_gazebofor launching the robot and camera in gazeborealsense_gazebo_pluginfor bridging the simulated camera data to ROS2 topicsrealsense.mdindocs/docker-modulesclosed: #91
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.