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

String conversion to bytes breaks python3 std_msgs/String implementation #33

Closed
nyxaria opened this issue Apr 1, 2020 · 2 comments
Closed

Comments

@nyxaria
Copy link
Contributor

nyxaria commented Apr 1, 2020

When running this package in python 3, the conversion of a string fails because of the ros implementation of std_msgs/msg/_String.py. They call encode() on the data field of a string object due to python3 being always True:

def serialize(self, buff):
    """
    serialize message into buffer
    :param buff: buffer, ``StringIO``
    """
    try
      _x = self.data
      length = len(_x)
      if python3 or type(_x) == str:
        _x = _x.encode('utf-8')

However, the rospy_message_converter already does this when it converts a string python object to a ros_primitive in rospy_message_converter/src/rospy_message_converter/message_converter.py:149:

def _convert_to_ros_primitive(field_type, field_value):
    if field_type == "string":
        field_value = field_value.encode('utf-8')
    return field_value

The fix is to add a check that we are not in python3 before doing the conversion.

def _convert_to_ros_primitive(field_type, field_value):
    if field_type == "string" and not python3:
        field_value = field_value.encode('utf-8')
    return field_value

This works for std_msgs/String and sensor_msgs/JointState but I am not sure if this breaks anything else as I don't have the infrastructure in place to test this.

I will create a PR shortly.

nyxaria pushed a commit to nyxaria/rospy_message_converter that referenced this issue Apr 1, 2020
@mintar
Copy link
Member

mintar commented Apr 1, 2020

BTW, you seem to have copied one line from _String.py incorrectly. In my version (Kinetic) at least it looks like this, which makes much more sense:

      if python3 or type(_x) == unicode:

@nyxaria
Copy link
Contributor Author

nyxaria commented Apr 2, 2020

You are right, I was messing around with the source code. In python3 str is equivalent to unicode so I thought I would try that, thanks for reminding me I will change it back!

nyxaria pushed a commit to nyxaria/rospy_message_converter that referenced this issue Apr 2, 2020
Co-Authored-By: Martin Günther <martin.guenther@dfki.de>
mintar added a commit that referenced this issue Apr 8, 2020
Co-Authored-By: Martin Günther <martin.guenther@dfki.de>
@mintar mintar closed this as completed Apr 8, 2020
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

No branches or pull requests

2 participants