Move planning_ros_msgs to be an external dependency

[ljarin]

In order to address #133, it is useful for the messages used by the planner to be independent from the rest of the stack so that kr_mav_control can link against them.

• rename planning_ros_msgs -> kr_planning_msgs
• rename planning_ros_utils -> kr_planning_rviz_plugins
• remove both of the above from kr_autonomous flight (still todo: add to external repos)
• move primitive_ros_utils (converts btw MPL and kr_planning_msgs) to the action_planner since it is only used there
• still todo: data_type.h (defines common datatypes like Vec3f) and data_ros_utils (random data conversions) should not live in kr_planning_rviz_plugins, and should be in some common location. data_type is also copied from mpl/jps. state_machine, action_trackers, and action_planner are the packages that use data_ros_utils

[not finished yet, will fail CI]

[versatran01]

Can you also post links to the new packages?

[XuRobotics]

Great! Thanks, Laura. Once you add to the external and fix the CI issue, I will clean-build and test it on my side, before we merge.

[ljarin]

@versatran01 https://github.com/KumarRobotics/kr_planning_msgs

[ljarin]

OK, @XuRobotics and @versatran01 , sorry leaving this open for so long, but I think it is ready for a test.

The only thing remaining that I did not do is move https://github.com/KumarRobotics/kr_planning_msgs/blob/main/kr_planning_rviz_plugins/include/kr_planning_rviz_plugins/data_ros_utils.h and https://github.com/KumarRobotics/kr_planning_msgs/blob/main/kr_planning_rviz_plugins/src/utils/data_ros_utils.cpp to somewhere more logical/remove them from kr_planning_msgs repo. But I can do that if you think it shouldn't be merged without that.

[ljarin]

Also I assume the docker build github action should be run? I'm not sure how to trigger that?

[versatran01]

This is too big for a proper review. Since it's just moving stuff around, if it builds and works we can go ahead and merge it. One thing I want to add is that data_type.h and data_ros_utils.h in kr_planning_msgs have no namespace, better to add one to prevent pollution of the global namespace and may cause resolution issues with stuff in jps/mpl.

This data_type.h shows up in 3 different places. https://github.com/KumarRobotics/kr_autonomous_flight/blob/master/autonomy_core/map_plan/jps3d/include/jps/data_type.h https://github.com/KumarRobotics/kr_autonomous_flight/blob/master/autonomy_core/map_plan/mpl/include/mpl_basis/data_type.h https://github.com/KumarRobotics/kr_planning_msgs/blob/main/kr_planning_rviz_plugins/include/kr_planning_rviz_plugins/data_type.h

This is extremely bad design and should be fixed if possible (not in this PR of course, but later).

[ljarin]

I think you mean https://github.com/KumarRobotics/kr_autonomous_flight/blob/master/autonomy_core/map_plan/mpl/include/mpl_basis/data_type.h as the 3rd one

[ljarin]

Yes, I agree...On the other hand there's only little name alias statements in that file.

The easiest solution would be one copy in kr_planning_msgs, which doesn't make too much sense.

But sounds good if we're punting that for later.

[ljarin]

I will add a namespace for data_ros_utils

[versatran01]

This is just a personal preference but the namespace kr_planning_rviz_plugins has 24 characters and is a bit too long. Maybe we could just use our organizational namespace kr. For example https://github.com/KumarRobotics/kr_utils/blob/master/kr_math/include/kr_math/pose.hpp

[ljarin]

Sure, I don't feel strongly

In the meanwhile I have discovered a small rviz bug that I will resolve before we merge

[tdinesh]

@ljarin any updates on this? Is this ready to merge?

[ljarin]

I found a bug in the rviz plug-in right before I left. Will address this weekend, sorry 😔

[tdinesh]

I found a bug in the rviz plug-in right before I left. Will address this weekend, sorry pensive

Ping @ljarin

[ljarin]

Under the principle of better late than never... I think this is now working. The changes needed were in https://github.com/KumarRobotics/kr_planning_msgs so the only change to this PR was rebasing to master and changing the namespace name.

~Big caveat is at the moment this breaks the visualization of anything other than position of Trajectory and Primitive. Same as for SplineTrajectory, there is currently no yaw, velocity, acceleration, or jerk visualization any more (though I did not go as far as to remove the checkboxes). If this is important to somebody, maybe @XuRobotics ? I can fix it though.~

To remind you, this was decoupling the messages and RViz plugins from MPL so that you don't have to build the whole autonomy stack in order to use the messages/develop a separate package, which is my use case. Since MPL is no longer part of the Rviz plugins, I had to add back functions to calculate the position/velocity/etc. of the primitive.

[ljarin]

So I finally reimplemented the missing visualization features (https://github.com/KumarRobotics/kr_planning_msgs), would it be possible to give this a try @XuRobotics @fcladera ?