-
Notifications
You must be signed in to change notification settings - Fork 6
Fix GRO830 Gazebo models issues... and QoL updates #44
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
Fix GRO830 Gazebo models issues... and QoL updates #44
Conversation
This commit also applies auto-formatting. CHANGES: - Auto-format; - Fix `Gazebo/<color>` material warnings; - Comment out attributes that cause "undefined attribute in SDF model" warnings; - Update Gazebo plugin names from Fortress to Harmonic (Jazzy compatible).
This commit applies the changes from `4ff0088` and `11673d4` to all relevant launch files in `racecar_gazebo/launch`. This should make it so all lab exercises and the validation work without issues caused by the models used by Gazebo. This commit also applies auto-formatting to the changed files. NOTES: - This commit has NOT yet been tested on a faculty's lab PC. It is required to test the lab exercises AND the validation BEFORE merging this commit onto the main branch; - Tests conducted and passed on my personal laptop (Ubuntu 24.04, ROS Jazzy).
lopetit
left a comment
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.
Ça semble ok de mon côté
|
Confirmed working on lab PC with the following "tweaks" applied to the base image:
The following "tweaks" were also applied to the build procedure:
@lopetit, a note on these "tweaks" SHOULD be added to the educational material (in the lab's section), because the lab PCs' image can't be changed until august 2026. |
|
@MathieuMRodrigue, once you confirmed the changes are working on your side, could you take care of merging the PR? In my opinion, a "rebase and merge" is the most appropriate method. |
chameau5050
left a comment
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.
maybe a tiny change while we are there
|
@MarcOlivierFecteau can you add these 2 tweaks to doc/gro830-lab-a25.md? I have already added a link to this in the educational material. |
Additionally, this commit removes trailing spaces. NOTES: - This is a "QoL" commit: no tests are required.
The three launch files used to start the Gazebo simulation were basically the same, with only the `.world` file name being different. Ergo, I took the identical code from the launch files, and added an argument to specify the world file loaded into Gazebo. NOTES: - This change involves non-negligible modifications to the documentation in the educational material; - This commit has NOT yet been tested on a faculty's lab PC, and MUST be tested BEFORE merging the PR into the main branch. TODO: - Test the "unity" launch file on the faculty's lab PCs (ideally, by several people).
Documentation for both tweaks has been added to doc/gro830-lab-a25.md. |
This makes the "general launch file" for starting the Gazebo simulation work. NOTES: - Do NOT use f-strings inside `IncludeLaunchDescription`! As a general rule, try to avoid f-strings in ROS2 lanch files: use the `launch.substitutions` module instead; - This commit has been tested on my personal laptop (Ubuntu 24.04, ROS Jazzy, Gazebo Harmonic). TODO: - Test this commit on a faculty's lab PC (ideally, with both `IGN_GAZEBO_RESOURCE_PATH` and `GZ_SIM_RESOURCE_PATH` used as environment variable, to take into account potential oversights from students... and tutors / technical support staff).
chameau5050
left a comment
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.
good job, but I do prefer the old method
|
e59279f confirmed working on a lab PC with both source /opt/ros/jazzy/setup.bash
if [ -f ~/ros2_ws/install/local_setup.bash ]; then
source ~/ros2_ws/install/local_setup.bash
fi
unset IGN_GAZEBO_RESOURCE_PATH # Also tested with this line commentedBelow are the commands used for the test: cd ~/ros2_ws
colcon build --packages-skip serial pb2roscpp
source install/local_setup.bash
ros2 launch racecar_gazebo simulation.launch.py # Test 1: default value
ros2 launch racecar_gazebo simulation.launch.py world:=tunnel # Test 2: lab world (navigation)
ros2 launch racecar_gazebo simulation.launch.py world:=tunnel_genie # Test 3: validation
ros2 launch racecar_gazebo simulation.launch.py world:=circuit # Test 4: lab world (behaviors) |
This commit implements [this requested change](https://github.com/SherbyRobotics/racecar/pull/44/files/e59279f6461f49c918ca342c3cabcb69d45ba424#r2459951687). It essentially "rolls back" to using Python f-strings for string interpolation of the path to the world file loaded in Gazebo instead of using ROS's `launch.substitutions` module. TODO: - Test this commit on a faculty's lab PC BEFORE merging the PR.
chameau5050
left a comment
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.
Looks good to me ! Good job.
The changes in ab34974 aimed to remove warnings when starting the Gazebo simulation caused by a discrepancy in attributes between the `.gazebo` file and the SDF model. However, commenting out these attributes made the SLAM stopped working because it couldn't properly convert the LiDAR messages, which in turn caused it so the map couldn't be fetched. This commit rolls back the changes, and the SLAM now works properly. NOTES: - The attributes warnings have reappeared. Eventually, we will need to address this issue, but for now it is considered low priority.
No description provided.