-
Notifications
You must be signed in to change notification settings - Fork 45
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
Added UTF-8 support for diagnostic messages to rqt_robot_monitor #80
Added UTF-8 support for diagnostic messages to rqt_robot_monitor #80
Conversation
@dirk-thomas Any thoughts on this? |
The code is incompatible with Python 3 which makes me hesitant to merge it as-is. |
Are there any resource on setting up a python 3 ROS installation? I spent a while searching for instructions and have been unable to find any. |
861d534
to
d056f66
Compare
No, sadly there are none. The easiest way is to create a Python 3 virtual env, install all ROS Python packages in there and then compile ROS from source using the CMake variable |
So while i'm still having some issues getting everything running with python3 (tf2 still hasn't been upgraded ros/geometry2#50 so I need to be more fine grain about what I compile) it looks like no modifications should be needed when running under python3. From looking at the generated python message serialization/deserialization code it looks like when running under python3 strings in messages are automatically decoded using UTF8 when they are received (for a simple example see the generated code for std_msgs/String). Would a pull request that uses a similar technique to the message generation to only activate when running under python 2 then be accepted? |
I actually did get rqt_robot_monitor running under python3, but get the following stack trace when running it:
|
That is weird. The code should include |
Don't know if there is something funky with my virtual env cause the directory |
d056f66
to
7d8ce0f
Compare
I'm going to close this because I don't really have a need for it any more and I think this may be better suited for the rospy core anyway (so it applies to all strings) |
Fixes #79