search

mehmetkillioglu / ros2_message_converter

Ros2_message_converter is a lightweight ROS2 package and Python library to convert from Python dictionaries and JSON messages to rclpy messages, and vice versa
BSD 3-Clause "New" or "Revised" License
11 stars 7 forks source link

Fix timestamp conversions #1

Closed bbferka closed 3 years ago

bbferka commented 3 years ago

The current implementation fails to convert timestamps to dicts and back; To reconstruct simply run: 

from geometry_msgs.msg import TransformStamped
from ros2_message_converter import message_converter                                                                                
t = TransformStamped()
d = message_converter.convert_ros_message_to_dictionary(t) 
message_converter.convert_dictionary_to_ros_message('geometry_msgs/TransformStamped', d)

This will throw the following AttributeError: 

  File "/home/bbferka/work/ros2/scanning_ws/build/ros2_message_converter/ros2_message_converter/message_converter.py", line 290, in _convert_from_ros_time
    'secs'  : field_value.secs,
AttributeError: 'Time' object has no attribute 'secs'

This PR addresses this issue by first renaming nsecs and secs to nanosec and sec respectively and by actually disabling the check for timestamp field_types. In ROS2 Timestamps are just another type of message, so general conversion works fine.

mehmetkillioglu commented 3 years ago

Hi. Thank you for the pull request. This package was created to handle communication with "rosbridge_suite" using WebSocket and JSON messages. However, it hasn't been maintained for a year and is designed to work with ROS2 Dashing and ROS1 Kinetic in the first place.

The communication using the rosbridge was between ROS1 Websocket server - ROS2 client for an internal project. Therefore, the dictionaries created with convert_ros_message_to_dictionary actually generate the dictionary for ROS1, which is nsecs and secs. Therefore, dictionaries are processed and generated as ROS1 type, and messages are processed and generated as ROS2 type. This might be the cause of that error. Also, the latest updated branch was foxy, not master. It might be fixed on that branch. As I mentioned, this package was for a specific internal project and might not be suitable for all cases.

I might divide the message conversion to be generic, so the generated dictionaries might be configured to be either ROS1 type or ROS2 type. I will get back to this when I have time.

Again, thank you for the PR. I appreciate it.

bbferka commented 3 years ago

@mehmetkillioglu thanks for getting back on this; Will close this PR now since the changes of @marshallpt are more complete;