Skip to content

Terrain: bounded retry, NoTile classification, and no-terrain-data propagation #14443

@DonLakeFlyer

Description

@DonLakeFlyer

Background

Work begun in a development session added bounded retry logic and no-tile terminal-failure classification to terrain tile fetching, along with unit tests. The work was reverted before merging because it became too large for a stable release cycle. This issue captures everything that needs to be done properly.

Production changes needed

QGeoTiledMapReplyQGC (src/QtLocationPlugin/)

  • Add FailureClassification enum (None, NoTile, RetryExhausted, Permanent)
  • Implement bounded retry with kMaxRetryCount = 3 and back-off delays {500, 1000, 2000} ms
  • Classify HTTP failure modes:
    • 404, 410 → NoTile (no retry)
    • 408, 425, 429, 500, 502, 503, 504 → Retryable
    • All others → Permanent
  • Classify retryable network errors: TimeoutError, TemporaryNetworkFailureError, NetworkSessionFailedError, HostNotFoundError, ConnectionRefusedError, RemoteHostClosedError
  • Expose failureClassification() accessor
  • Make init(), failureClassification(), _startNetworkRequest(), and a _retryDelayMs(int) hook virtual (needed for test subclassing)
  • Promote _networkManager to protected

TerrainTileManager (src/Terrain/)

  • Add QSet<QString> _noTerrainTiles — a permanent blacklist of tile hashes that returned 404/410
  • Guard _noTerrainTiles reads and writes with _tilesMutex (same pattern as _tiles)
  • In getAltitudesForCoordinates: check _noTerrainTiles first; return true with *noTerrainData = true on hit (skips cache + network)
  • In _terrainDone: on NoTile classification insert hash into _noTerrainTiles; on other errors call _tileFailed()
  • Add bool *noTerrainData out-param to getAltitudesForCoordinatesmake it required (not optional); update all call sites
  • Add protected virtual _createTileReply() factory (enables test injection without touching the network)
  • Drain loop in _terrainDone must propagate noTerrainData = true for all three query modes (coordinate, path, carpet)

TerrainQuery (src/Terrain/)

  • Thread noTerrainData param through TerrainAtCoordinateQuery::getAltitudesForCoordinates

TerrainProtocolHandler (src/Vehicle/)

  • In _sendTerrainData: check noTerrainData flag and retire the specific gridBit from _currentTerrainRequest.mask — prevents infinite retry for uncovered ocean/arctic areas
  • In _handleTerrainReport debug block: distinguish "NoTile" / "Error" / "N/A" / altitude value in the qgcAlt string

Test changes needed

test/QtLocationPlugin/QGeoTiledMapReplyQGCTest (new file)

Uses FakeNetworkAccessManager + FakeNetworkReply + TestableQGeoTiledMapReplyQGC (zero-delay retries):

  • _testNetworkErrorRetryExhaustsAfterThreeAttempts — 4× TimeoutErrorcallCount == 4, RetryExhausted
  • _testNetworkErrorRetrySucceedsOnSecondAttemptTimeoutError then HTTP 200 → callCount == 2, not RetryExhausted
  • _testHttpRetryableStatusRetryExhausts — 4× HTTP 503 → callCount == 4, RetryExhausted
  • _testHttpNoTileDoesNotRetry — HTTP 404 → callCount == 1, NoTile

test/Terrain/TerrainTileManagerTest (new file)

Uses MockTerrainTileReply + TestableTerrainTileManager + StubTerrainQueryInterface:

  • _testNoTilePropagatesCoordinateFailure
  • _testRetryExhaustedPropagatesCoordinateFailure
  • _testPermanentErrorPropagatesCoordinateFailure
  • _testNoTilePropagatesPathFailure
  • _testNoTilePropagatesCarpetFailure
  • _testQueuedCoordinateQueryDrainedOnNoTile — second query enqueued while downloading; _terrainDone NoTile drains it
  • _testQueuedPathQueryDrainedOnNoTile — same for path query

Note on queued-drain tests: the second stub's signal never fires in early attempts because _terrainDone's drain loop calls getAltitudesForCoordinates which hits _noTerrainTiles and returns true — but the queued request's tile hash may differ from the one just inserted (path/coordinate queries span multiple tile hashes). Care is needed to ensure all coordinates in the queued request map to the same tile hash as the one that was blacklisted, or to use coordinates that provably fall in the same tile.

Notes

  • _noTerrainTiles grows without bound — consider a size cap or LRU for long-running sessions (separate issue)
  • Clear _noTerrainTiles when the active elevation provider changes (settings change signal)

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions