LinkInterface/Vehicle uses mutexes to synchronize writing bytes

[DonLakeFlyer]

Previously writing bytes out to a link went through a sequence of multiple queued messages in Vehicle and LinkInterface to prevent thread safety issues. The problem is that doing it that way is dog slow. So slow that in cases where a joystick was being used on a device with a very slow CPU the MANUAL_CONTROL messages didn't make it through to the vehicle at a high enough frequency. It also chew more CPU than needed.

What happens now is that both Vehicle and LinkInterface use mutexes for thread safety in the following calls which are threadsafe:

• LinkInterface::writeBytesThreadSafe
• Vehicle::sendMessageOnLinkThreadSafe
• Vehicle::sendJoystickDataThreadSafe - Joystick calls this directly instead of posting queued signal

The end result is Mavlink message sending QGC->Vehicle which is measured in nanoseconds instead of 100s of milliseconds. And also when using a joystick a significant reduction in CPU usage.

[dogmaphobic]

Is this lock needed given that link->writeBytesThreadSafe() has its own lock? What is it protecting?

[DonLakeFlyer]

Makes sure this stuff is thread safe as well:

    if (!link->isConnected()) {
        return false;
    }

    // Give the plugin a chance to adjust
    _firmwarePlugin->adjustOutgoingMavlinkMessage(this, link, &message);

[DonLakeFlyer]

Since the joystick send will come through this path from another thread.

[DonLakeFlyer]

This link->isConnected() probably isn't too problematic. But this _firmwarePlugin->adjustOutgoingMavlinkMessage is since it maintains a mavlink channel status if I remember correctly.

[dogmaphobic]

Yup, It doesn't look like it is needed. The adjustOutgoingMavlinkMessage() function is 99.9% void. The only time something actually is done is for this:

    case MAVLINK_MSG_ID_PARAM_SET:
        _handleOutgoingParamSet(vehicle, outgoingLink, message);

Might as well put a mutex in there rather than annoying the other 99.9% of the time this never used.

[DonLakeFlyer]

The problem is that it's a firmwareplugin override so you have no idea what can happen in there in a custom build. Without a change to the FirmwarePlugin method to mark as threadsafe required and likely a name change to prevent old unthreadsafe code from leaking through it needed. I can certainly do that. Seems like a better way to go intstead of the upper layers doing things which won't be needed most of the time.

[DonLakeFlyer]

Also there is gimbal control signal which when that gets fully implemented should switch to using this path as well.

[DonLakeFlyer]

I should have also mentioned that this pull removes all the advanced joystick modes. Only joystick usage for RC is now supported.

[DonLakeFlyer]

Crap, closed by accident.

[DonLakeFlyer]

@dogmaphobic All good now?

[dogmaphobic]

@dogmaphobic All good now?

Yup! :)

[lukegluke]

@DonLakeFlyer, @dogmaphobic this one leads to #9751