Added FileNotFound error code according to new Mavlink FTP specification

[MatejFranceskin]

Update to Mavlink FTP protocol added a new error code for FileNotFound error.
mavlink/mavlink-devguide#199

Until now a errno was set to ENOENT and had to be interpreted as such in MavlinkFTP client.

Code change is aligned and tested with MAVSDK MavlinkFTP module and its integration tests.

[dagar]

Mavlink FTP tests failing on Jenkins.
http://ci.px4.io:8080/blue/organizations/jenkins/PX4%2FFirmware/detail/PR-12976/1/pipeline

[LorenzMeier]

@MatejFranceskin You need to update the test as well or fix the implementation. If the test needs to be updated then the backwards compatibility might have been broken, which is a hint that the change needs to be fixed. We can't upgrade all QGC / SDK version in parallel.

[MatejFranceskin]

One test actually pointed out to a real problem which is fixed now. Other tests have been changed.

QGC currently doesn't check the errno return codes and will behave the same regardless of this PR. When this is merged I will make a PR also for QGC so that it will display new error message.

MAVSDK MavlinkFTP module will work with older and newer firmwares.

[bkueng]

CI still fails.

[dagar]

@MatejFranceskin does make tests work locally?

[bkueng]

I still see CI fails:

INFO  [mavlink_tests] RUNNING TEST: _removedirectory_test

ERROR [mavlink_tests] Compare failed: Incorrect payload size - (reply->size:2) (test->reply_size:1) (../../src/modules/mavlink/mavlink_tests/mavlink_ftp_test.cpp:694)

ERROR [mavlink_tests] TEST FAILED: _removedirectory_test

Otherwise looks good.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[hamishwillee]

@julianoes Can you merge this if it is OK?

[julianoes]

Rebased and force pushed, let's see.

[codecov]

Codecov Report

Merging #12976 into master will decrease coverage by 14.85%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           master   #12976       +/-   ##
===========================================
- Coverage   53.01%   38.15%   -14.86%     
===========================================
  Files         625      584       -41     
  Lines       53032    49531     -3501     
===========================================
- Hits        28113    18900     -9213     
- Misses      24919    30631     +5712

Flag	Coverage Δ
#mavsdk	?
#python	3.15% <0%> (-96.85%)	⬇️
#sitl_mission_FW	29.39% <0%> (-70.61%)	⬇️
#sitl_mission_MC_box	29.75% <0%> (-70.25%)	⬇️
#sitl_mission_MC_offboard_att	28.29% <0%> (+0.78%)	⬆️
#sitl_mission_MC_offboard_pos	28.48% <0%> (-71.52%)	⬇️
#sitl_mission_Rover	100% <ø> (ø)	⬆️
#sitl_mission_VTOL_standard	100% <ø> (+67.14%)	⬆️
#sitl_mission_VTOL_tailsitter	33.95% <0%> (+1.13%)	⬆️
#sitl_python_FW	29.47% <0%> (+1.05%)	⬆️
#sitl_python_MC_box	29.83% <0%> (+0.62%)	⬆️
#sitl_python_MC_offboard_att	100% <ø> (ø)	⬆️
#sitl_python_MC_offboard_pos	100% <ø> (+72%)	⬆️
#sitl_python_Rover	100% <ø> (+74.41%)	⬆️
#sitl_python_VTOL_standard	34.01% <0%> (+0.95%)	⬆️
#sitl_python_VTOL_tailsitter	100% <ø> (+66.97%)	⬆️
#unittest	?

Impacted Files	Coverage Δ
...modules/mavlink/mavlink_tests/mavlink_ftp_test.cpp	0% <0%> (-77.52%)	⬇️
src/modules/mavlink/mavlink_ftp.cpp	2.06% <0%> (-43.23%)	⬇️
Tools/px_process_airframes.py	75.55% <0%> (ø)	⬆️
src/systemcmds/tests/test_bezierQuad.cpp	0% <0%> (-100%)	⬇️
src/lib/controllib/BlockLimitSym.cpp	0% <0%> (-100%)	⬇️
src/modules/navigator/mission_block.h	0% <0%> (-100%)	⬇️
src/systemcmds/tests/test_search_min.cpp	0% <0%> (-100%)	⬇️
src/systemcmds/tests/test_hrt.cpp	0% <0%> (-100%)	⬇️
...c/modules/mavlink/mavlink_tests/mavlink_ftp_test.h	0% <0%> (-100%)	⬇️
src/modules/navigator/takeoff.h	0% <0%> (-100%)	⬇️
... and 319 more

[julianoes]

I'm puzzled. The tests pass locally for me as well as in docker. This must be some permission issue that we can't write files in the working directory.

[bkueng]

Can you add more instrumentation? The result shows Compare failed: Didn't get Nak back indicating a test that should fail did not.

[julianoes]

This now hardfaults on px4_fmu-v2_test. I'm checking.

[julianoes]

This now passes my tests, let's see what CI says.

[bkueng]

CI still fails on MacOS.

[LorenzMeier]

Finally fixed - merging.

[julianoes]

@LorenzMeier thanks for merging but you managed to squash (overwrite) 22h hours of my time and 11 commits with valuable diff information 😲. I don't care too much about my name being in the history but more about the fact that I wrote down why and how I changed things for the next confused reader. And squashing as a final step is ok but not even having it in the GitHub PR here is bad.

For anyone looking for some context later, here are my commit notes:

mavlink: include standard instead of common

This should fix a build warning on macOS in mavlink_conversions.h.

---

mavlink: fix on target unit tests

This fixes the unit tests on target, e.g. px4_fmu-v2_test.

The problem was that no MAVLink instances are instantiated and hence
mavlink_status_t for COMM_0 is null. When the mavlink message is packed
it accesses the status->signing which is then garbage and leads to a
segfault.

The fix involves using the MAVLink headers without convenience functions
defined for the unit tests and therefore not trying to use data from the
mavlink module.

---

mavlink_ftp: remove test specific to permissions

This suddenly passed in CI but locally failed as expected. This is just
confusing, so I'm removing this.

---

ROMFS: remove deleted folder

This data is now generated on the fly.

---

mavlink_ftp: re-enable all tests, and on NuttX

Some tests were disabled because they had been failing.
They should now be fixed.

Also, the tests were not included for NuttX targets. I've added that
back and fixed them there as well. Some differences in the tests remain
between Linux and NuttX:

• The errno is different on NuttX for mkdir("/one/two") when "/one" is
  missing.
• readdir on NuttX does not report . and .. like specified by POSIX.

---

mavlink_ftp: remove option for hidden folders

The option to ignore hidden folders was not used, not tested, and only
implemented for folders but not for files which could lead to surprises.

I therefore decided to remove the option alltogether and always list
hidden files and folders (. and .. are marked as skipped/"S").

---

mavlink_ftp: generate test files on the fly

Instead of using test files from the tree, we now generate them before
each test. This has the advantage that we do not need a specific NuttX
target which uploads (or includes) these files.

[LorenzMeier]

Fair ask regarding the commit messages. From the summary lines it did sound like CI debugging, that might have been the wrong call here, sorry! I’m wondering if some of that information would have rather belonged into code or a dedicated CI doc?

I believe that most people only will look at a commit message if they dont understand the change and some of the information you have in there would probably benefit anyone extending these tests in the future.