add optional parameter binary_array_as_str=True to convert_ros_messag…

[michrag]

…e_to_json in order to fix issue #45 (with a test to validate the fix)

[mintar]

@Otacon5555 wrote in #45:

I see that the test I added is passing only on noetic and failing on both kinetic and melodic, but I actually have melodic (with python 3.6) and the test was passing.

Your setup is a bit unusual. The combinations that are officially supported by ROS are:

• Kinetic + Ubuntu 16.04 + Python 2.7
• Melodic + Ubuntu 18.04 + Python 2.7
• Noetic + Ubuntu 20.04 + Python 3.8

See here for details.

These three combinations are tested by the CI for this repo. So if only the Noetic test passes, that means that it only works on Python 3, and you need to fix it for Python 2.7. You can add more commits to this PR (= Pull Request) by pushing more commits to the master branch on your repo.

You can also click on the failed tests to see the output of the failed tests:

File "/ws/src/rospy_message_converter/test/test_message_converter.py", line 147, in test_ros_message_with_3uint8_array_binary_array_as_array

    self.assertEqual(dictionary["data"], expected_data)

AssertionError: Lists differ: ['a', 'b', 'c'] != [97, 98, 99]

As they say, "it works on my machine"...!

That's exactly why we have CI! :)

[michrag]

I'm having a hard time debugging it due to my unusual environment, so how can I print to the console? Simply adding print statements is not working. Thank you.

[mintar]

I'm having a hard time debugging it due to my unusual environment, so how can I print to the console? Simply adding print statements is not working. Thank you.

Maybe just copy the test contents into a regular small python script, from which you can print. You can use the Dockerfiles and the command from the .travis.yml to test your code, like this:

export CI_ROS_DISTRO="kinetic"
docker build --pull -t rospy_message_converter_$CI_ROS_DISTRO -f Dockerfile-$CI_ROS_DISTRO .

# interactive console:
roscd rospy_message_converter
docker run --rm -v $PWD:/ws/src/rospy_message_converter -it rospy_message_converter_$CI_ROS_DISTRO

# just run the tests
docker run --rm -v $PWD:/ws/src/rospy_message_converter rospy_message_converter_$CI_ROS_DISTRO /bin/bash -c "source devel/setup.bash && CTEST_OUTPUT_ON_FAILURE=1 catkin_make test"

[michrag]

Maybe just copy the test contents into a regular small python script, from which you can print

Good idea. Doing so I believe the problem is in the getattr function behaving differently on python3 than python2.

Inserting some print functions in the convert_ros_message_to_dictionary(message, binary_array_as_str=True):

function as follows

def convert_ros_message_to_dictionary(message, binary_array_as_str=True):
    print("message: ", message)
    dictionary = {}
    message_fields = _get_message_fields(message)
    print("message_fields: ", message_fields)
    for field_name, field_type in message_fields:
        field_value = getattr(message, field_name)
        print("field_name: ", field_name)
        print("field_type: ", field_type)
        print("field_value: ", field_value)
        dictionary[field_name] = _convert_from_ros_type(field_type, field_value, binary_array_as_str)
    print("dictionary: ", dictionary)
    return dictionary

I see two different outputs: the first one is with python2 and the second one is with python3

message:  data: [97, 98, 99]
message_fields:  [('data', 'uint8[3]')]
field_name:  data
field_type:  uint8[3]
field_value:  abc
dictionary:  {'data': ['a', 'b', 'c']}

message:  data: [97, 98, 99]
message_fields:  [('data', 'uint8[3]')]
field_name:  data
field_type:  uint8[3]
field_value:  b'abc'
dictionary:  {'data': [97, 98, 99]}

I honestly have no idea how to fix that.

P.S.: I also changed _get_message_fields as follows to make it behave the same in both python version

def _get_message_fields(message):
    return list(zip(message.__slots__, message._slot_types))

[mintar]

Ok, I've fixed and merged it in #49. I've also renamed the parameter to binary_array_as_bytes. Thanks!

[michrag]

Thanks to you!

All the tests are passing also on my environment.

Problem is, in my use case it was still not working, and I've found why (my bad in my patch): the new boolean value is not propagated in the next function calls, and since my array was nested, its value was lost before decoding it.

So, I've done the fix, it's very easy, maybe it's more efficient if you do it directly on this repo instead of me forking and creating a new pull request?

It's the following:

 src/rospy_message_converter/message_converter.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/rospy_message_converter/message_converter.py b/src/rospy_message_converter/message_converter.py
index 1de38e3..162de55 100644
--- a/src/rospy_message_converter/message_converter.py
+++ b/src/rospy_message_converter/message_converter.py
@@ -251,9 +251,9 @@ def _convert_from_ros_type(field_type, field_value, binary_array_as_bytes=True):
     elif _is_field_type_a_primitive_array(field_type):
         field_value = list(field_value)
     elif _is_field_type_an_array(field_type):
-        field_value = _convert_from_ros_array(field_type, field_value)
+        field_value = _convert_from_ros_array(field_type, field_value, binary_array_as_bytes)
     else:
-        field_value = convert_ros_message_to_dictionary(field_value)
+        field_value = convert_ros_message_to_dictionary(field_value, binary_array_as_bytes)

     return field_value

@@ -292,10 +292,10 @@ def _convert_from_ros_primitive(field_type, field_value):
         field_value = field_value.decode('utf-8')
     return field_value

-def _convert_from_ros_array(field_type, field_value):
+def _convert_from_ros_array(field_type, field_value, binary_array_as_bytes=True):
     # use index to raise ValueError if '[' not present
     list_type = field_type[:field_type.index('[')]
-    return [_convert_from_ros_type(list_type, value) for value in field_value]
+    return [_convert_from_ros_type(list_type, value, binary_array_as_bytes) for value in field_value]

 def _get_message_fields(message):
     return zip(message.__slots__, message._slot_types)

Thanks again!

[mintar]

Ok, done! #50