fix(CorridorScanComplexItem): emit specifiesCoordinateChanged when polyline validity changes - #14749
Conversation
…lyline validity changes
There was a problem hiding this comment.
Pull request overview
This PR fixes a mission-line rendering issue by ensuring CorridorScanComplexItem::specifiesCoordinate() state transitions (driven by corridor polyline vertex count) correctly emit specifiesCoordinateChanged, allowing MissionController to recalculate connector flight-path segments when the corridor becomes coordinate-specified (or stops being so).
Changes:
- Emit
specifiesCoordinateChangedprecisely onQGCMapPolyline::countChangedtransitions by caching the previousspecifiesCoordinate()value. - Initialize the cached state in the constructor to avoid dependence on constructor ordering.
- Add a unit test verifying correct emission behavior across 0→1→2→3 vertices, corridor width changes, and
clear().
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/MissionManager/CorridorScanComplexItem.h |
Adds _updateSpecifiesCoordinate slot and a cached _specifiesCoordinate state used to gate signal emission. |
src/MissionManager/CorridorScanComplexItem.cc |
Connects polyline countChanged to _updateSpecifiesCoordinate, initializes cache, and emits specifiesCoordinateChanged only on state flips. |
test/MissionManager/CorridorScanComplexItemTest.h |
Registers a new unit test slot for specifiesCoordinateChanged emission behavior. |
test/MissionManager/CorridorScanComplexItemTest.cc |
Implements regression test validating emission correctness and avoiding spurious emissions. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14749 +/- ##
==========================================
+ Coverage 25.47% 32.63% +7.16%
==========================================
Files 769 784 +15
Lines 65912 67562 +1650
Branches 30495 31291 +796
==========================================
+ Hits 16788 22052 +5264
+ Misses 37285 30652 -6633
- Partials 11839 14858 +3019
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 476 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 0 passed, 0 failed, 0 skipped. Test Resultslinux-coverage-integration: 37 passed, 0 skipped Code Coverage
Artifact Sizes
Updated: 2026-07-30 16:42:11 UTC • Commit: a2986cc • Triggered by: Android |
Replaces #14725.
Description
CorridorScanComplexItem::specifiesCoordinate()is dynamic (_corridorPolyline.count() > 1) but the class never emitted thespecifiesCoordinateChangedNOTIFY signal.MissionControlleronly rebuilds waypoint connector lines on that signal (plus a one-shotisIncompleteChangedwhich is consumed during item construction), so after tracing a corridor the connector line from the previous waypoint to the corridor entry never appeared until some unrelated action forced a full recalc.Why this approach instead of #14725
#14725 fixed the same bug but had a few weaknesses this PR addresses:
specifiesCoordinate()depends on the polyline vertex count, so the new_updateSpecifiesCoordinate()slot is connected toQGCMapPolyline::countChangedrather than piggybacking the emit inside_rebuildCorridorPolygon(). The polygon-rebuild slot is also triggered by corridor width changes (which can never affectspecifiesCoordinate()), and a future refactor of the rebuild logic can't silently drop the signal emission.countChangedcovers every mutation path —appendVertex,clear(), andsetPath/JSON load all route through it (verified throughQmlObjectListModel/ObjectItemModelBase, including the depth-counted model-reset path), so both the false→true and true→false transitions are always signaled._specifiesCoordinate = specifiesCoordinate()line at a point where the polyline is guaranteed empty (the KML/SHP load runs after the signal connections). Instead the cache is anchored once after the connects via_updateSpecifiesCoordinate(), which keeps it correct regardless of future constructor reordering._testSpecifiesCoordinateChangedunit test covers: no emission below 2 vertices, exactly one emission on the 1→2 transition, no spurious emission on 2→3 or corridor width changes, and the true→false transition onclear(). The test fails on master and passes with this fix.Type of Change
Testing
Platforms Tested
By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).