Cool, thanks! However, you have to do the change in https://github.com/mavlink/MAVSDK-Proto first and then run the generation, and then add your implementation.

Also see: https://mavsdk.mavlink.io/main/en/cpp/contributing/plugins.html#add-api-to-proto

I had one CI error. I needed to run tools/run-docker.sh tools/generate_docs.sh --overwrite.
I think that is missing from the contribution guide. Otherwise, this is good for review now.

Thanks @Ryanf55. Can you just confirm what value "toggle" is? Is it 2? I would like to make the mavlink PR and then update the docs here accordingly.

Thanks @Ryanf55. Can you just confirm what value "toggle" is? Is it 2? I would like to make the mavlink PR and then update the docs here accordingly.

For ArduPilot, any value other than 0 and 1 is considered a toggle.
I would like to avoid scope creep on undocumented behaviors in AP and focus on what's in the mavlink spec now.
Is there a reason we can't add toggle later once it's in the official mavlink spec?

I just thought it's a good time to do the spec work before we forget about it again. I couldn't actually see the source for toggle in Mission Planner, but I did see the else clause in ArduPilot. Having an action for "all other values" is generally a bad idea because it means you can't add any more values without breaking backwards compatibility.

I understand you want to get this PR in ASAP but you have to understand that as a MAVSDK maintainer I don't have the same urgency.

If you need it right now, you can already fire of the command with MavlinkPassthrough or MavlinkDirect by the way.

Ok. Of note, QGC only supports 0 and 1 as values.
https://github.com/mavlink/qgroundcontrol/blob/90b06c065093230d03808ccb58fae815ca7f67be/src/FirmwarePlugin/APM/APM-MavCmdInfoCommon.json#L52

I can find time to test mission planner and check the MAVLink inspector to see what value it is using as toggle.

I tested mission planner. The "Toggle" button in the relay panel does not make sense at all.

If the relay is currently low, the toggle button sends a command for high then low.

If the relay is high, it issues a low command, a high command, then a low command.

So, toggle will change the state of the relay, but the ending state is different depending on the initial state.

It is only sending values of 0 or 1, not something like -1 or 2.

Thus, the code path we saw in AP for toggle feature seems unused by either GCS or MP.

Ok, so this was a false herring. Should we just make it an easy boolean then to set 0 or 1? I usually want the API surface with MAVSDK to be as small as possible instead of open ended interfaces that we tend to have in MAVLink.

Ok, so this was a false herring. Should we just make it an easy boolean then to set 0 or 1? I usually want the API surface with MAVSDK to be as small as possible instead of open ended interfaces that we tend to have in MAVLink.

What do you think about an enum

enum RelayState {
0: off
1: on
}

Then we can add toggle later without breaking ABI.

Perfect

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
14.2% Duplication on New Code

See analysis details on SonarQube Cloud

@Ryanf55 I hope you're ok that I took this over and pushed it over the finish line.