Offboard doesnt want to enable after a is_active() check.

[mcelhennyi]

MavSDK version v0.35.1

I am having an issue where the offboard does not want to enable. It seems like a few things...a race condition with a bad check (mavsdk side) and a bad way of checking the state of offboard (my side).

I was checking if the _offboard->is_active() but that was always true (because _mode != Mode::NotActive - it was PositionNed) before calling _offboard->start()...this was not a good way of doing it because even when the is_active switched to false so I could start the offboard (which happened periodically randomly) it would respond back with a failure because

        if (_mode == Mode::NotActive) {
            return Offboard::Result::NoSetpointSet;

(offboard_impl.cpp)^^

How I fixed it:

In process_heartbeat() i changed _mode != Mode::NotActive to _mode == Mode::NotActive . This caused the race condition of the NotActive being set too quickly to not happen anymore...

AND

I, instead of checking is_active(), I check that _flightMode != Telemetry::FlightMode::Offboard and this allowed it to go into the offboard mode and respect my movement commands.

My code is here.

I am not sure if the changes I made are proper and dont want to PR them without a discussion about this issue first.

I also have been conversing with @JonasVautherin here about this issue if you would like some more context.

[julianoes]

@mcelhennyi thanks for bringing this up. So you basically check the flight mode instead of what MAVSDK is set to, right? I'm not entirely convinced yet but let's discuss it.

Could you share the user code that uses the offboard API? I'm trying to figure out why it doesn't work for you in the way it's designed.

[mcelhennyi]

Yes, I swapped from is_active to just using the telemetry mode instead, since it would be more accurate. I think there should be a secondary call in offboard mode that is is_active() which returns what it returns now...and is_enabled or somehing like that which returns offboard mode is on or not.

Sure, my user code is here: https://github.com/mcelhennyi/NXP-HoverGames-2/blob/ebc11e9577710f4cf036a230838d943761f902f8/code/src/system/agent/Agent.cpp#L305

[julianoes]

By the way:

https://github.com/mcelhennyi/NXP-HoverGames-2/blob/ebc11e9577710f4cf036a230838d943761f902f8/code/src/system/agent/Agent.cpp#L118-L123

This can just be:

std::cout << "Drone is in LandedState: " << landedState << std::endl;

[julianoes]

https://github.com/mcelhennyi/NXP-HoverGames-2/blob/ebc11e9577710f4cf036a230838d943761f902f8/code/src/system/agent/Agent.cpp#L293-L298

Is this actually a problem that can happen? Or is the printf maybe incorrect?

[mcelhennyi]

By the way:

https://github.com/mcelhennyi/NXP-HoverGames-2/blob/ebc11e9577710f4cf036a230838d943761f902f8/code/src/system/agent/Agent.cpp#L118-L123

This can just be:

std::cout << "Drone is in LandedState: " << landedState << std::endl;

Good to know...i never did look to see if << was overridden or not.

https://github.com/mcelhennyi/NXP-HoverGames-2/blob/ebc11e9577710f4cf036a230838d943761f902f8/code/src/system/agent/Agent.cpp#L293-L298

Is this actually a problem that can happen? Or is the printf maybe incorrect?

Technically my wording is incorrect (TARGETING_MODE vs RETURN_TO_LAUNCH) - bad copy-pasta....but the set position does return a status, although I have not seen it result in anything other than success in my testing.

[julianoes]

It should always return "Success" unless there is a connection problem in which case nothing will work anymore.

MAVSDK/src/plugins/offboard/offboard_impl.cpp

Lines 353 to 354 in 14c5154

	return _parent->send_message(message) ? Offboard::Result::Success :
	Offboard::Result::ConnectionError;

So, let me try to rephrase the problem:
We have two states:

1. The PX4 flight mode which can be set to offboard or not.
2. The mode of the MAVSDK offboard class which is some setpoint, or NotActive.

The way start() works is that it requires some setpoint to be set beforehand. If that's the case it should switch the mode to offboard. So then my question is which was the failure case that you got into that meant that you needed to check is_active()?
Or what was the problem that you ran into in the first place?

[mcelhennyi]

I checked is_active() because it seemed like (before I looked into the code base) that this would be a good thing to check before commanding it to start() since I wouldnt want to start then start again (that felt like bad form). Then come to find out is_active() seems more useful internally as a way to see if a setpoint was set or not...but like I said above even internally there was a flaw with it being set back to NotActive too quickly. At some points it would allow me past the isActve with a false return and then I would command a start to only be told that it didnt have a set point set...which i had just set a few lines above.

But yes, I think there are two states/requirements:

1. the state the drone is actually in (offboard/stabilize/etc) - which should drive a users decision to start or stop offboard.
2. The internal mode that allows for the offboard mavsdk system to bark at you for not setting set points recently enough. (isActive()) - which in the current state should really not even be used by the user.

[julianoes]

At some points it would allow me past the isActve with a false return and then I would command a start to only be told that it didnt have a set point set...which i had just set a few lines above.

Hm, so this would mean it resets here:

MAVSDK/src/plugins/offboard/offboard_impl.cpp

Lines 617 to 622 in 2269a0c

	if (!offboard_mode_active && _mode != Mode::NotActive &&
	_time.elapsed_since_s(_last_started) > 1.5) {
	// It seems that we are no longer in offboard mode but still trying to send
	// setpoints. Let's stop for now.
	stop_sending_setpoints();
	}

Basically, the point is that you set a setpoint and then immediately start it. If it's not started within 1.5 seconds, it will timeout again. I see how that is a bit confusing.
We could think about removing this timeout, or making it longer.

Alternatively, we could think of changing the API so that the user never has to call start() at all. Instead as soon as you set the first setpoint it automatically starts sending that setpoint and then tries to switch the mode. Once the mode is in offboard it would confirm.

[mcelhennyi]

offboard.autoStart = true/offboard.enableAutoStart() maybe?

But yea, it would reset there. One of the changes I made, but was unsure of its correctness was changing the != to == for the _mode part of that if(). However I made that change then (i think) immediately made the telemetry mode change as well. So not sure what effect (if any) that != to == had.

I think calling start makes sense, but I also think its valuable for start to return the success or failure of switching from whatever mode its in to Offboard mode. That would make the most sense, since offboard being started means two things: 1) The set point is set and the thread that sends them to the AP is in fact running AND 2) the AP mode has changed to offboard succesfully and the system is using the set points as expected.

[julianoes]

I think calling start makes sense, but I also think its valuable for start to return the success or failure of switching from whatever mode its in to Offboard mode.

Yes, I think we should probably add that. So we try to start offboard mode and then we wait until it's actually active. Once it's active, we return.

And in the same way we could have is_active check both, if setpoints are sent and if the mode is offboard.

[mcelhennyi]

I agree, that makes the most sense to me.

[julianoes]

Ok, I'll put that on my todo list but I can't promise you when I get to it. If you're faster, I'd appreciate a pull request 😉.

[julianoes]

Funny enough I just ran into this in the PX4 integration tests here:
https://github.com/PX4/PX4-Autopilot/blob/82d6cc3dba3bbc8d6fbd0a3449e7d67560d4a67d/test/mavsdk_tests/autopilot_tester.cpp#L425-L426

I received "NoSetpoint" like you said so.

[mcelhennyi]

Ok, I'll put that on my todo list but I can't promise you when I get to it. If you're faster, I'd appreciate a pull request .

I probably will not be able to get to it any time soon either, unfortunately.

[julianoes]

Closing, comment again if this is still an issue.