After an upload times out, a delayed copy of the last received item can receive MAV_MISSION_ACCEPTED even though the remaining items were never uploaded.
Assisted-by: Codex
Signed-off-by: onelittlechildawa <onelittlechild@outlook.com>
The legacy numeric mapping reports Position Slow and Guided Course as RTL, and Altitude Cruise as Landing.
Assisted-by: Codex
Signed-off-by: onelittlechildawa <onelittlechild@outlook.com>
The persistent rate setpoint is also populated by its subscription.
Generating a manual fixed-wing rate setpoint only replaces the forward
thrust, leaving the other components from previously received setpoints.
Clear those components before publishing the Acro setpoint.
Assisted-by: Codex
Signed-off-by: onelittlechildawa <onelittlechild@outlook.com>
Acknowledged entries are retained until timeout so lagging channels can
register the same command. Exclude their acknowledged channel state
from ACK matching so it cannot consume ACKs for later pending entries.
Assisted-by: Codex
Signed-off-by: onelittlechildawa <onelittlechild@outlook.com>
The old boolean is serialized as BSON_INT32, but the migration reads
the double member and can disable an enabled setting. Rename the node
and use the normal parameter import path.
Validation: four focused host checks cover enabled/disabled values,
a mismatched BSON type, and an unrelated parameter.
Assisted-by: Codex
Signed-off-by: onelittlechildawa <onelittlechild@outlook.com>
GPS_UBX_CFG_INTF declares six bits (0-5), so its range is 0-63, but it
declares max: 32. 32 is the value of the highest bit on its own
(I2C_OUT_PROT_RTCM3X = 1 << 5), not the all-bits-set value, so every
value from 33 to 63 is published to ground stations as out of range.
The smallest of them is UBX input plus RTCM3X output on I2C, 1 + 32.
The parameter was introduced in df441ac202 with all six bits and
max: 32 in the same hunk, so the mask never outgrew its maximum.
GPS_1_GNSS and GPS_2_GNSS in the same file are also six-bit masks and
both declare max: 63. They were corrected from 31 to 63 in c90ccabbe0
when NAVIC was added as their sixth bit, which is the same arithmetic.
Tools/module_config/generate_params.py derives (1 << (max_bit + 1)) - 1
for a bitmask with no explicit max, which is 63 here.
The metadata check in srcparser.py validates each bit on its own,
int(min) <= 2**index <= int(max), so 2**5 = 32 <= 32 passes by exactly
one and the inconsistency was never flagged.
This is ground-station metadata only: min and max are not compiled into
the firmware, so runtime behaviour is unchanged.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Qutibah Ananzeh <38795261+Ti-03@users.noreply.github.com>
The job ran brew update and then installed whatever Homebrew was publishing, except Gazebo, which stayed on one commit. Those were built against different protobufs, so a release upstream failed the build with no commit here. Check the package repos out at commits recorded in the tree and do not update them. Run on macos-15 only, so the OS label cannot move either.
Assisted-by: Grok:grok-4.7
Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
The build runs brew update and then pours Gazebo from one pinned tap commit while the rest of the install, including protobuf, comes from current Homebrew. A release in either repo fails the check with no commit here, and the GitHub-hosted image moves on its own, so the job cannot be a closed set. Drop the build and the weekly pin refresh. The setup script, the pins, and the refresh script stay for local use.
Assisted-by: Grok:grok-4.7
Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
make px4_sitl fails when Homebrew's protobuf is not the one the pinned Gazebo bottles were generated against. Those generated headers are a fatal error against any other version, and that has been failing these legs since 2026-09-02. Take the check off push and pull requests. workflow_dispatch still runs it.
Assisted-by: Grok:grok-4.7
Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
* fix(ekf2): tag EKF2_MAG_TYPE STATUSTEXT for event dedup
Missing trailing tab meant a modern GCS wouldn't suppress this legacy
STATUSTEXT in favor of the paired event, showing the message twice.
Signed-off-by: Balduin <balduin@auterion.com>
* fix(navigator): tag in-air landing STATUSTEXT for event dedup
Missing trailing tab meant a modern GCS wouldn't suppress this legacy
STATUSTEXT in favor of the paired feasibility_mis_in_air_landing_req
event, showing the message twice.
Signed-off-by: Balduin <balduin@auterion.com>
* fix(mavlink): stop dropping the CAN/camera ID collision warning
This STATUSTEXT was tagged for event-dedup suppression but has no
paired event, so a modern GCS silently discarded it instead of
showing it.
Signed-off-by: Balduin <balduin@auterion.com>
---------
Signed-off-by: Balduin <balduin@auterion.com>
A completed mission upload only updated the active mission when the
uploaded mission differed from the stored one:
// Only need to update if the mission actually changed
if (_transfer_current_crc32 != _crc32[MAV_MISSION_TYPE_MISSION]) {
update_active_mission(...);
}
update_active_mission() is what resets the current index and tells the
navigator about it, so re-uploading an identical mission left the index
wherever it was. After a reboot that index is restored from the mission
state in dataman, which is the last item of the previous flight, and the
mission then reports itself finished a few milliseconds after being
started:
INFO [navigator] Executing Mission
INFO [navigator] Mission finished, landed
WARN [navigator] No valid mission available, refusing takeoff
An upload means the mission is to be flown from the start, so update the
active mission on every completed upload.
* feat(boards): add LSM6DSV support for Kakute H7 Wing
* fix(boards): lower Kakute H7 Wing OSD work queue priority
Run SPI2 below the SPI3 IMU queue to reduce delays caused by ATXXXX updates.
Every gz .pb.h refuses to compile against anything but the exact protobuf
its gencode came from, so the bottles gz-tap-pin.txt holds osrf/simulation
at only build against the protobuf homebrew-core shipped the day OSRF built
them. homebrew-core moves protobuf on its own schedule and OSRF rebuilds
days later, so `make px4_sitl` has been failing on macOS on every branch
since homebrew-core shipped protobuf 36.2.
Pin protobuf as well, to the homebrew-core revision that installs the
version the pinned gz bottles carry, and derive that pin from the bottles
themselves so the two can never disagree. A protobuf release then becomes a
no-op here instead of days of red CI while OSRF catches up.
The pinned install has to run after the other simulation packages and with
the dependents check off: opencv@4 depends on protobuf too, brew resolves a
dependency against the versions recorded in the dependent's bottle, and it
reinstalls dependents whose linkage it has just broken. Pinning the formula
with `brew pin` is not an option either, brew then refuses to install
anything that depends on it.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
The NuttX 12.12.0 defconfig refresh dropped CONFIG_MTD_W25N and
CONFIG_W25N_SPIFREQUENCY from this board while the other W25N boards
(kakuteh7mini, kakuteh7v2) kept them. The option still exists in NuttX
12.12 and defaults to off, and the board's init.c only registers the
flash under #ifdef CONFIG_MTD_W25N, so the onboard 128 MB NAND was no
longer initialised: no /fs/flash, no logging, no flash-backed dataman.
Restore both options with the 104 MHz SPI frequency the board used
before.
Signed-off-by: Julian Oes <julian@oes.ch>
The BSON document length is the first four bytes of the document and is signed.
bson_decoder_init_buf() checked it against the buffer size, but the check only
ran when the value was positive, so a negative length skipped it and decoding
proceeded with no bound. bson_decoder_init_file() did not check the length at
all.
Reject a negative length in both, and add a regression test, since the existing
BSON test only covers a round trip of data it encoded itself.
Reported by @lihnucs.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
GIMBAL_DEVICE_INFORMATION carries three 32-byte name fields. The handler copies
each into the uORB message and then writes the terminator into
gimbal_device_info_msg, the decoded MAVLink struct it is about to discard,
rather than into the topic it publishes.
The copies are the same size in both directions, so nothing overflows. What
publishes is a field that can hold 32 bytes with no terminator, which any
consumer reading it as a string would run off.
No consumer does today, so this changes no behaviour. It is wrong as written and
the next reader of those fields would inherit it.
Reported by @lihnucs.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
whitelisting only PX4 source directory made git commands
fail when executed on the submodules
Signed-off-by: Beniamino Pozzan <beniamino.pozzan@gmail.com>
The PR title was interpolated directly into the shell command, so bash
parsed it as syntax before argparse ever saw it. A title containing double
quotes split into several arguments and failed the check for the wrong
reason, and a hostile title could run arbitrary commands on the runner.
Pass it via the environment like the labeler workflow already does, so it
is data rather than syntax.
Also make sure a report exists when the check fails without writing one,
otherwise the posting step dies on a missing comment.md and hides the
actual error.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
* docs(mavlink): present link encryption alongside message signing
The hardening guide told integrators that production deployments must
enable message signing, presenting it as the only way to secure a MAVLink
link. Encrypting the link below MAVLink, with an encrypted radio, a VPN or
IPsec, is at least as strong: it uses standard, reviewed cryptography, it
gives confidentiality as well as authentication, and it covers every
interface on the link rather than only MAVLink.
Present both options, and reword the passages that assumed signing was the
only one.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
* docs(security): develop security fixes in public pull requests
Record that fixes are developed as ordinary public pull requests, with no
private forks and no embargoed branches.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
* docs(security): add security scope document
Describe the boundary the code implements today: what PX4 protects, what it
assumes about the layers beneath it, what the shipped default does and does
not do, and two lists for what is always in scope and what is out. It
describes the boundary rather than pre-deciding reports. A finding that fits
neither list stays a judgement call that maintainers make on the report.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
* docs(security): list a security maintainer for report triage
Security report triage had no named owner, so an unacknowledged report had
nowhere to go except the release managers. Add a Security block to the
maintainers list, and point the follow-up path in SECURITY.md at it.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
* docs(mavlink): note that the first signing key is unauthenticated
The guide already says to provision over a trusted link, and the info box says
that changing or disabling a key requires a signed message. It does not say that
setting the first one cannot, since there is no key to sign with yet, so a reader
can come away believing provisioning itself is protected.
Say plainly that the window exists, what an attacker gets from it, and how to
recover.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
* Apply batched suggestions from code review
Co-authored-by: Sheren N <sherenyn@ad.uni-paderborn.de>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
Co-authored-by: Julian Oes <julian@oes.ch>
* feat(security): more review fixup
* fix(security): only 1.18 gets security bugfixes
* fix(security): link to SECURITY_SCOPE
* fix(securiy): small wording fixups
* docs(security): scope by attacker position, not bug class
The "Always in scope" list made any memory corruption, race or hang a
vulnerability regardless of who can reach it. That is not how reports
have been handled: sanitizer runs in SITL have found and fixed many
such bugs as ordinary PRs, and a peer on an unsecured link already has
a shell.
Replace it with one test: a finding is a vulnerability when it gives
capability to an attacker who has neither the operator's access nor
physical access. Keep the in-tree board configuration rule, point to
the sanitizer docs, and fix two typos.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
* fix(docs): formatting
* fix(maintainers): add Ramon to security as well
* docs(security): draw the boundary as a mermaid diagram
GitHub renders mermaid natively, so the boundary diagram no longer has to
be maintained as hand-aligned ASCII. Label the two zones with the wording
of the sentences below it, so the picture and the prose say the same
thing.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
* fix(docs): review fixups
* docs(security): say how to isolate the offboard transports
A direct cable between flight controller and companion is not enough on
its own: the agent or router republishes into the DDS or Zenoh network
on the companion, so that network is inside the boundary too.
Keep the scope document to what is inside the boundary and put the
isolation advice next to the uXRCE-DDS and Zenoh setup instructions.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
---------
Signed-off-by: Julian Oes <julian@oes.ch>
Co-authored-by: Sheren N <sherenyn@ad.uni-paderborn.de>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
The ListDirectoryWithTime (opcode 16) entry format is
<type><name>\t<size>\t<mtime>\0, but PX4 only applied it to files and
sent directories as a bare D<name>. MAVSDK's server already sends
D<name>\t0\t<mtime>, so the two servers disagreed and a client had to
special-case which one it was talking to.
Directories are now stat()ed for their modification time and reported
with size 0, matching MAVSDK. The plain ListDirectory (opcode 3)
response is unchanged: existing clients take everything after the D as
the directory name there.
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Julian Oes <julian@oes.ch>
Include PX4/NuttX#418 so the IMXRT and STM32 Ethernet drivers report
IFF_RUNNING through netdev_carrier_on/off(). The Zenoh startup check
relies on these interface flags.
Assisted-by: Codex:gpt-6-astra
Signed-off-by: YG_Kim <yeogyeom1@naver.com>
Zenoh can enter a blocking session open before its NuttX network
interface has finished starting. Check interface readiness once at
startup, with a bounded wait that honors stop requests, while keeping
the existing session retry loop.
Initialize the session handle for safe early cleanup and retain atomic
connection status. Have getNetworkConfig() initialize and terminate
its output strings so both callers receive valid C strings.
Assisted-by: Claude:claude-fable-5-1
Assisted-by: Codex:gpt-6-astra
Signed-off-by: YG_Kim <yeogyeom1@naver.com>
Address review feedback on the split between entity creation and data
requests:
- Reuse data_reader_id() in dds_topics.h.em instead of duplicating the
ID formula in the on_topic_update() switch cases.
- Drop the now-unused best_effort_in_stream_id parameter from
SendTopicsSubs::init() and RcvTopicsPubs::init().
- Handle UXR_INVALID_REQUEST_ID in RcvTopicsPubs::request_data(): the
reliable output stream has only STREAM_HISTORY slots, and a slot is
freed only by an Agent ACKNACK, not by uxr_flash_output_streams().
On overflow, wait for delivery to be confirmed (which drains
acknowledged slots) and retry once before giving up.
Verified on hardware by shrinking the reliable stream's slot size to
force the overflow path: recovery completes in ~2.5ms per occurrence
and the session still connects. With the stock buffer size and the
current topic count (38 readers, ~760B of requests against 2016B of
capacity), the overflow path is not exercised in practice.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: danielbuleandra <daniel.buleandra@auterion.com>
* docs(releases): add v1.18 notes for five backported fixes
The estimator selector, VTOL mission return, analog battery filter,
home altitude filter and mission end fixes are on release/1.18 now,
this adds their entries to the release notes.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Saibernard Yogendran <bernie97@seas.upenn.edu>
* MOve entries
* docs(releases): add the v1.18 note for the position controller filter fix
#28534 merged onto release/1.18 after the first five entries.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Saibernard Yogendran <bernie97@seas.upenn.edu>
---------
Signed-off-by: Saibernard Yogendran <bernie97@seas.upenn.edu>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
* fix(navigator): report a failed geofence load instead of leaving a fragment
_updateFence() resets _num_polygons and rebuilds the fence from dataman one
polygon at a time. It can stop early two ways: loadWait() failing for an entry,
or the array resize failing to allocate. Both logged to the console and left
whatever had been loaded so far in place.
A fragment of a fence is worse than no fence. Missing inclusion polygons permit
positions the fence excluded, missing exclusion polygons open up areas it
protected, and unlike an empty fence it still looks to the operator like a fence
is loaded.
Drop what was loaded on either failure and report it with mavlink_log_critical
and an event, so the state is unambiguous and the operator is told the fence is
not active rather than finding out in flight.
Reported by @lihnucs, who found the allocation path.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
* rework(navigator): use _clearFence in ~Geofence()
* refactor(navigator): move new functions in hpp
* fix(navigator): pub GF_STATUS_FAILED when _updateFence fails and ensure GF is reloaded if previously failed with new request with same opaque id
* refactor(navigator): remove redundant check
* feat(msg): add GF_STATUS_FAILED to GeofenceStatus.msg
* refactor(navigator): rename _fence_updated to _fence_loaded
The flag now means the requested fence was loaded successfully, and
"updated" reads like a uORB or dataman change. Default it to false
since nothing is loaded at boot.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Julian Oes <julian@oes.ch>
---------
Signed-off-by: Julian Oes <julian@oes.ch>
Co-authored-by: jonas <jonas.perolini@rigi.tech>
* fix(msg): correct comments regarding default values
The mentioned defaulting happens in
FwLateralLongitudinalControl::updateLongitudinalControlConfiguration,
where they clearly default to the climb / sink rate _setpoint_, not the
respective maximum.
No new message version needed as only comment is changed.
* fix(msg): correct stale publisher reference in FixedWingLateralGuidanceStatus
fw_pos_control was split into fw_mode_manager and
fw_lateral_longitudinal_control; the topic is published by
the mode manager.
* feat(failure-injection): implement motor failure injection with soft and hard modes
* feat(failure-injection): update motor failure injection descriptions and behaviors for annunciated and un-annunciated modes
* feat(failure-injection): update motor failure injection terminology to detected and undetected modes
* feat(failure-injection): refine motor failure injection handling for detected and undetected modes
* fix(boards): remove FailureInjector::update() from itcm_functions
FailureInjector was replaced by the shared failure_injection library, so
the symbol no longer exists and the ITCM check fails on the boards that
still list it. Its successor failure_injection::Config::update() is an
inline no-op unless MODULES_FAILURE_INJECTION_MANAGER is enabled, which
none of these boards do, so there is no replacement symbol to map.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Claudio-Chies <61051109+Claudio-Chies@users.noreply.github.com>
---------
Signed-off-by: Claudio-Chies <61051109+Claudio-Chies@users.noreply.github.com>
Co-authored-by: Claudio Chies <chiesc@chies.com>
The entry loop checks it has room before appending, but the readdir() error path
does not: it writes the skip identifier and a terminator unconditionally. If the
buffer already holds exactly kMaxDataLength bytes of valid entries when readdir()
fails with errno set, those two bytes land past payload->data[].
payload->size then exceeds kMaxDataLength, and _reply() computes
kMaxDataLength - payload->size as the length for a memset. That subtraction is
performed on promoted ints and converted to size_t, so -2 becomes SIZE_MAX - 1
and the memset walks off across the address space.
Apply the same room check the entry loop uses, and clamp payload->size in
_reply() so no future producer can wrap that subtraction either.
Reported by @lihnucs.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
The parser indexes every one of its buffers with a counter that resets only on
a delimiter or end of line, so a sensor that sends neither writes past the end
of each of them.
_readbuf is 30 bytes indexed by _byte_counter, which resets on '\n'. The four
field buffers, _raw_speed[5], _raw_angle[5], _raw_status[2] and
_raw_checksum[3], are indexed by _msg_byte_counter, which resets on ',', '=',
'*', '$' or '\r'. A response such as "$A,WVP=" followed by a long run of digits
walks off the speed buffer and into whatever follows it.
Bound each write. The field buffers keep their last byte clear, since the
atoi() calls rely on the terminator the reset memset leaves behind.
Those memsets also cleared _readbuf using sizeof(_linebuf), a different and
smaller array, so ten bytes of the previous message survived into the next and
fed the checksum. Use the right size.
Reported by @kmm2003.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
write() checked for space using _packet_length, the declared size of the
message being assembled, while the memcpy copies buflen. _packet_length
decrements across calls as a message is written in chunks, so it can be small
while buflen is large, and the check then passes for a copy that does not fit.
A 300-byte write followed by a 10-byte MAVLink v2 header leaves the write index
at 310 and _packet_length at 6, so a following 50-byte write passes the check
and copies twenty bytes past the end of the 340-byte buffer.
Check the space against both the remaining packet and the bytes this call
copies.
_packet_length -= buflen also wrapped, since _packet_length is a uint16_t and
buflen can exceed it whenever the declared length and the bytes actually
written disagree. The value then never reached zero and the packet-assembly
state stayed set. Saturate at zero instead.
Reported by @Finder16 and @Lqs66.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
LOG_REQUEST_LIST, LOG_REQUEST_DATA, LOG_REQUEST_END and LOG_ERASE all carry
target_system and target_component, and MavlinkLogHandler ignored both. The
message is not even decoded in handle_log_erase(): every LOG_ERASE seen on the
link deletes this vehicle's logs, whoever it was addressed to.
On a link shared with more than one vehicle, which is the normal case for a
ground station managing several or for a swarm, erasing one vehicle's logs
erases them everywhere, and a listing or download request for one vehicle is
answered by all of them, corrupting the transfers.
Check the target in each handler, next to the decode it already performs. Only
handle_log_request_end() and handle_log_erase() gain a decode, because they had
none.
Reported by @20210607 and @Nathaniel-hl.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
sgDecodeSVR() walks a running offset through optional fields, each present only
when its flag bit is set, and never checks that the frame actually carried them.
The receive buffer is a fixed 259-byte Packet, so a short frame does not read
out of bounds, but it does decode leftover bytes from the previous message.
The result is a traffic target published with a latitude, longitude, altitude
and velocity assembled from stale data, which the navigator then treats as real
traffic.
Sum the payload the advertised field set requires and reject the frame if the
advertised payload length is shorter.
Assisted-by: Claude:claude-opus-5[1m]
Signed-off-by: Julian Oes <julian@oes.ch>
