AP_Logger: handle LOG_DISARMED=0 in mavlink-logging backend

[julled]

Description

In the current implementation the mavlink backend cant be used without warning messages from the GCS if the GCS doesnt want to / can permanently receive the whole logging stream (e.g. LOG_DISARMED=0 which is the default value). To explain the erroneous behaviour i want to show 3 possible situations which are possible with the two possible operation modes of the mavlink-router which implementats the GCS side of mavlink-logging:

1.

• mavlink-router: logging mode "always"
• APM: DISARMED||ARMED + LOG_BACKEND_TYPE=MAVLINK-BIT-SET
• Mavlink-logging backend detects mavlink-router which saves data to disc. Mavlink-logging backend logging_failed == false

2.

• mavlink-router: logging mode "while-armed"
• APM: DISARMED + LOG_DISARMED=0 + LOG_BACKEND_TYPE=MAVLINK-BIT-SET
• Mavlink-logging backend doesnt detect mavlink-router as no data is saved. Mavlink-logging backend logging_failed == true

3.

• mavlink-router: logging mode "while-armed"
• APM: ARMED + LOG_DISARMED=0 + LOG_BACKEND_TYPE=MAVLINK-BIT-SET
• Mavlink-logging backend detects mavlink-router which saves data to disc. Mavlink-logging backend logging_failed == false

Case 1 is a inconvient for the user as a lot of non interesting data is saved.

Case 2 prevents usage of the arming checks for logging and creates warnings in AP_RCTelemetry.cpp, as the mavlink-logger always fails.

Implementation details

This PR tries to create a exception behaviour for Case 2 to not fail automatically when no logging stream is supposed to be sent via mavlink(LOG_DISARMED=0 + DISARMED).

Tests performed

Tested Usecases 1,2,3 with SITL on my local machine successfully without further warnings by safetychecks and also checked the logging_healthy variable manually in the debugger.

mavlink-router config file used:

[General]
# Valid values: <auto>, <common> or <ardupilotmega>
# Default: <auto>
MavlinkDialect = ardupilotmega

# folder to put logs to
Log = /tmp/logs

# Define when to store flight stack or telemetry logs. From the start of
# mavlink-router until it's stopped or just while the vehicle is armed.
# Valid values: <always>, <while-armed>
# Default: <always>
LogMode = while-armed
#LogMode = always

[UdpEndpoint in_binlogrecorder]
Mode = Server
Address = 127.0.0.1
Port = 14550

[booo]

This addresses #26688

[booo]

https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Logger/AP_Logger.cpp#L629 contains logic that tells us if logging should happen and consists of multiple conditions. It seems to me that we start duplicating code here that is present elsewhere. Which leads to different behavior for different backends in the long run.

[peterbarker]

This should be a single commit, not three.

We have documentation in the Wiki about how to clean the commits up, let me know if you need help past that.

[julled]

This should be a single commit, not three.

We have documentation in the Wiki about how to clean the commits up, let me know if you need help past that.

Thanks for the review, i fixed this as you wished.

[julled]

@peterbarker I am a bit confused that github still asks for a change request but its resolved already.. does this block a possible merge?

[peterbarker]

I've just remembered why we do things this way in master.

It's so you can't arm unless there's a logging client currently connected. i.e. we want the logging subsystem to be working before the vehicle becomes armed.

@julled have you tested what happens if the logging client (the one on the companion computer) is not running and you try to arm?

[julled]

I've just remembered why we do things this way in master.

It's so you can't arm unless there's a logging client currently connected. i.e. we want the logging subsystem to be working before the vehicle becomes armed.

@julled have you tested what happens if the logging client (the one on the companion computer) is not running and you try to arm?

Then the logging preflight check fail, because there is no data sent (as there is noone requesting data). Thats basically the same as case nr 2, as the logging client(mavlink-router) doesnt ask for any data to be sent.

How could LOG_DISARMED=0 be used in this case, as ideally there should be some kind of acknowledgement from the logging client even when no actual (huge!) log data is sent (see mavlink-router code) I dont know how other mavlink-logging client implementations handle this.