tu-darmstadt-ros-pkg / hector_quadrotor

hector_quadrotor contains packages related to modeling, control and simulation of quadrotor UAV systems.
Other
379 stars 276 forks source link

Migrate to new control structure #34

Closed paulbovbel closed 8 years ago

paulbovbel commented 9 years ago

Add new action and service types

Break controllers into attitude, velocity, and position

Split up _controller package into _interface and _controllers

Migrate to control_toolbox's PID implementation, waiting on https://github.com/ros-controls/control_toolbox/pull/38 for antiwindup equivalent functionality

Add _action package containing pose, landing, and takeoff action servers

Update teleop to new control structure

meyerj commented 8 years ago

Hi Paul,

I finally had the time to release the hector packages into jade and to review your patch this weekend. It seems like a valuable contribution to hector_quadrotor and I appreciate the amount of time you spent to implement this! You told me in Hamburg that you are not working on this project anymore, but perhaps you find some time to answer some questions below.

I consider to merge this branch into jade-devel to be released into jade as soon as the anti-windup patch would be merged into control_toolbox and ros_control is released. I think for indigo the changes are too heavy and the missing anti-windup patch might be a blocker. I merged the latest patches for jade into this branch, added some patches and pushed it as clearpathrobotics-asctec-merge-2.

Some questions:

  1. Do you want to maintain the new packages or can I update the maintainer tag in the package.xml files to myself?
  2. I assume all your contributions are also BSD licensed and I can copy the existing license headers to the new header and source files?
  3. Where does the branch name asctec-merge come from? Did you plan to use this stack to simulate an AscTec drone or are parts of the code motivated by AscTec code?

Open issues that I found so far:

I have not tested the interaction with hector_pose_estimation yet, if the quadrotor is spawned with use_ground_truth_for_control:=false. In this case the pose and twist is estimated from the simulated noisy IMU, magneto and GPS measurements and not taken from ground truth. Is it correct that you never tested this mode and the current PID parameters might be too tight with imperfect state information? I noticed that you even removed the hector_quadrotor_pose_estimation package in your working branch.

paulbovbel commented 8 years ago

@meyerj thanks for taking a look at this! I've been a little out-of-the-loop with github but if you have some time to sort through this pull and make improvements, I'd be happy to help any way I can.

I have a BSD licensed hardware driver for Asctec Pelican/Hummingbird, along with the associated URDFs for simulation, ready to go when this is merged and released. Of course we're still waiting for ros_control release into jade.

Do you see a clean way to do both the Euler-angle based approach, along with the vector difference? Perhaps it may be best just to have 4 controllers (position, velocity, attitude, and your original direct-velocity controller)

I would be happy to take maintainership of stuff I wrote independently, pass it to you, or we can share. I need to go carefully go through the headers of this pull, and the package.xml's, and make sure there's proper attribution.

With regards to execution order, it wasn't a huge deal for my application (running at 100 Hz with pretty low specification to hit), but I can see how that's an issue in more high-precision applications.

paulbovbel commented 8 years ago

Moved discussion to #44