Px4-autopilot: Offboard Review

[Stifael]

The offboard implementation is very confusing for several reason:

• not everything is documented (in PX4 and mavlink)
• race condition is not taking care of (messages are sent to commander and mc_pos_control at the same time)
• triplet message is misused

I wanted to do some offboard refactor for flighttask, but then I realized that there are a few code implementation that are difficult to understand.

A few missing pieces:

• mavlink docu is missing for offtboard takeoff/land/idle/loiter (https://github.com/PX4/Firmware/blob/master/src/modules/mavlink/mavlink_receiver.cpp#L868-L871)
• the mapping from mavlink typemask to offboard_control_mode to control_mode is not consistent:
  
  
  
  • ignore_position is checked for 0x7 (=111b): In PX4, control_mode.flag_control_position_enabled corresponds to horizontal direction, control_mode.flag_control_altitude_enabled corresponds to vertical direction. However, the mavlink type mask 0x7 (=ignore x and y and z) is mapped to control_mode.flag_control_position_enabled.
  
  
• https://github.com/PX4/Firmware/blob/master/src/modules/commander/commander.cpp#L3733-L3736: why does attitude control depend on the outer loop as well?

[AlexisTM]

@Stifael

For the outer loop, it's that the attitude control needs the yaw/yaw_rate as input.

[Stifael]

For the outer loop, it's that the attitude control needs the yaw/yaw_rate as input.

Ok, but still don't understand why position and velocity is required...
The input to the attitude control is an attitude quaternion and a scalar thrust.

[AlexisTM]

You can send "body_rates", "roll pitch yaw thrust" or even "actuator control" commands over OFFBOARD mode.

Therefore, depending on the input, we have to enable controls depending on the lowest level control. If you receive body_rate from the offboard companion,

You will not run the attitude control which outputs body_rate setpoints or lower.
You will not run the body_rate controller if you receive actuator setpoints or lower.
You will not run position controller if you receive velocity setpoints or lower.

Indeed there is one problem. If you ignore everything in your offboard message (SET_TARGET_LOCAL_NED) with a flag ignoring everything, the copter just shuts down the motors, because the different controls are not in one message, so we cannot check the consistency of the command.

A solution to it was proposed here, to only allow 4 degrees of freedom in the fancy you prefer: http://discuss.px4.io/t/idea-for-a-more-versatile-offboard-control/4721 Yet, it did not go further as the offboard mode is currently reworked using Flight Tasks (see the PR about it https://github.com/PX4/Firmware/pulls?utf8=%E2%9C%93&q=is%3Apr+flighttask)

The idea is to give a single channel where people use OFFBOARD commands, thus you don't have race conditions on the control and you can only control 4 degrees at the same time (X, Y, Z, Yaw).

[Stifael]

You will not run the attitude control which outputs body_rate setpoints or lower.
You will not run the body_rate controller if you receive actuator setpoints or lower.
You will not run position controller if you receive velocity setpoints or lower.

I think we are talking about the same thing, but I am questioning the implementation.
For instance, in the current firmware for attitude control from offboard, all outer loops have to be true (or not ignored)
https://github.com/PX4/Firmware/blob/master/src/modules/commander/commander.cpp#L3743-L3746

Yet, it did not go further as the offboard mode is currently reworked using Flight Tasks (see the PR about it https://github.com/PX4/Firmware/pulls?utf8=%E2%9C%93&q=is%3Apr+flighttask)

I know that there is a PR because I opened it. However, there is no reason to not continue to clean up the offboard implementation as you suggested here http://discuss.px4.io/t/idea-for-a-more-versatile-offboard-control/4721, although with Flighttask it can be farther simplified.

[hamishwillee]

@Stifael At the end of this process can we please review and update:

• https://docs.px4.io/en/flight_modes/offboard.html
• https://dev.px4.io/en/ros/offboard_control.html

[FaboNo]

I am trying to understand the offboard mode on my side as well ans I would like to clarify the following points:
1- looks like you can only send SET_TARGET_LOCAL_NED mavlink msg (which corresponds to SETPOINT_RAW mavros msg), Am I correct? Because the example given in https://dev.px4.io/en/ros/mavros_offboard.html is using setpoint_position/local mavros msg and there is no velocity in it?

1. you can send mavlink/mavros msg only on TELEM2 for the offboard mode? Otherwise (TELEM1 or USB), they will not be taken into account?
   Thank you for helping me to understand the offboard mode

[AlexisTM]

1. All the mavros topics are working. They'll be converted to the same Mavlink message on the backend.

2. I already tested the USB and TELEM2. I never tested TELEM1 as we use a GPS. I'm sure you can use TELEM1, but with the lowest rates.

[FaboNo]

Thank you Alexis. I am using the USB port to communicate with the FCU. I am trying to understand how the offboard is working because so far, I do not succeed in my experiments.
I have a SLAM which is sending poses thru /mavros/vision_pose/pose and I can see that LPE and the attitude_estimator_q are taking into account the inputs because the updated poses are available thru /mavros/local_position/pose BUT when I am activating the offboard mode and let'say send a 0,0,2 to take off and hovering, the mav is drifting like it does not lock on the position(s) issued by the slam.
At least this is my understanding so far, like an updated pose is computed but not sent to "low "level" controllers.
Do you have any idea of what I am missing here?

[colineRamee]

Hi,
I am also trying to understand the offboard mode and there's something that has been escaping me, and I would really appreciate some clarification.
In modules/mavlink/mavlink_receiver.cpp at line 874 I don't understand why these variables are defined like that:

bool is_takeoff_sp = (bool)(set_position_target_local_ned.type_mask & 0x1000);
bool is_land_sp = (bool)(set_position_target_local_ned.type_mask & 0x2000);
bool is_loiter_sp = (bool)(set_position_target_local_ned.type_mask & 0x3000);
bool is_idle_sp = (bool)(set_position_target_local_ned.type_mask & 0x4000);

Doesn't this lead to is_loiter_sp being true when the type mask starts with 0x1000 (= 0b1 00...) or 0x2000 (= 0b10 00..) (take_off or land), and to is_land_sp and is_take_off_sp being true when the type mask is 0x3000(= 0b11 00..)?
If I understand correctly this is somehow handled later line 913 by an if else if statement where a comment states that the order matter, but since loiter mode is first and is true for any message with 0x1000, 0x2000 or 0x3000, I don't understand how the setpoint type can ever be set to land or takeoff.
Am I missing something obvious? Please let me know.
Why not do something like:

bool is_takeoff_sp = !(bool)((set_position_target_local_ned.type_mask & 0xf000)^0x1000);
bool is_land_sp = !(bool)((set_position_target_local_ned.type_mask & 0xf000)^0x2000);
bool is_loiter_sp = !(bool)((set_position_target_local_ned.type_mask & 0xf000)^0x3000);
bool is_idle_sp = !(bool)((set_position_target_local_ned.type_mask & 0xf000)^0x4000);

Which would only return true when the leading 4 bits are exactly identical.

[azmat-bilal]

i would confirm that the is_takeoff_sp and is_land_sp dont work becuase of the reason mentioned above i.e. the '& 0x3000' condition gets true whenever either landing or takeoff bits are set, and since loiter is the first condition in the if else statement, it always put the drone in that state. as work around i have disabled the loiter functionality by

bool is_loiter_sp = (bool)(set_position_target_local_ned.type_mask & 0x0000);
but this is just to make landing/takeoff work. for proper implementation, there is work in progress in mavlink repo where loiter mask bit is moved to 0x4000 and idle to 0x8000. ref. https://github.com/mavlink/mavlink/blob/45a756eedabb9d8e83322575dd3384735541dbb5/message_definitions/v1.0/common.xml

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.