-
Notifications
You must be signed in to change notification settings - Fork 115
Update Turtlebot3-gazebo example to use Humble distro #606
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Run: | ||
| # ros2 launch turtlebot3_gazebo turtlebot3_world.launch.py | ||
| # ros2 run turtlebot3_teleop teleop_keyboard | ||
|
|
||
| { pkgs ? import ../. {} }: | ||
| with pkgs; | ||
| with rosPackages.humble; | ||
| with pythonPackages; | ||
|
|
||
| mkShell { | ||
| buildInputs = [ | ||
| glibcLocales | ||
| (buildEnv { paths = [ | ||
| ros-base | ||
| turtlebot3-teleop | ||
| turtlebot3-gazebo | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This package depends on gazebo_ros_pkgs, which is deprecated. They recommend to follow https://gazebosim.org/docs/latest/gazebo_classic_migration. Seems like https://gazebosim.org/docs/latest/migrating_gazebo_classic_ros2_packages/ might be what you need. I also wonder whether #591 contains what you intend to do here or not. There is a new example ros2-turtlebot4-gz.nix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The proposed pull request certainly tackles some of the issues, and I appreciate that effort! However, I believe it's really important for the layer to create a welcoming experience for new users. If there's a common issue with building widely used packages, it might not provide the best first-time experience for them. I encourage you to take a closer look at my earlier comments and consider the options I've suggested- they're intended to help us all move forward together! |
||
| ]; }) | ||
| ]; | ||
|
|
||
| ROS_HOSTNAME = "localhost"; | ||
| ROS_MASTER_URI = "http://localhost:11311"; | ||
| TURTLEBOT3_MODEL = "burger"; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change the names of existing files (and attributes below). Users may rely on these names in their Nix expressions and this would break them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was discussing that with @mdrodrigo and he brought a valid point that the example was not working as nobody noticed that it was failing to build. So I'm pretty sure that users are not relying on this otherwise we would have received issues about it failing to build and run. Considering that I think that would be better to rename the attribute so it is clear which version it is using and causes less confusion for users when testing it.
What do you think?