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

Handling a byte with message_to_yaml and set_message_fields #14

Open
squizz617 opened this issue Nov 8, 2021 · 1 comment
Open

Handling a byte with message_to_yaml and set_message_fields #14

squizz617 opened this issue Nov 8, 2021 · 1 comment
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@squizz617
Copy link

squizz617 commented Nov 8, 2021

Hi,
I came across an issue of set_message_fields not handling the Byte type correctly. Please take a look at the cases below.

Cases

  1. Directly via ros2cli
$ ros2 topic pub --once /dummy_topic std_msgs/Byte '{data: "\x00"}'
Failed to populate field: Value '' is expected to be a dictionary but is a str
$ ros2 topic pub --once /dummy_topic std_msgs/Byte "{data: '\x00'}"
Failed to populate field: Value '\x00' is expected to be a dictionary but is a str
  1. Invoking message_to_yaml and then set_message_fields
#!/usr/bin/python3

from rosidl_runtime_py import set_message_fields, message_to_yaml
import yaml
from std_msgs.msg import Byte

msg = Byte()
print(msg) # std_msgs.msg.Byte(data=b'\x00')
msg_yaml = message_to_yaml(msg)
print(msg_yaml) # data: "\0"

# mimic the behavior of ros2 topic pub
values_dictionary = yaml.safe_load(msg_yaml)
print(values_dictionary) # {'data': '\x00'}
set_message_fields(msg, values_dictionary) # fails

I end up getting the following error:

Traceback (most recent call last):
  File "/opt/ros/foxy/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py", line 55, in set_message_fields
    value = field_type(field_value)
TypeError: string argument without an encoding

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/ros/foxy/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py", line 39, in set_message_fields
    items = values.items()
AttributeError: 'str' object has no attribute 'items'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "p.py", line 8, in <module>
    set_message_fields(msg, values_dictionary)
  File "/opt/ros/foxy/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py", line 58, in set_message_fields
    set_message_fields(value, field_value)
  File "/opt/ros/foxy/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py", line 41, in set_message_fields
    raise TypeError(
TypeError: Value '' is expected to be a dictionary but is a str

Reason

When message_to_yaml processes a message and returns a YAML string, the byte string is no longer of type 'byte' because _convert_value coverts the byte to a string. When this YAML string is loaded as a dictionary through yaml.safe_load(values) and set_message_fields is invoked with this dictionary, the data e.g., "\x00", is regarded as a string, while the type information claims it's a byte.

Thoughts / suggestions

I am aware that a byte-typed data can be published via the following command:

$ ros2 topic pub --once /dummy std_msgs/Byte "{data: {0}}" 

However, as per the docstring, message_to_yaml "Converts a ROS message to a YAML string", and ros2 topic pub takes args as a YAML string (doc), an attempt to publish a byte message converted with message_to_yaml should not fail.

In addition, ros2interface proto, which relies on message_to_yaml to show the prototype of a message, prints the following result, which can mislead the users into trying to publish a byte as advised and fail.

$ ros2 interface proto std_msgs/msg/Byte
"data: "\0"
"

$ ros2 topic pub --once /dummy std_msgs/Byte "{data: '\0'}"
Failed to populate field: Value '\0' is expected to be a dictionary but is a str

So, I think either

  • message_to_yaml should return "data: {value}" if datatype is byte, or
  • set_message_fields should be modified to be able to tell a string from an escaped byte..

What do you think?
Thanks!

@mintar
Copy link

mintar commented Sep 7, 2022

I agree, I was just hit by the same bug. In my mind, message_to_ordereddict and set_message_fields should be the inverse of each other. In other words, if you use message_to_ordereddict to convert ros_msg_in to dictionary and then use set_message_fields to convert dictionary to ros_msg_out, then it should hold that ros_msg_in == ros_msg_out. This works for all ROS messages except Byte:

from std_msgs.msg import Float32, Byte
from rosidl_runtime_py.convert import message_to_ordereddict
from rosidl_runtime_py.set_message import set_message_fields

# Float32 (and all other types) work as expected:
float_in = Float32(data=1.23)
float_dict = message_to_ordereddict(float_in)
float_out = Float32()
set_message_fields(float_out, float_dict)
float_in == float_out
# result: True

# Byte fails:
byte_in = Byte(data=bytes([5]))
byte_dict = message_to_ordereddict(byte_in)
# byte_dict == OrderedDict([('data', '\x05')])
byte_out = Byte()
set_message_fields(byte_out, byte_dict)
# result:
# ---------------------------------------------------------------------------
# TypeError                                 Traceback (most recent call last)
# /opt/ros/galactic/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py in set_message_fields(msg, values)
#      54             try:
# ---> 55                 value = field_type(field_value)
#      56             except TypeError:
#
# TypeError: string argument without an encoding
#
# During handling of the above exception, another exception occurred:
#
# AttributeError                            Traceback (most recent call last)
# /opt/ros/galactic/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py in set_message_fields(msg, values)
#      38     try:
# ---> 39         items = values.items()
#      40     except AttributeError:
#
# AttributeError: 'str' object has no attribute 'items'
#
# During handling of the above exception, another exception occurred:
#
# TypeError                                 Traceback (most recent call last)
# <ipython-input-39-7056841faebb> in <module>
# ----> 1 set_message_fields(byte_out, byte_dict)
#
# /opt/ros/galactic/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py in set_message_fields(msg, values)
#      56             except TypeError:
#      57                 value = field_type()
# ---> 58                 set_message_fields(value, field_value)
#      59         rosidl_type = get_message_slot_types(msg)[field_name]
#      60         # Check if field is an array of ROS messages
#
# /opt/ros/galactic/lib/python3.8/site-packages/rosidl_runtime_py/set_message.py in set_message_fields(msg, values)
#      39         items = values.items()
#      40     except AttributeError:
# ---> 41         raise TypeError(
#      42             "Value '%s' is expected to be a dictionary but is a %s" %
#      43             (values, type(values).__name__))
#
# TypeError: Value '' is expected to be a dictionary but is a str

# With `bytes([5])` instead of `'\x05'`, it works:
byte_dict_fixed = OrderedDict([('data', bytes([5]))])
set_message_fields(byte_out, byte_dict_fixed)
byte_in == byte_out
# result: True

Solution

Change the following code:

if isinstance(value, bytes):
if truncate_length is not None and len(value) > truncate_length:
value = ''.join([chr(c) for c in value[:truncate_length]]) + '...'
else:
value = ''.join([chr(c) for c in value])

to this:

if isinstance(value, bytes):
    pass

Or alternatively, if you must keep the truncation:

if truncate_length is not None and len(value) > truncate_length:
    value = bytearray(''.join([chr(c) for c in value[:truncate_length]]) + '...', 'utf-8')

@clalancette clalancette added bug Something isn't working help wanted Extra attention is needed labels Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants