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

Rviz Panel Tutorial #4869

Open
wants to merge 21 commits into
base: rolling
Choose a base branch
from
Open

Rviz Panel Tutorial #4869

wants to merge 21 commits into from

Conversation

DLu
Copy link
Contributor

@DLu DLu commented Nov 15, 2024

Following the smash success of #3384, I have made a very similar looking tutorial for making a RViz panel. Mostly because I wanted the boilerplate code to be written somewhere.

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

@DLu thanks for this PR.

just a quick question though. rviz does not have those examples or samples that user can refer to? i was thinking that if there are, we could link those information here instead of having the whole implementation in ros2 documentation here.

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.

@DLu thanks for this! Much appreciated!

I added a bunch of suggestions that I think will help clarify things. I generally like to see us follow the pattern of:

  • Tell the user what they are going to do.
  • Show them the code with lots of in-line comments.
  • Recap what we just did.

I added suggestions in that direction.

DLu and others added 11 commits November 21, 2024 15:59
@kscottz
Copy link
Collaborator

kscottz commented Nov 22, 2024

@DLu went and merged and resolved all of my comments. If someone else signs off I am good to go on this.

I'll do my part to make sure people know about this once it gets merged.

Your help is appreciated. 🫡

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-news-for-the-week-of-november-18th-2024/40777/1

Fix lint complaints

Signed-off-by: Katherine Scott <[email protected]>
One more pass on trailing whitespace

Signed-off-by: Katherine Scott <[email protected]>
Now adding whitespace.

Signed-off-by: Katherine Scott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants