Porting to ROS2 #48
Porting to ROS2 #48
Conversation
RoboticsYY
force-pushed
the
pr-ros2
branch
4 times, most recently
from
1287a10
to
09c78ea
Compare
| const std::string md5 = Md5().value(); | ||
| const std::string datatype = rosidl_generator_traits::data_type<M>(); | ||
| // TODO: Get MD5Sum value | ||
| // typedef typename ros::message_traits::MD5Sum<M> Md5; |
There was a problem hiding this comment.
I'm not sure if md5 will be supported and also I didn't see any other corresponding trait. But it seems like hashing is critical here. Couldn't we just hash the serialized message type string using std::hash https://en.cppreference.com/w/cpp/utility/hash?
There was a problem hiding this comment.
Great idea! I found a method from openssl to get the md5sum string from the message type. But I think this is still not equal to the function on ros1 that returns the hash number of the message definition. I'm not sure if this is enough for the application.
There was a problem hiding this comment.
Looks good to me! Are you sure this is different? ROS1 also seems to hash the message type, but then again this check should actually compare if the message definition matches. For now this should work though, as this should really just protect against using mismatching message versions. Maybe we could throw a warning about this.
RoboticsYY
force-pushed
the
pr-ros2
branch
8 times, most recently
from
d29dc9a
to
2ce731d
Compare
There was a problem hiding this comment.
@RoboticsYY Looks good to me, if you think this is ready I'll merge this.
RoboticsYY
force-pushed
the
pr-ros2
branch
2 times, most recently
from
2a905be
to
459ca95
Compare
RoboticsYY
force-pushed
the
pr-ros2
branch
2 times, most recently
from
689101d
to
48259c9
Compare
RoboticsYY
force-pushed
the
pr-ros2
branch
3 times, most recently
from
a0ee1f7
to
506449f
Compare
.travis.yml
Outdated
| @@ -1,6 +1,6 @@ | |||
| # This config file for Travis CI utilizes https://github.com/ros-planning/moveit_ci/ package. | |||
| sudo: required | |||
| dist: trusty | |||
| dist: xenial | |||
There was a problem hiding this comment.
| dist: xenial | |
| dist: bionic |
There was a problem hiding this comment.
This is not critical, though
|
@RoboticsYY These are really a lot of commits. Are you planning to group them in chunks? Otherwise I would squash-merge |
|
@henningkayser It's OK for me to make a squash-merge. |
This PR intends to port the
warehouse_rospackage to ROS2.The migration involves:
tftotf2.