Replace offboard control setpoint topic with simple offboard control mode topic, remove setpoint data from topic

[thomasgubler]

This is a cleanup/rework of how mavlink offboard setpoints are handled. Because I plan to add offboard control support to ros sitl (in the mavlink dummy node) this was an important first step.

• No functionality change is intended.
• The PR is not tested yet.

@mhkabir @sjwilks please have a look

Overview of Changes

• Replace offboard_control_setpoint with offboard_control_mode
• Remove all setpoint data from the topic as it's not used anymore
  (setpoint data is directly routed into position/attitude setpoint topics
  for some time now)
• Remove mode enum and replace with ignore booleans which map better to
  the mavlink message
• Mavlink: Rework parsing of offboard setpoints
• Commander: in offboard mode set control flags based on ignore flags
  instead of enum

TODO

• Check why velocity control does not work

[aerialhedgehog]

looks like this still uses the same mavlink messages, so the c_uart_example should be unaffected

[thomasgubler]

@aerialhedgehog yes the change is Firmware internal only. Thanks for checking the c_uart_example

[LorenzMeier]

This looks great and like a really neat simplification!

[thomasgubler]

I just realised that this does not work in the current state if the sender uses an asynchronous pattern to send the data.
E.g send alternating set_attitude_target with throttle ignored (containing attitude data) and set_attitude_target with attitude ignored (containing throttle data).

Mavros uses exactly this pattern.

Maybe I need to add a mode enum again. However it will be simplified compared to the old version.
Another solution would be to ignore the ignore flags of the mavlink message if all attitude data (= {q, body angular rates}) is ignored.

[mhkabir]

@thomasgubler This is something we want to resolve in mavros itself.
The core of the issue being, we don't have any standard ROS message to transport an attitude target + throttle.
The ideal thing here would be to implement custom messages according to the SIG (http://wiki.ros.org/sig/MicroAirVehicle), but we've been holding off until the spec stabilizes. Perhaps we can implement the setpoint custom messages now and release them as a separate package.

[thomasgubler]

@mhkabir I pushed an quick attempt for a fix. I will have to recheck tomorrow if I got the logic right.

[thomasgubler]

I rebased and improved the attitude logic

[thomasgubler]

by @aerialhedgehog in #1810

Firmware/offboard (ref PR #1786)
velocity control - does not move
position control - works, but i can't control how fast it moves to the next set point

I updated the todo list in the top comment to reflect this.

[LorenzMeier]

@aerialhedgehog Would you have time to look at the velocity control bits?

[matt-beall]

I'm currently working on adding functionality to accept actuator control mavlink messages in the mavlink_receiver and to be able to set raw actuator controls while in offboard mode. The work in this branch will change how my code is working. I wonder the work should be combined? My fork is here: https://github.com/mcsauder/Pixhawk-Firmware/tree/actuatorControlOffboardMode

I've already merged the offboardmode code into my repo. Since my work is dependent on the offboard mode, should I submit a pull request for this to the offboardmode branch or simply to master?

[LorenzMeier]

@matt-beall Thomas will be able to provide more feedback. In general sending the PR against the offboardmode branch might make sense because it will make your changes more obvious. As it will go into master as well I don't see it delaying your work going into master.

[thomasgubler]

@matt-beall cool, looking forward to it! If this PR is still open once you are ready for a PR then you should do your PR against this branch. Otherwise against master. I would recommend that you frequently rebase your branch against this one while you are working on the feature.

[thomasgubler]

@aerialhedgehog What kind of setpoint did you send in your velocity setpoint test? How/by which tool was the setpoint generated? Was it a pure velocity setpoint or a combination of velocity and position?

[aerialhedgehog]

@LorenzMeier will do.
@thomasgubler i was running this example --
https://github.com/aerialhedgehog/c_uart_interface_example/blob/object/mavlink_control.cpp#L184

which makes a call to this function for velocity command
https://github.com/aerialhedgehog/c_uart_interface_example/blob/object/autopilot_interface.cpp#L105
and this function for yaw command
https://github.com/aerialhedgehog/c_uart_interface_example/blob/object/autopilot_interface.cpp#L155

This is where the bit masks are defined
https://github.com/aerialhedgehog/c_uart_interface_example/blob/object/autopilot_interface.h#L105

[thomasgubler]

I glanced over the code again, but I could not immediately see whats wrong with velocity setpoints.
@LorenzMeier would merging this and opening an issue to the regression be acceptable? I guess velocity setpoints are used by a very small fraction of the people merging regularly form master. Not merging this PR is blocking other branches.

[LorenzMeier]

@thomasgubler Makes sense. We want to keep moving ;-).