DFKI-NI / rospy_message_converter

Converts between Python dictionaries and JSON to ROS messages.
BSD 3-Clause "New" or "Revised" License
226 stars 101 forks source link

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

Closed nyxaria closed 4 years ago

nyxaria commented 4 years ago

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.

mintar commented 4 years ago

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 commented 4 years ago

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!