-
Notifications
You must be signed in to change notification settings - Fork 486
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
Support plotting for entities with / in the name #3187
Support plotting for entities with / in the name #3187
Conversation
I noticed while testing the rexrov models from the rexrov model in Field-Robotics-Lab/dave that the plotting window does not work properly when elements of a model include a forward-slash `/` in the entity name. To fix the problem, the `/` characters are encoded as %2f in physics::Base::URI(). Corresponding decoding is added to the gui/plot/Palette class. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
cc20b5e
to
0dc1a39
Compare
I noticed a CI build failure with gcc-8 that doesn't occur with gcc-9. Try to avoid failure by using std::string's compare function instead of an == operator with a const char * string literal. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, test passes for me. Just one minor comment but PR is fine as is.
gazebo/physics/Base.cc
Outdated
@@ -383,15 +384,19 @@ common::URI Base::URI() const | |||
{ | |||
if (p->GetParent()) | |||
{ | |||
uri.Path().PushFront(p->GetName()); | |||
std::string escapedParentName = p->GetName(); | |||
boost::replace_all(escapedParentName, "/", "%2f"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use common::replaceAll
instead of boost
? I do still see boost
being used in gazebo so maybe not a big deal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switched to common::replaceAll
in 7edb013
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
This would prevent a string containing "%2f" from incorrectly being unescaped to "/". Signed-off-by: Steve Peters <scpeters@openrobotics.org>
This fixes some downstream compilation issues. Follow-up to gazebosim#3187. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
* Don't include TaskGroup.hh from transport.hh This fixes some downstream compilation issues. Follow-up to #3187. * check_test_ran: don't use grep Check the file for the desired string directly in python without using grep to improve Windows support. * Use python3 with check_test_ran.py for qt tests Follow-up to #3155. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
I noticed while testing the rexrov models from
Field-Robotics-Lab/dave that the plotting window
does not work properly when elements of a model include
a forward-slash
/
in the entity name. To fix the problem,the
/
characters are encoded as %2f in physics::Base::URI().Corresponding decoding is added to the gui/plot/Palette class.
Fixes #3181.