When DO_REPOSITION asked for a change into GUIDED and the mode change
was refused (for example GUIDED being blocked by FLTMODE_GCSBLOCK) the
result of set_mode() was ignored. The requested location was still
loaded with set_guided_WP() and the command was ACCEPTED, so a vehicle
in AUTO stayed in AUTO but flew towards the reposition target.
Return MAV_RESULT_FAILED instead, leaving the current mode's navigation
alone.
run_coverage.py now exits on the first failing test suite by default;
keep running the remaining suites so we still get a coverage report.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
use as a diagnostic tool was limited when it would exit on some failures but not on others.
Make it exit on failures by default, add a flag to allow a user to get consistent behaviour the other way
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rover workflow's build job primes ccache with
--enable-math-check-indexes, matching sitltest-rover and
sitltest-sailboat, which both pass "Rover" as the run_autotest name.
sitltest-balancebot passes "BalanceBot", so it configured without the
flag, missed the cache on 872 of 1382 compiles and spent 6m33s
building ardurover against 14s in the other two jobs.
Key the flag on the build step rather than the name, so BalanceBot
builds the same binary as the other build.Rover jobs.
this test was relaying on the vehicle disarming even with an action of NONE. Since we're changing that behaviour, change the test to set the failsafe to a value which will disarm the vehicle
Lists a directory holding a file whose entry is exactly the 239 bytes a
listing payload carries, one a byte longer than that, and a short one.
The first has to be listed, the second cannot be and must not be, and the
third proves dropping the one which cannot be sent did not end the
listing.
An entry needing exactly as many bytes as the payload holds was treated as
unsendable and dropped. It does fit: AP_HAL::Util::vsnprintf builds its
BufferPrinter with size-1 and then writes the terminator itself at
str[size-1], and the byte that overwrites is the entry's own trailing NUL,
so the encoding which lands in the buffer is the one intended.
Compare against the size rather than allowing it, in all three places, so
the two packing loops still agree on what can never be sent.
Puts a file and a directory in the root, lists it both with and without
times, and checks both come back - and, with times, that they carry the
modification time they were given.
The path of the directory being listed is put in front of each entry's
name in order to stat it, with a "/" between. The root's path is already
just "/", so that produced "//name".
Most filesystems collapse that - FatFs skips duplicated separators
(ff.c:3019) and so does littlefs (lfs.c:1503) - but SITL's map_filename()
strips exactly one leading "/", so "//name" escapes to the host root. The
stat then fails, and a failed stat drops the entry, so every file in the
root went missing and the listing came back with directories only.
ArduPilot's own backend_by_path() also strips exactly one leading slash,
so a doubled separator would likewise miss a virtual backend.
Lists a directory to leave entries in the reply buffer, then asks past the
end of that listing, and checks the one byte NAK which comes back has
nothing but zeros behind the error code.
The whole reply buffer went out whatever the reply's size, so a short
reply was padded with bytes which are not part of it.
Those bytes come from the same reply, not an earlier one: setup_reply()
clears the whole transaction, but list_dir()'s offset-skip loop then
formats each entry it skips into response.data as scratch, and a listing
which ends in an EndOfFile NAK sets only data[0] - leaving the last
skipped entry sitting behind the error code.
Copy only the bytes the reply says it has. The rest of the packet is
already zero, and since MAVLink 2 trims trailing zeros from a payload, a
short reply now goes out shorter as well.
The scratch reuse behind it is left as it is; not sending the bytes is
what keeps them off the air.
test Renode / cubeorangeplus-quadplane (push) Canceled after 0s
test scripts / build (astyle-cleanliness) (push) Canceled after 0s
test scripts / build (check_autotest_options) (push) Canceled after 0s
test scripts / build (logger_metadata) (push) Canceled after 0s
test scripts / build (param-file-validation) (push) Canceled after 0s
test scripts / build (param_parse) (push) Canceled after 0s
test scripts / build (python-cleanliness) (push) Canceled after 0s
test scripts / build (shellcheck) (push) Canceled after 0s
test scripts / build (validate_board_list) (push) Canceled after 0s
SmartAudio 2.1 reports a variable number of supported power levels. Validate the fixed fields, advertised level count, and CRC instead of requiring the maximum-size response struct.
SIM_Parachute deploys as soon as the PWM on SIM_PARA_PIN reads 1250 or
more, and it starts watching that pin the moment the pin number is set.
The parachute tests set the pin in the same set_parameters() call as the
release servo's function, which races the servo output: assigning
SERVO9_FUNCTION=27 leaves the channel at its previous value until
AP_Parachute drives it to CHUTE_SERVO_OFF (1100 by default, below the
trigger), and anything at or above 1250 in that window fires the chute
during setup.
The test then fails in a way which does not look like a setup problem at
all: "BANG! Parachute deployed" arrives before the test starts waiting
for it, and the real release later in the mission is silent because the
chute has already gone, so the wait times out with "Failed to receive
text: bang". Seen overnight in Parachute and in GCSFailsafe's parachute
subtest, one run in 26 each.
Configure the vehicle first, wait for the release channel to reach its
off position, and only then let the simulation watch the pin. Done in
one helper, as four tests set this up the same way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lints workflow syntax, action inputs, and runner labels/matrix on every
push and pull request touching the workflows or composite actions.
Fix the SC1090, SC2129 and SC2006 shellcheck findings actionlint
surfaces in esp32_build.yml, macos_build.yml and test_sitl_periph.yml
(group the summary-file appends, mark the non-constant `source
~/.bash_profile` as intentional, swap a backtick command substitution
for $(...)), and narrow the shellcheck suppression list to just
SC2086, the pre-existing quoting style across the build scripts that's
out of scope here.
The test waits for a statustext to reach it over the FRSky passthrough
link, and allowed "7 * self.speedup" simulated seconds for it. That
scales the wrong way. Measured, with an unlimited budget, the simulated
time needed to receive the wanted text is
speedup 1 2 5 10 20 100
needs 58.4 53.9 48.7 39.4 39.9 9.9 s
allowed 7 14 35 70 140 700 s
The requirement falls as the speedup rises, because what has to happen
first is that the queue ahead of our text drains, and at a high speedup
far less simulated time passes while that happens in wall clock. The
budget rose instead, so it handed out 71 times what was needed at the
default speedup - no assertion at all - and less than was needed at
anything below about 7. It failed every time at --speedup=5, three
passes out of three in an overnight sweep, and had only 1.8x margin at
--speedup=10.
Allow a fixed 150 simulated seconds: 2.6x the slowest measured, and
still a real bound at the default speedup. Passes at speedups 1, 5, 10
and 100.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AP_Baro_MS56XX::_init() fills in the device ID from devtype() before
AP_Baro_MS5837::_init() gets a chance to assign _subtype, so the
devtype byte of BARO*_DEVID was taken from an uninitialised member.
In practice it read back as 0, which ground stations decode as an
UNKNOWN sensor. Pressure was unaffected because _calculate() only runs
once _subtype has been set.
Pick the variant in devtype() from _cal_reg.c1, which the parent has
already read out of PROM by the time it stamps the ID, and drop
_subtype so there is only one source of truth.
Co-authored-by: Cursor <cursoragent@cursor.com>
test_sitl_copter, test_sitl_rover and test_sitl_sub register four
matchers from .github/problem-matchers/ but ignored all of .github/
bar actions and their own file, so a matcher edit never reached the
workflow it was written for. They gain the negation esp32_build.yml
already carries. test_scripting.yml filters on an allowlist instead,
so Lua.json has to be named there.
'Tools/autotest/location.txt' never existed -- the file has been
locations.txt since f13e6079bc -- and common.py became
vehicle_test_suite.py in 00bbb61411. Both patterns had been matching
nothing, so the files they were meant to ignore were in fact triggering
the workflows.
The file went away with b0a961ce17 ("waf: Delete .pydevproject with
legacy Python 2"); the ignore entry has matched nothing since, in all 17
workflows that carried it.
Unlike the v3 IMUs, where the sensor ODR is programmed to the backend
rate, the v1 FIFO fills at the 8kHz sensor rate when fast sampling and
the backend rate is a software decimation. Reading a non-primary at 2x
loop rate left 20 samples in the FIFO between beats, and with bus
latency on top it reached the depth at which _read_fifo() already
expects corrupt samples: an MPU6000 in the second slot gave a
continuous stream of "stop at 8 of 48" and temperature resets.
Hold a fast-sampling non-primary at 1kHz, one read buffer of samples
per beat, so the FIFO stays well clear of that depth. The 2x loop rate
scaling now only applies without fast sampling, where the FIFO fills at
the backend rate as it does on v3.
Mirror the v3 driver's set_primary(): with the fast rate loop's dynamic
FIFO enabled a non-primary IMU is read at 2x loop rate, bounded to
400..1000Hz, and the primary at the full backend rate. Without the
override the v1 driver ignored primary changes and read every IMU at
the full backend rate.
The bug this guards against is a silent mis-report: a second local run
of process_scan_build_output.py moved its reports beneath the previous
run's tmp/scan-build and then read the previous run's findings. Check
that outside GitHub Actions the reports stay where scan-build put them
and a repeat run reads its own findings, that under GitHub Actions they
are archived beneath the workspace, and that an existing archive or a
workspace that is not an absolute path fails the run rather than
losing the reports.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
autotest sets TMPDIR to the checkout tmp directory, but the workflow
creates and uploads reports through a path containing the canonical
repository name. Forks with another name therefore point scan-build at
a directory that does not exist.
Use GitHub's workspace path so the job is independent of repository
name.
AI-assisted: root cause analysis and patch drafted with OpenAI Codex.
Derive the fixed archive destination from GitHub's workspace instead of
a path containing the canonical repository name, so the reports stay
inside the checkout when a fork is renamed.
Archive only under GitHub Actions. The old path doubled as the CI
check: /__w/ardupilot/ardupilot/tmp never exists on a developer
machine, so archive_rename() was a no-op there by accident. A path
derived from the checkout passes that check on any Linux developer
machine that has run scan-build, because autotest.py points TMPDIR at
the checkout's tmp directory, and a second local run would then move
its reports beneath the first run's tmp/scan-build, where the
non-recursive plist glob does not find them. The check is now the
GITHUB_ACTIONS variable, as elsewhere in Tools/, and an existing
destination is refused rather than nested under.
GITHUB_WORKSPACE must be absolute. An empty value would put the
archive relative to the current directory, where the upload step only
warns that it found nothing; an unset one now fails the same way
instead of raising KeyError.
AI-assisted: root cause analysis and patch drafted with OpenAI Codex;
archive gating reviewed and drafted with Claude.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The wait loop sent MAV_CMD_RUN_PREARM_CHECKS on every iteration, and
the loop fed itself: each run emits several PreArm statustexts, so
the recv_match never blocked and the sends never paused. At parallel
speedups this reached ~1150 commands per wall second - the vehicle's
main loop dropped to 200Hz answering them, 291k MSG records (22MB)
went into the session log, and the logger io thread was driven to
drop 11562 messages - the only non-zero drop count in a 907-log
audit of parallel-run file-backend logs.
Send at most one prearm run per simulated second, as
assert_prearm_failure already does. Validated: two commands per run
(was ~15000), zero dropped messages, test passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reparent airspeed_EAS and get_unconstrained_airspeed_EAS from
AP_AHRS_DCM to AP_AHRS_Backend so backends other than DCM can supply a
synthetic airspeed without DCM compiled in. As with the wind
estimation, the bodies are left in place in AP_AHRS_DCM.cpp - only the
class qualifier and the surrounding guard (AP_AHRS_DCM_ENABLED ->
AP_AHRS_ENABLED) change. _last_airspeed_TAS moves to the base class;
DCM still updates it from drift_correction.
The synthetic estimate is gated on the backend having a ground
velocity from a source independent of airspeed. Callers pass the
backend's published Estimates::have_velocity_source down through
airspeed_TAS/airspeed_EAS; DCM-internal callers pass have_gps()
directly, which is the same fact and the same value the old code
read. The AP::gps() test the base class would otherwise have needed
would be wrong for e.g. ExternalAHRS, which sources velocity from
its own device rather than the autopilot's GPS.
As with the wind-estimation move, the airspeed_sensor_enabled helpers
are now DCM's own (9ed23d57e1), so the moved code tests the airspeed
sensor directly - the same nullptr/use/healthy test, written the way
the frontend's _should_use_airspeed_sensor now does.
Add have_velocity_source to AP_AHRS_Backend::Estimates: true if the
backend has a ground-velocity source that is independent of airspeed
(e.g. GPS), so the value may be used for synthetic airspeed without
circularity. DCM publishes have_gps(). It may be true while
velocity_NED_valid is false (e.g. DCM with a 2D GPS fix).
Reparent the wind-triangle estimator (estimate_wind and
set_external_wind_estimate) from AP_AHRS_DCM to AP_AHRS_Backend so any
backend can run it without DCM compiled in. The method bodies are left
exactly where they are in AP_AHRS_DCM.cpp to preserve their history -
only the class qualifier changes and the surrounding guard switches
from AP_AHRS_DCM_ENABLED to AP_AHRS_ENABLED so they are compiled
whenever AHRS is. Supporting state moves to the base class; DCM keeps
its no-argument estimate_wind wrapper, which passes
_body_dcm_matrix.colx().
One adaptation: the airspeed_sensor_enabled helpers are now DCM's own
(9ed23d57e1), so the straight-flight branch tests the airspeed sensor
directly - the same nullptr/use/healthy test, written the way the
frontend's _should_use_airspeed_sensor now does.
Split estimate_wind into a no-argument wrapper and an implementation
taking the velocity and the fuselage forward direction. The matrix was
only ever used for _body_dcm_matrix.colx() (the trim-corrected body
forward axis in the earth frame), so pass that unit vector in directly
rather than the full attitude matrix. Callers pass
_body_dcm_matrix.colx(); the vector must be a unit vector and is not
normalised here.
original read-twice suffers from beat frequencies where the 50Hz update on the device and the 50Hz update rate of the backend drift relative to one another. The device appears dead when that happens.
guided_above_terrain_posvelaccel_sub.lua integrates its position target
forward by the time since the last callback. When a callback arrived
later than 2/RUN_HZ it substituted 1/RUN_HZ for the real interval:
if (dt > 2.0 / RUN_HZ) then
dt = 1.0 / RUN_HZ
end
so a callback 243ms late advanced the target by 50ms and the remaining
193ms was discarded. The position target then falls behind the clock,
and the vehicle - which tracks that target accurately - covers less
ground than the commanded speed implies.
Measured from a failing Sub.GuidedAboveTerrain run under autotest at
--parallel=16, over the 60 simulated seconds the test watches:
GUIP updates 1047 at 17.2Hz (nominal 20Hz)
late callbacks 76 of 1047 (7.3%), worst 243ms
time discarded 5.82s, so 55.08s integrated of 60.90s elapsed
distance 27.90m of an expected 30.00m, +-1.00m -> failed
The dataflash shows the loss is upstream of the controller, not in it:
the script commanded 0.495m/s and the position controller achieved
0.442m/s against a desired 0.443m/s - it tracked what it was given to
within 0.001m/s, and what it was given was slow.
Cap the step at a fixed MAX_DT instead, so ordinary jitter is
integrated honestly and only a real stall is bounded. Pinning the
simulation speedup was tried first and is not a fix: at
context_set_speedup(10) the run still lost ground, reaching 28.93m,
because it reduces how often callbacks are late without changing what
happens when they are.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Peter Barker
Clyde McQueen
Yves
rubikscube05
gradvizor
Sharanie0612
Michelle Rossouw
Pierre Kancir
dependabot[bot]
Willian GalvaniandCursor
olliw42
Andy Piper
James HarveyandClaude Fable 5.1