option: Implement an option to not write state

[nbertram]

In some circumstances forcing the writing of telemetry logs is unhelpful. In my case I'm running a companion computer on the aircraft with a read only filesystem to make it a bit safer to power down at the end of a flight. Most of its mavproxy job is relaying data to a 4G modem and importing ntrip data. It doesn't need to save telemetry logs as the actual GCS on the ground can do that, plus the FC has its own binlogs.

It looks like there was already quasi-support to not write logs (parts of the code that write logs check for the queue before using it) so I've just piggybacked this patch on to that functionality. I've tested that this operates correctly on a read-only filesystem and no longer bails with "ERROR: opening log file for writing".

Thanks for the consideration...

[stephendade]

This leaves mpstate.status.logdir = None, which can break other modules. In particular, combined with the --continue option.

This also stops other modules from automatically saving information (such as params, waypoints, fence).

Recommend removing this statement.

[tridge]

we do check for logdir==None in a few places, eg. fence and param, likely just need a few more checks

[stephendade]

Looks fairly good. The only issue is that it skips more than just the telemetry logs. It also stops automated saving of other information, such as params and waypoints.

[peterbarker]

On Tue, 25 Oct 2022, Stephen Dade wrote: Looks fairly good. The only issue is that it skips more than just the telemetry logs. It also stops automated saving of other information, such as params and waypoints.

The stated goal is to run on a read-only file-system, so that's probably fine for the author :-) PR title could be amended, perhaps.

[stephendade]

PR title could be amended, perhaps.

And the --no-telemetry-logs argument too, so it doesn't lead to user confusion.

[nbertram]

Hmm I see I've opened a can of worms.

Letting mavproxy resolve the logs dir means it does both a free-disk check and a recursive mkdir on that directory, which obviously fails and bails on RO filesystem, though I'm not necessarily saying we need to gold plate the RO FS case. It's perfectly possible to mount a tmpfs under the state dir to get around the inability to make those directories, though the limited RAM in embedded systems will probably fail the 200MB free space check on a tmpfs.

I imagine the not-logging case is more generally useful than writing nothing though. Nothing rotates or cleans up flight logs automatically, so it doesn't take much to accidentally chew up a lot of disk on a server/device dedicated to mavproxy.

How about I put back the state directory resolution, but remove the 200MB free space check if logging is disabled? Would that be more palatable? Or add an additional option to override that check?

[stephendade]

How about I put back the state directory resolution, but remove the 200MB free space check if logging is disabled? Would that be more palatable? Or add an additional option to override that check?

Happy with removing to 200MB check if logging's disabled.

[tridge]

I think supporting RO filesystem is worthwhile, maybe a --read-only-fs option is clearer though, and we'd use it everywhere we save files, including telem logs? If initial PR missed some spots that would be OK, but testing on a ROFS (or one without permissions to write) should find issues pretty quickly

[nbertram]

Option renamed to more clearly reflect it's not going to save any state.

As tridge mentions, easy enough to test:

$ mkdir state-ro
$ chown 444 state-ro
$ python3 ./MAVProxy/mavproxy.py --state-basedir=./state-ro
ERROR: opening log file for writing: [Errno 13] Permission denied: './state-ro/mav.tlog'
$ python3 ./MAVProxy/mavproxy.py --state-basedir=./state-ro --no-state
Connect udpout:10.9.9.4:14550 source_system=255
Note: Not saving telemetry logs
Waiting for heartbeat from 10.9.9.4:14550
MAV> Detected vehicle 1:1 on link 0
online system 1

Given the orthogonal problem in #1120 it's probably best not to name the option explicitly after --read-only-fs as it does have other use-cases. I actually first reached for this option on a MAVProxy bounce host that was just an intermediary relay between a companion and the GCS, and given telemetry logs were being kept at other hops it wasn't necessary to fill the disk on that server either!

I suppose a puritan might say MAVProxy should default to not saving state unless it's asked to. I suspect the reason it does is because it's become more like a GCS, and people sometimes need that tlog in emergencies so it's helpful for it to be there...

[stephendade]

I've checked and tested this PR. No issues from me.