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

Move user data storage to Node from Visual #58

Open
osrf-migration opened this issue Feb 28, 2020 · 2 comments
Open

Move user data storage to Node from Visual #58

osrf-migration opened this issue Feb 28, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@osrf-migration
Copy link

Original report (archived issue) by John Shepherd (Bitbucket: John Shepherd, GitHub: jshep1).


Summary

I propose that we move the functions SetUserData and UserData currently residing within the Visual to the Node class in Dome. This will not affect the current usage of the function, but could provide better scalability.

Motivation

The functions SetUserData and UserData were introduced in This PR. It's become quite handy in that certain data can be accessed and set at the plugin level without having to depend upon other classes. Accessing a visual's entity id as well as being able to set booleans (via ints) in order to give a preview of what moving a visual to a given location might look like without having to make the service request has become quite useful. However, I do have to make downcasts from NodePtr -> VisualPtr in order to access this function which is not guaranteed to succeed. Moving both of these functions would maintain the current Visual user data support while also solving my issue of downcasting.

Describe alternatives you've considered

Currently, I'm casting from a Node to Visual which isn't guaranteed to succeed as a work-around.

What do you think? @iche033

@osrf-migration
Copy link
Author

Original comment by Ian Chen (Bitbucket: Ian Chen, GitHub: iche033).


yeah that sounds good to me. We released ign-rendering3 with this feature already so just need to make sure we don’t break ABI. If we want this in ign-rendering3, the function may need to be duplicated in Node class and we’ll just make a note to remove the one from Visual class in ign-rendering4

@osrf-migration
Copy link
Author

Original comment by Louise Poubel (Bitbucket: chapulina, GitHub: chapulina).


While at it, it would be nice to also add uint64_t to Variant, so that we can properly store ignition::gazebo::Entity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants