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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ros2] Update documentation for installation instructions and bridge examples #142

Merged
merged 8 commits into from
Mar 27, 2021
Merged

[ros2] Update documentation for installation instructions and bridge examples #142

merged 8 commits into from
Mar 27, 2021

Conversation

AndrejOrsula
Copy link
Contributor

@AndrejOrsula AndrejOrsula commented Mar 26, 2021

馃 Bug fix

Summary

Few minor changes to installation instructions and examples for ROS 2. Tested on Ubuntu Focal, with Edifice built from source (Docker).

Installation instructions:

  • rosdep install complains if Ignition libraries are included in requirements
    • Skipping keys for Citadel, Dome and Edifice versions of the required libraries (I wish there was a wildcard support)
  • Both README.md and ros_ign_bridge/README.md contained installation instructions
    • Removed instructions from ros_ign_bridge/README.md
    • Note: Please let me know if there was a reason behind this. If so, we can revert the change and update the instructions there so that they are correct

Examples:

  • Source command of ROS 2 workspace was using invalid ROS distribution (melodic instead of foxy)
    • Fixed
  • Bridge example for image transport contained a screenshot for the example using ROS 1
    • Replaced with ROS 2 alternative

Checklist

  • Signed all commits for DCO
  • Updated documentation (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)

Note to maintainers: Remember to use Squash-Merge

Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
@chapulina chapulina added this to Inbox in Core development via automation Mar 26, 2021
@chapulina chapulina moved this from Inbox to In review in Core development Mar 26, 2021
@chapulina chapulina added ROS 2 ROS 2 馃彴 citadel Ignition Citadel 馃敭 dome Ignition Dome 馃彚 edifice Ignition Edifice labels Mar 26, 2021
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

I think it makes sense to just keep the install instructions in one place, the other libraries don't have install instructions in the particular README.md.

@chapulina do you have a different opinion ?

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Removed instructions from ros_ign_bridge/README.md

Thanks, that was already done for ROS 1 but we never updated ROS 2.

README.md Outdated Show resolved Hide resolved
ros_ign_bridge/README.md Show resolved Hide resolved
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
@chapulina chapulina enabled auto-merge (squash) March 26, 2021 23:53
Signed-off-by: Andrej Orsula <orsula.andrej@gmail.com>
auto-merge was automatically disabled March 27, 2021 14:05

Head branch was pushed to by a user without write access

@AndrejOrsula
Copy link
Contributor Author

Note: Added one more commit into this PR to remove FluidPressure TODO from support matrix. Demo for it was enabled and tested in #144, but I added the commit here to avoid merge conflicts.

@chapulina chapulina merged commit 9ddfa24 into gazebosim:ros2 Mar 27, 2021
Core development automation moved this from In review to Done Mar 27, 2021
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
馃彴 citadel Ignition Citadel 馃敭 dome Ignition Dome 馃彚 edifice Ignition Edifice ROS 2 ROS 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants