search

KumarRobotics / kr_mav_control

Code for quadrotor control
BSD 3-Clause "New" or "Revised" License
106 stars 42 forks source link

Refactor/namespace #127

Closed tdinesh closed 4 years ago

versatran01 commented 4 years ago

One suggestion, in CMakeLists maybe use ${PROJECT_NAME}_node instead of kr_xxx_node.

tdinesh commented 4 years ago

One suggestion, in CMakeLists maybe use ${PROJECT_NAME}_node instead of kr_xxx_node.

This should work for packages with only single nodes.

Lets address CMake version updates (>3), cleanups in another PR. This one has already too many changes.

kartikmohta commented 4 years ago

Since this is going to be a huge breaking change, what do you think about changing quadrotor to mav at a bunch of places? kr_quadrotor_msgs -> kr_mav_msgs and so on

tdinesh commented 4 years ago

Since this is going to be a huge breaking change, what do you think about changing quadrotor to mav at a bunch of places? kr_quadrotor_msgs -> kr_mav_msgs and so on

Makes sense.

tdinesh commented 4 years ago

@kartikmohta does a new launch package kr_qc_launch to consolidate all the launch files, configs, scripts sound good? Currently they are strewn across kr_quadrotor_simulater, kr_mav_manager and trackers_manager.

That way anyone can just copy this launch folder into their own separate package for individual robots.

kartikmohta commented 4 years ago

Yeah, that is a good idea, we had something similar in FLA too :slightly_smiling_face:

tdinesh commented 4 years ago

@versatran01 @kartikmohta Can you test these and finalize?

kartikmohta commented 4 years ago

Will try it this weekend. The day job keeps me quite busy during the week :slightly_smiling_face:

tdinesh commented 4 years ago

Thanks! Just pushing this as multiple packages are getting broken with this PR. Tired of switching branches.

For testing (mostly for the multi robot scripts) use the following branches kr_ui mrsl_quadrotor multi_mav_manager

tdinesh commented 4 years ago

I think the only remaining API breaking change I can think of is removing/keeping Action suffix in some of the Tracker names. For example kr_trackers/LissajousTrackerAction vs kr_trackers/TrajectoryTracker. Both have action interfaces, but the nomenclature is different.

Thoughts @ljarin (I think you pointed this out earlier), @kartikmohta

kartikmohta commented 4 years ago

I guess removing the Action suffix is good, it's just a historical artifact that it's there in the name.

tdinesh commented 4 years ago

I think these are all the changes from my end. Unless someone else has API breaking suggestions :)

Ready to be merged!

tdinesh commented 4 years ago

@kartikmohta Yuezhan @tyuezhan has tested all the examples. Ready to merge?