search

ros-geographic-info / geographic_info

ROS packages for geographic information
http://ros.org/wiki/geographic_info
60 stars 61 forks source link

changing uuid_msgs/UniqueID to unique_identifier_msgs/UUID #26

Closed mlautman closed 4 years ago

mikaelarguedas commented 5 years ago

@bmagyar When the time for merging this has come, could you please consider reviewing/merging https://github.com/ros-geographic-info/geographic_info/pull/23 before applying this to remove unused code before updating it?

klintan commented 4 years ago

Any news on this one ?

klintan commented 4 years ago

@mlautman I tried this out with robot_localization and I got a dynamic linking error, related to Connext (which should be an optional middleware) 

dyld: Library not loaded: @loader_path/libnddsc.dylib
  Referenced from: /ros2/ros2-osx/lib/librosidl_typesupport_connext_c.dylib
  Reason: image not found

I think it is related to all these exported dependencies, I don't think it should be exported, only ament_export_dependencies(rosidl_default_runtime) ? At least it seemed to work in robot_localization when only that is exported.

ament_export_dependencies(ament_cmake)
ament_export_dependencies(std_msgs)
ament_export_dependencies(geometry_msgs)
ament_export_dependencies(unique_identifier_msgs)
ament_export_dependencies(rosidl_default_generators)
ament_export_dependencies(rosidl_default_runtime)

Maybe you'll want to investigate/fix that in this PR before merge.

klintan commented 4 years ago

Any chance we could get this one going ? It's a dependency of robot_localization navsat_transform_node and not having it released for ROS2 complicates things. Let me know if there is anything I can do to push these ROS2 PRs forward.

SteveMacenski commented 4 years ago

@mlautman closing after geographic-info ros2 branch update #28 which takes care of it.

I see you have a file for ROS1->2 conversion, if that's still required, I'd merge a PR with that.

mlautman commented 4 years ago

Sorry for falling off the face of the earth for a bit there. I am glad to see that #28 got merged.

Feel free to take the conversion.yaml and put it in another PR