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 Marker DisplayTypes Tutorial #4821

Merged
merged 9 commits into from
Nov 21, 2024
Merged

Add RViz Marker DisplayTypes Tutorial #4821

merged 9 commits into from
Nov 21, 2024

Conversation

LeweC
Copy link
Contributor

@LeweC LeweC commented Oct 22, 2024

This PR is a continuation of the initial issue #3864, the discussion in the abandoned PR #3908 which led to the first completed PR #4040 of the porting of old RViz tutorial series.

This is the next tutorial about RViz and especially RViz Marker Types, which is the basis for the following tutorial series I want to port in later PRs as soon as this one is ready because I have some time now.

The link to the original Marker DisplayTypes Tutorial in ROS1 which served as a basis.

In the old RViz documentation page the DisplayTypes: Marker was grouped with the other DisplayTypes on a subpage. In contrast, I decided to keep it on the main RViz subpage for this version, as I do not intend to add new PRs for the other DisplayTypes in the near future. I want to finish the current tutorial series first. I do not think we need a subpage for the Marker DisplayTypes page until we have more than one other DisplayType Page, and I think this page still fits nicely into the main RViz page at the moment, but I am interested in your opinion.

I think this version is ready for your next review step (as it had one or two review steps in #3908)

@ahcorde ahcorde added the backport-all backport at reviewers discretion; from rolling to all versions label Nov 11, 2024
@ahcorde
Copy link
Contributor

ahcorde commented Nov 11, 2024

@clalancette Do you mind to take a quick look ?

Copy link
Collaborator

@kscottz kscottz left a comment

Choose a reason for hiding this comment

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

@LeweC This is fantastic and we really appreciate your contribution! Thank you so much.

This is a fantastic start to a decent RViz marker tutorial. Obviously there is a lot more we could add here, but I think this is good enough to add in once the suggestions are merged.


* ``frame_locked``:

Tells RViz to retransform the marker into the current location of the specified frame every update cycle.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not your fault, but I wish this was a bit clearer. The docs are pretty sparse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it a bit. Mainly added a sentence, I hope this makes it clearer.

Co-authored-by: Katherine Scott <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Lewe Christiansen <[email protected]>
Copy link
Contributor

@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.

@LeweC
Copy link
Contributor Author

LeweC commented Nov 21, 2024

Do you mind to check linters ? https://github.com/ros2/ros2_documentation/actions/runs/11952606475/job/33318689926?pr=4821

Yeah I was just in the procces of doing that. I am pretty sure my setup is/was correct, I think it came from one of the suggestions. Fixed now and I think I added all your guys feedback.

Copy link
Contributor

@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.

Thank you for iterating with us!

@ahcorde ahcorde merged commit 8c6ae4a into ros2:rolling Nov 21, 2024
4 checks passed
mergify bot pushed a commit that referenced this pull request Nov 21, 2024
Signed-off-by: Lewe Christiansen <[email protected]>
Co-authored-by: lewec <[email protected]>
(cherry picked from commit 8c6ae4a)
mergify bot pushed a commit that referenced this pull request Nov 21, 2024
Signed-off-by: Lewe Christiansen <[email protected]>
Co-authored-by: lewec <[email protected]>
(cherry picked from commit 8c6ae4a)
mergify bot pushed a commit that referenced this pull request Nov 21, 2024
Signed-off-by: Lewe Christiansen <[email protected]>
Co-authored-by: lewec <[email protected]>
(cherry picked from commit 8c6ae4a)
ahcorde pushed a commit that referenced this pull request Nov 21, 2024
Signed-off-by: Lewe Christiansen <[email protected]>
Co-authored-by: lewec <[email protected]>
(cherry picked from commit 8c6ae4a)

Co-authored-by: Lewe Christiansen <[email protected]>
ahcorde pushed a commit that referenced this pull request Nov 21, 2024
Signed-off-by: Lewe Christiansen <[email protected]>
Co-authored-by: lewec <[email protected]>
(cherry picked from commit 8c6ae4a)

Co-authored-by: Lewe Christiansen <[email protected]>
ahcorde pushed a commit that referenced this pull request Nov 21, 2024
Signed-off-by: Lewe Christiansen <[email protected]>
Co-authored-by: lewec <[email protected]>
(cherry picked from commit 8c6ae4a)

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