MAVSDK: Can't arm immediately after instantiating Action plugin

@shakthi-prashanth-m

So, you mean is_armable() should also perform health check.

More precisely, I mean that is_armable() should always and completely indicate that the vehicle is ready to arm. I also mean that it should be possible to work out exactly why that it is not arming.

That should mean that all the checks that are done by the heath check are also done by is_armable() - but there would be more checks if there are other conditions that can give a true health check when arming is not ready.

I think this is what @julianoes meant when he said:

I think it should be duplicated, that’s a tradeoff when preventing dependencies.

What he meant is that you should reimplement the health checking functionality in Action (as opposed to having it in the core (say) or calling the Telemetry version.

Yes, this could (and probably should) be a private method. I think the method would be run inside is_armable() (not as part of init - after all, what if health isn’t OK at init but you run arm a few minutes later?). There might be more than the health check run in is_armable() - whatever it takes to make sure the vehicle really is ready to arm.

Does that make sense?

the is_armable() must be 100% reliable. If this passes, the only reason arming should fail would be a subsequent change in state - ie a failsafe engaged between is_armable being true, and arm being received.

So, you mean is_armable() should also perform health check. I think this is what @julianoes meant when he said:

I think it should be duplicated, that’s a tradeoff when preventing dependencies.

In that case, we could have a private method:

void Action::health_all_ok_async(health_all_ok_callback_t callback);

that performs health check during Action::init(); and its result will reflect in is_armable() value. In other words, is_armable() will be true only when health_all_ok is true. I think this should solve the issue.

@shakthi-prashanth-m @JonasVautherin

I think it should be fine. If user want to understand why was command denied, he may install Telemetry plugin and perform health check, etc. Sounds good ?

So yes to the broad point that is_armable() in the Action plugin is just binary information, and that getting detailed information on why something is not arming requires loading Telemetry module.

• the is_armable() must be 100% reliable. If this passes, the only reason arming should fail would be a subsequent change in state - ie a failsafe engaged between is_armable being true, and arm being received.
• It must be possible to find out all blocking reasons, including land state or non-initialisation - is this the case?

So this code looks good …

while (!action->is_armable()) {
   std::cout << "Waiting for to arm..." << std::endl;
   //Here you could check Telemetry to find out why things are failing
   sleep_for(seconds(1));
}
// Arm vehicle
std::cout << "Arming..." << std::endl;
auto arm_result = action->arm();
handle_action_err_exit(arm_result, "Arming failed: "); //Could also check to find out why
std::cout << "Armed." << std::endl;

But generally you should not get the following log (a command denied) because is_armable has returned true.

Discovered device with UUID: 4294967298
Waiting for to arm...
Arming...
[03:50:24|Warn ] command temporarily rejected (176). (mavlink_commands.cpp:143)
Arming failed: Command denied

So I wouldn’t have:

The vehicle might reject commands while it is initializing, and if you want to know when it is ready, you get this information from the telemetry."

as it implies that you have to check on telemetry to know when things are ready, but in fact you only check on telemetry to find out *why it is not ready. Perhaps:

The vehicle might reject commands while it is initializing. You should check the is_armable() state to determine when it is ready to arm. If you wish to understand what condition the vehicle is waiting on then you can get this information from the telemetry."

I vote for is_armable()

OK, but can/should we correspondingly remove health_all_ok() as they effectively serve the same purpose.

Possible solutions we talked about on the call:

1. Add Action::is_armable().
2. Move landed/in-air state to core and get it in action plugin.
3. Cache last message received of each type and let action get the it to determine the state.
4. Do nothing, and just be aware that a plugin needs to be instantiated as early as possible.