PX4 / PX4-Autopilot

PX4 Autopilot Software
https://px4.io
BSD 3-Clause "New" or "Revised" License
8.26k stars 13.42k forks source link

Goto/reposition needs to be its own mode and not execute in Hold #14576

Open MaEtUgR opened 4 years ago

MaEtUgR commented 4 years ago

Describe problem solved by the proposed feature As brought up many times before it's undesirable to have the vehicle fly away from it's current position in hold mode and goto/reposition commands https://mavlink.io/en/messages/common.html#MAV_CMD_DO_REPOSITION should get executed in a goto mode and not in Hold mode. This allows dedicating Hold/Loiter mode to pausing what was done before and wait at the current location.

Additional context This was brought up multiple times by e.g. @hamishwillee @julianoes @DonLakeFlyer

bresch commented 4 years ago

Yes, I totally agree. If we do that, hold can send zero velocities and NAN position in the triplet, and the drone will stop nicely, without overshoot.

MaEtUgR commented 4 years ago

If we do that, hold can send zero velocities and NAN position in the triplet, and the drone will stop nicely, without overshoot.

To the trajectory generation not the position controller right? Because the trajectory generator takes care to not dirft, the position controller will not do more than integral control and still drift over time.

dagar commented 4 years ago

I also agree, but keep in mind it might be a little tricky to get this changed cleanly everywhere. At the moment we're dependent on px4_custom_mode.h being manually deployed/copied everywhere and I don't believe there's any real capability check at the mavlink level for this.

Maybe we could do something to get the new mode (and capability check) in place relatively soon to give it time to trickle out. Then in the mean time we could do something like have the new mode operate within HOLD after the first valid DO_REPOSITION is received.

DonLakeFlyer commented 4 years ago

So I was wrong about how I implement actually pausing the vehicle in QGC with PX4 Firmware. It does this:

void PX4FirmwarePlugin::pauseVehicle(Vehicle* vehicle)
{
    vehicle->sendMavCommand(vehicle->defaultComponentId(),
                            MAV_CMD_DO_REPOSITION,
                            true,   // show error if failed
                            -1.0f,
                            MAV_DO_REPOSITION_FLAGS_CHANGE_MODE,
                            0.0f,
                            NAN,
                            NAN,
                            NAN,
                            NAN);
}

QGC doesn't not specifically put the vehicle in Hold flight mode for Pause, the firmware does. I wrote this code ages ago, I vaguely remember some issue that this allow Pause to work in more cases than changing flight mode to Hold. But my memory may be incorrect.

The current problem is that I can't determine when Pause should be enabled since the test for that is the Vehicle being in Hold flight mode.

A more direct method would be Hold really means the vehicle is holding position. QGC enabled pause when flight mode is not Hold and pauses the vehicle by putting it into Hold. Which I believe matches this discussion and how the proposed solution.

hamishwillee commented 4 years ago

A more direct method would be Hold really means the vehicle is holding position. QGC enabled pause when flight mode is not Hold and pauses the vehicle by putting it into Hold. Which I believe matches this discussion and how the proposed solution.

Yes.

should get executed in a goto mode and not in Hold mode

More generic that just a goto mode because we'd also do Orbit in this context. I believe QGC uses the (internal) concept of Guided mode, meaning guided by QGC.