fix(navigator): check loiter radius against geofences on mission upload - #28527
Open
masonzeng702550 wants to merge 1 commit into
Open
fix(navigator): check loiter radius against geofences on mission upload#28527masonzeng702550 wants to merge 1 commit into
masonzeng702550 wants to merge 1 commit into
Conversation
| if (!checkLoiterPerimeterAgainstGeofence(missionitem)) { | ||
| mavlink_log_critical(_navigator->get_mavlink_log_pub(), "Geofence violation for loiter radius of waypoint %zu\t", | ||
| i + 1); | ||
| events::send<int16_t>(events::ID("navigator_mis_geofence_violation_loiter"), {events::Log::Error, events::LogInternal::Info}, |
Contributor
There was a problem hiding this comment.
[error] bugprone-chained-comparison [error]
chained comparison v0 < v1 > v2 may generate unintended results, use parentheses to specify order of evaluation or a logical operator to separate comparison expressions
Contributor
🔎 FLASH Analysispx4_fmu-v5x [Total VM Diff: 392 byte (0.02 %)]px4_fmu-v6x [Total VM Diff: 376 byte (0.02 %)]Updated: 2026-09-03T15:06:08 |
masonzeng702550
force-pushed
the
fix-loiter-radius-geofence
branch
from
September 3, 2026 14:58
dbb9702 to
c21f154
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MissionFeasibilityChecker::checkMissionAgainstGeofence()validates one point per mission item — the item's own coordinate. For a loiter item that is the centre of the circle, so the radius takes no part in the decision. A loiter whose centre sits outside an exclusion zone but whose perimeter crosses into it is accepted at upload.Affected:
src/modules/navigator/mission_feasibility_checker.cppon v1.17.0 and on currentmain, which checks the same single point.Demonstrated impact
On PX4 SITL with an exclusion polygon and a loiter centre placed outside it at
R = 200 m:A control mission with a waypoint placed directly inside the zone is correctly rejected on the same build, so the fence is active and the acceptance above is a passing check rather than an absent one.
A reproducer exists. Following
SECURITY.mdI am not posting it; happy to share it privately.Fix
Sample the loiter perimeter and check each sample with the existing
checkPointAgainstAllGeofences(), in addition to the centre point. Sampling uses a 10 m maximum arc step, clamped to between 8 and 64 points, so the cost stays bounded for large radii.Covered commands are
NAV_CMD_LOITER_UNLIMITED,NAV_CMD_LOITER_TIME_LIMITandNAV_CMD_LOITER_TO_ALT. The radius is read fromitem.loiter_radius, whichmavlink_mission.cppalready populates from the correct parameter for each command.Test
Stock build and patched build, same fence and same three missions:
R = 200 m, perimeter crosses inR = 10 m, perimeter clearOnly the middle case changes. New message:
Geofence violation for loiter radius of waypoint 2.Known limitation
Sampling is discrete, so a geofence smaller than the arc step between two samples can still pass. Making the step adaptive to the smallest fence feature, or testing perimeter segments rather than points, would close that; I kept the step fixed to keep upload-time cost predictable and am happy to change the approach.
Found with AI assistance (Claude); the fix was written and tested by me in SITL.