ArduPilot / ardupilot_gz

Tools for ArduPilot ROS2 integration and testing on ROS 2 humble
GNU General Public License v3.0
31 stars 22 forks source link

Copter: Add iris with lidar in maze world #20

Closed pedro-fuoco closed 1 year ago

pedro-fuoco commented 1 year ago

Here are my tests: Screenshot from 2023-06-20 18-19-51 Screenshot from 2023-06-20 18-20-39 Screenshot from 2023-06-20 18-20-50

To launch it:

ros2 launch ardupilot_gz_bringup iris_maze.launch.py

Should this command be put in the README as well?

pedro-fuoco commented 1 year ago

There was a little bug regarding the lidar joint and the walls were a bit short. Just submitted a fixup @srmainwaring. Screenshot from 2023-06-21 15-12-45

srmainwaring commented 1 year ago

Looks good @pedro-fuoco! A couple of formatting fixes and a missing hook and rviz config file to add.

Update: posted suggested edits here: https://github.com/pedro-fuoco/ardupilot_gz/pull/1

srmainwaring commented 1 year ago

Should this command be put in the README as well?

Perhaps add another section to the README for more advanced variants?

pedro-fuoco commented 1 year ago

Thanks a lot for the feedback @srmainwaring, fixed the issues you mentioned and merged your PR

Ryanf55 commented 1 year ago

Can you add in some formatting like Ardupilot does for XML and run the hooks on your files before merge? https://github.com/ArduPilot/ardupilot/blob/master/.pre-commit-config.yaml#L36-L45

https://pre-commit.com/

srmainwaring commented 1 year ago

@pedro-fuoco - thanks for the updates.

I've a few more suggestions here: https://github.com/pedro-fuoco/ardupilot_gz/pull/2

iris_with_lidar_gz

iris_with_lidar_rviz

With these changes it's looking pretty complete!

pedro-fuoco commented 1 year ago

@srmainwaring thank you for the PR! I'm always learning a lot with the reviews and additions you make. Everything is was commited and rebased, I think we are good to merge it now :)

srmainwaring commented 1 year ago

@pedro-fuoco - almost there. Please squash / edit the commits to remove the [skip ci] (I should have edited them in my earlier PR). Tested the merged version and running well.

pedro-fuoco commented 1 year ago

@srmainwaring squashed everything and added you as co-author!