Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dockerfile and Workspace for slamtool-box #15

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

Assume-Zhan
Copy link
Collaborator

@Assume-Zhan Assume-Zhan commented Oct 20, 2023

Closes: #11

@Assume-Zhan Assume-Zhan force-pushed the slamtoolbox-humble-docker branch from bb6e173 to a0dac6f Compare October 21, 2023 06:11
- slam_toobox ref:
https://github.com/SteveMacenski/slam_toolbox

Originally I've the commit with folder named slamtoolbox_ws,
I found the term 'slamtoolbox' to be a bit challenging to comprehend,
which is why I altered it to 'slam_toolbox_ws'.

Add COPY bashrc in dockerfile
Ref: https://github.com/j3soon/ros2-agv-essentials/blob/master/template_ws/docker/Dockerfile#L48
- gitignore template: template_ws
https://github.com/j3soon/ros2-agv-essentials/blob/master/template_ws/.gitignore

- Error cache folder
Fixed in this PR: j3soon#13
- Docker-Compose template: template_ws
https://github.com/j3soon/ros2-agv-essentials/blob/master/template_ws/docker/docker-compose.yaml

- Modify name to slam-toolbox in docker compose
In this case I've only changed the name to slam-toolbox.
I considered it is redundant to seperate into different commit,
so I've combined them into same commit.
Due to the inconvenience of using a Git container, I decided to modify the mount folder in the Docker Compose configuration.

However, with this setup, there's a potential risk of inadvertently modifying the wrong workspace,
as both the .git folder and other workspace contents are mounted into the container,
which contains many similar elements such as Dockerfiles and docker-compose files.

Similar topic: j3soon#13
@Assume-Zhan Assume-Zhan force-pushed the slamtoolbox-humble-docker branch from a0dac6f to 03093c9 Compare October 21, 2023 07:20
@Assume-Zhan Assume-Zhan requested a review from j3soon October 21, 2023 14:17
Copy link
Owner

@j3soon j3soon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this PR.

The commit history is well-organized and informative. In addition, I learned a lot while reviewing this PR. Please resolve each comment and re-request a review. Thank you.

slam_toolbox_ws/src/slam_simulation/package.xml Outdated Show resolved Hide resolved
# if you'd like to immediately start continuing a map at a given pose
# or at the dock, but they are mutually exclusive, if pose is given
# will use pose
# map_file_name: /home/ros2-agv-essentials/slam_toolbox_ws/maps/map_posegraph
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the map_file_name is commented out. I wonder why the change is required in commit 311ea43.

Or should we include a section in README showing how to use a map file?

@@ -66,10 +66,42 @@ def generate_launch_description():
PythonLaunchDescriptionSource(gazebo_path)
)

# TF Ref:
# https://docs.ros.org/en/humble/Tutorials/Intermediate/Tf2/Writing-A-Tf2-Static-Broadcaster-Py.html#the-proper-way-to-publish-static-transforms
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest adding more info such as:

  • Add a comment on the dummy TFs:

    # These static TF values are merely dummy values. The preferred way is to store the TFs in the URDF/SDF file and publish through `robot_state_publisher`.
  • Add comment on the turtlebot SDF file:

    # The links are defined based on the following SDF file:
    # https://github.com/ROBOTIS-GIT/turtlebot3_simulations/blob/humble-devel/turtlebot3_gazebo/models/turtlebot3_waffle/model.sdf
  • Add comment on inspecting the SDF file:

    # The links can be visualized in the TF section in rviz2 by the following commands:
    # $ export TURTLEBOT3_MODEL=waffle
    # $ ros2 launch turtlebot3_gazebo empty_world.launch.py
    # $ ros2 launch turtlebot3_bringup rviz2.launch.py
    # Ref: https://emanual.robotis.com/docs/en/platform/turtlebot3/simulation/
    # Ref: https://idminer.gitbook.io/robotis/part-1-turtlebot3/15.-ros2/15.2.-bringup-qi-yong/15.2.2.-rviz2-3d-ke-shi-hua-gong-ju

    If you know a better (or shorter) command to visualize TFs in rviz2, feel free to use yours instead.

name = "link_to_imu",
arguments = ["0", "0", "0", "0", "0", "0", "base_link", "imu_link"],
parameters=[{'use_sim_time': True}]
)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to mention why these 3 links are necessary. Take base_footprint -> base_link as an example, it may be necessary since base_footprint is the base frame as set here: https://github.com/SteveMacenski/slam_toolbox/blob/8365b77710df4759cd96a14be2b3082ede22f15a/config/mapper_params_online_async.yaml#L15

```sh
docker attach ros2-slam-toolbox-ws
```

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we didn't build this package in the Dockerfile, we need to run colcon build and source $ROS2_WS/install/setup.bash here. Otherwise, the next step will fail:

Package 'slam_simulation' not found: "package 'slam_simulation' not found, searching: ['/opt/ros/humble']"

P.S. Maybe we can consider running colcon build in .bashrc? I'm not sure about this though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest implementing a straightforward conditional statement to verify whether the current folder corresponds to the workspace folder. If it does, we can execute 'colcon build' directly; otherwise, the script should do nothing preventing from building a wrong folder.

Or maybe we could directly add "colcon build" in README?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding colcon build in README sounds simpler.

slam_toolbox_ws/README.md Show resolved Hide resolved
ros2 launch slam_simulation slam_sim.launch.py
```

You can put something in the gazebo world, since currently is empty.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two suggestions here:

  1. Use a default gazebo map such as this or this for verifying that slam toolbox is indeed working as expected.
  2. Add a short guide on using rqt to control the turtlebot for the ease of testing.

Ref:
- Commit: https://github.com/SteveMacenski/slam_toolbox/tree/8365b77
- File: slam_toolbox/config/mapper_params_online_async.yaml
@Assume-Zhan Assume-Zhan force-pushed the slamtoolbox-humble-docker branch from 03093c9 to 4b687ad Compare November 16, 2023 11:25
- In this case, we want to automate the building process in bashrc.
- And preventing error when we haven't build the workspace

if-else statement in shell script
Ref: https://linuxize.com/post/bash-if-else-statement/
@Assume-Zhan Assume-Zhan self-assigned this Jan 26, 2024
@j3soon j3soon force-pushed the main branch 2 times, most recently from a8b1110 to dbd12e4 Compare October 23, 2024 19:17
@j3soon j3soon force-pushed the main branch 4 times, most recently from 9a326f9 to ffd229a Compare December 13, 2024 15:24
@j3soon j3soon force-pushed the main branch 3 times, most recently from ecf7008 to adf7444 Compare December 15, 2024 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Dockerfile for SLAM Toolbox
2 participants