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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RViz UserGuide #4040

Merged
merged 7 commits into from
Mar 14, 2024
Merged

Add RViz UserGuide #4040

merged 7 commits into from
Mar 14, 2024

Conversation

LeweC
Copy link
Contributor

@LeweC LeweC commented Nov 30, 2023

As discussed in PR #3908 and initial Issue #3864.

I am not sure where this UserGuide would fit in the current structure.
I only came up with the idea of having it in the Tutorials/RViz folder because I could not find a section for UserGuides and needed something to commit to open this PR.

Maybe this is the right place for it, or did you have a better idea?

Also, I will be a bit busy until the end of January. So if anyone wants to replicate this UserGuide for ROS2, feel free to do so.
There is some more information in the above mentioned PR. If not, I have time in February to focus on both PRs and maybe we can even merge them soon.

@LeweC LeweC changed the title Add RVizUserGuide Add RViz UserGuide Nov 30, 2023
@clalancette
Copy link
Contributor

Maybe this is the right place for it, or did you have a better idea?

Yes, this is exactly what I had in mind. With this directory in place, we can then go on and put the tutorials in #3908 in the same place.

I'll take a quick look at the rest of this and leave some feedback.

@LeweC
Copy link
Contributor Author

LeweC commented Feb 12, 2024

Okay, now I have time to work on this again. While I do that, do you have any ideas or recommendations on how to deal with the two linked youtube videos in there? Do you know or have any new RViz examples or should we just not link to a video there?

@clalancette
Copy link
Contributor

Do you know or have any new RViz examples or should we just not link to a video there?

Let's just leave them out for now. We can always add in videos again later if we decide to make something.

@LeweC
Copy link
Contributor Author

LeweC commented Feb 26, 2024

Okay, I think this version is ready for a first review. Here are a few notes:

  • I did not include the link to the Troubleshooting page in the original UserGuide because that is a whole new task.
  • I am not entirely happy with how the table of "Built-in Display Types" looks when I build it locally, because you have to scroll to see the links.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Thanks for picking this back up! I've done a first pass review, please take a look.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This is definitely getting closer! Thanks for iterating.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I've left a lot of comments, but we are just down to nitpicks now. Once this round is done, I'm pretty happy with this and I'll run the tests again.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

There are some linting errors; see the GitHub jobs for those.

Once those are fixed, I'll suggest marking this as "Ready for Review", and we can merge it in. Thank you for all of the hard work here.

@LeweC LeweC marked this pull request as ready for review March 14, 2024 08:05
@LeweC LeweC requested a review from audrow as a code owner March 14, 2024 08:05
@clalancette clalancette added the backport-all backport at reviewers discretion; from rolling to all versions label Mar 14, 2024
@clalancette clalancette merged commit 99fd85d into ros2:rolling Mar 14, 2024
4 checks passed
mergify bot pushed a commit that referenced this pull request Mar 14, 2024
* Add place for UserGuide

(cherry picked from commit 99fd85d)
mergify bot pushed a commit that referenced this pull request Mar 14, 2024
* Add place for UserGuide

(cherry picked from commit 99fd85d)
clalancette pushed a commit that referenced this pull request Mar 14, 2024
* Add place for UserGuide

(cherry picked from commit 99fd85d)

Co-authored-by: Lewe Christiansen <[email protected]>
clalancette pushed a commit that referenced this pull request Mar 14, 2024
* Add place for UserGuide

(cherry picked from commit 99fd85d)

Co-authored-by: Lewe Christiansen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants