-
Notifications
You must be signed in to change notification settings - Fork 22
more worlds in gazebo #595
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
Conversation
""" WalkthroughThe updates include a new section in the README explaining how to select simulation worlds, an updated commit hash for the Changes
Suggested reviewers
Estimated code review effort1 (<30 minutes) 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
husarion_ugv_gazebo/launch/simulation.launch.py (1)
51-55
: Clarify the missingy
coordinate for the outdoor scene.The
outdoor
scene configuration doesn't specify ay
coordinate, while other scenes with custom positions include all three coordinates. This inconsistency might lead to confusion.If the intention is to use the default
y
value (likely 0), consider explicitly setting it for clarity:"outdoor": { "gz_world": '"https://fuel.gazebosim.org/1.0/Cole/worlds/Rubicon World"', "x": "3", + "y": "0", "z": "2", },
husarion_ugv_gazebo/README.md (1)
12-25
: Minor documentation improvements.The documentation is clear and helpful. Consider these small refinements:
### Worlds -In order to run the simulation with a different world, use: +To run the simulation with a different world, use: -``` +```bash ros2 launch husarion_ugv_gazebo simulation.launch.py scene:=outdoor</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 702a1bd20f6c13ecff038e9df55e92a0c13b45f9 and 4b0d4891ce6d6a38caed7fa2636033794c918a0f. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `husarion_ugv_gazebo/README.md` (1 hunks) * `husarion_ugv_gazebo/launch/simulation.launch.py` (5 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>husarion_ugv_gazebo/README.md</summary> [style] ~13-~13: Consider a more concise word here. Context: ... with the given arguments. ### Worlds In order to run the simulation with a different wor... (IN_ORDER_TO_PREMIUM) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>husarion_ugv_gazebo/README.md</summary> 16-16: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)</summary> * GitHub Check: Run unit tests hardware build type * GitHub Check: Run unit tests simulation build type </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>husarion_ugv_gazebo/launch/simulation.launch.py (1)</summary> `65-91`: **Well-structured implementation of scene configuration.** The dynamic parameter passing using dictionary unpacking is clean and maintainable. The approach allows easy extension with new scenes in the future. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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.
Sorry, I forget about this review (ping me in the future ;) ).
Looking on this PR IMO, we duplicate the logic of gz_world
arg. What about adding additional world like this: https://github.com/husarion/husarion_gz_worlds/blob/main/worlds/sonoma_raceway.sdf
where we can specify x, y, z offset.
At best, could we simplify the execution of this argument so that we don't have to enter the entire path, but instead have aliases to the ones we already have?
So both
gz_world:=/path/to/racaway.sdf
and
gz_world:=raceway
could work.
Hi @rafal-gorecki, The reason why it is a new argument in I attempted moving the entire world in reverse by creating a new SDF in the So that leads me to a conclusion that we shouldn't be trying to modify the worlds themselves, but only move the robot within them. If we really do not wish to add new arguments, I could modify |
727d6fe
to
4f5dcbe
Compare
Okay, I have refactored everything, implementing new worlds as .sdf files in Most changes were introduced in Test each new world using: ros2 launch husarion_ugv_gazebo simulation.launch.py gz_world:=rubicon Worlds to test:
|
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 update hash in vcs after merge.
Description
I have added a new
scene
launch argument inhusarion_ugv_gazebo
, insimulation.launch.py
, which can be set to predefined strings likeoutdoor
,ioffice
, etc. This allows to easily switch between worlds.Specifying the argument effectively updates the launch arguments passed to descendant launch files,
husarion_gz_worlds/gz_sim.launch.py
, andhusarion_ugv_gazebo/simulate_robot.launch.py
. This is done in order to load a different world and to set a more suitable initial position of the robot.Try it out like this:
Scenes to review:
husarion_gz_worlds
office
- Husarion office fromhusarion_gz_worlds
raceway
- modified Sonoma Raceway fromhusarion_gz_worlds
outdoor
- Rubicon World from Gazebo Fuel @ Colecave
- Cave World from Gazebo Fuel @ OpenRoboticsRequirements
Tests 🧪
Summary by CodeRabbit
Documentation
Chores
Refactor