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

Query regarding creating a custom display plugin. #1310

Open
suchetanrs opened this issue Dec 12, 2024 · 4 comments
Open

Query regarding creating a custom display plugin. #1310

suchetanrs opened this issue Dec 12, 2024 · 4 comments
Labels
help wanted Extra attention is needed

Comments

@suchetanrs
Copy link
Contributor

suchetanrs commented Dec 12, 2024

Hello!

I had a question while writing a custom display plugin to visualize my custom messages.

I have the following custom message

std_msgs/Header header
geometry_msgs/PointStamped point
geometry_msgs/PolygonStamped polygon
geometry_msgs/PoseStamped pose

In my display plugin, I thought it would be simpler to directly use the processMessage function from rviz_default_plugins individually for the point, polygon and the pose.
I've got the PointStamped section and the PolygonStamped section of my custom message to work fine because the processMessage function was public and protected respectively and hence accessible by my custom display plugin. However, it's not the case with the default PoseStamped rviz display plugin. The processMessage is made private here.

Any suggestions on how I should proceed in this case? Was this function supposed to be protected / public?

Thanks for your help in advance!

@suchetanrs suchetanrs changed the title Query regarding custom display plugin. Query regarding creating a custom display plugin. Dec 12, 2024
@suchetanrs
Copy link
Contributor Author

suchetanrs commented Dec 13, 2024

After looking at the codebase further, I observe that the access control doesn't seem to be consistent across different messages that inherit from rviz_common::MessageFilterDisplay.

Message Type Status
AccelStamped processMsg is private
DepthCloudDisplay processMsg is public
Effort processMsg is private
GridCells processMsg is public
Map processMsg is public
Odometry processMsg is public
Path processMsg is public
PointStamped processMsg is public
PoseArray processMsg is public
Range processMsg is public
Twist processMsg is private
Wrench processMsg is public

In use cases such as mine, I feel like it is best to leave it public / protected for all of them. Will a PR help here? Please let me know.

@clalancette
Copy link
Contributor

I'm fine with switching them all to protected. If you'd like to make a PR to switch all of them, please go ahead.

@clalancette clalancette added the help wanted Extra attention is needed label Dec 13, 2024
@suchetanrs
Copy link
Contributor Author

Great! Thanks. I'll raise a PR for this.

@suchetanrs
Copy link
Contributor Author

Hello @clalancette !
There were a few errors in the table above. I've updated it.
I have raised a PR #1311 which moves the private ones to protected.
I was not able to move the public ones to protected because doing that would not allow me to compile the unit tests for those displays. Please let me know what I can do in this case :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants