Port camp2 multi-layer + OSM/WMTS map system into deployed CAMP; retire obsoleted + dead ROS 1 code#60
Open
rolker wants to merge 49 commits into
Open
Port camp2 multi-layer + OSM/WMTS map system into deployed CAMP; retire obsoleted + dead ROS 1 code#60rolker wants to merge 49 commits into
rolker wants to merge 49 commits into
Conversation
added 5 commits
June 1, 2026 20:23
Stage the camp2 map-system port (Web-Mercator scene, multi-layer + OSM/WMTS, tabbed Layers/Mission UI, manager-window retirement) into 6 self-contained PRs; PR1 dead-code sweep and PR2 substrate import carry no behavior change.
Depth preserved as a first-class layer type (multiple layers, tree-order overlap resolution); one-time settings reset; start an ADR system in camp; no radar on this boat; preserve scale-dependent rendering behavior.
Reframe as adopt-framework-preserve-functionality (three buckets); extract camp2 map into shared libcamp_map (camp2 not retired); commit parity matrices (Phase 0) and gate every replacement on verified >= parity; add tests where practicable; split PR3 (3a/3b/3c) + geoToPixel shim. Resolves review findings.
Delete the dead ROS 1 and disabled code the camp2 map port obsoletes,
ahead of the substrate import. No behavior change.
- radar/ (ROS 1 RadarSector/OpenGL; superseded by the grids path) plus
its orphaned menu actions, slots, and showRadar/selectRadarColor
project signals (the connections that consumed them were already
commented out, so the menu items were invisible no-ops)
- geoviz/ (ROS 1, 0 CMakeLists refs)
- sonar_manager/ (.cpp not compiled; only a stale .ui was referenced)
- sound_play/ (commented out pending ROS 2 port) plus dead remnants,
including the inert "Say something" menu item whose slot was already
commented out
- scaledview.{h,cpp} (unused legacy zoom/pan helper)
- RadarDisplayType/GeovizDisplayType GeoGraphicsItem type() enum entries
- CMakeLists.txt references to all of the above
Part of #59.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rolker
changed the title
Per-pair parity matrices built from actual source (file:line cited), gating later replacements. Confirms the three buckets and records the retirement blockers each true-replacement pair must clear before its camp original is deleted: - raster: depth band + getDepth(geo) query is camp-only (-> PR3c); color tables / mipmap / band compositing are parity (verbatim copy) - markers: DELETEALL all-namespace fan-out is a camp2 regression; DELETE leaks empty objects; CUBE is a camp2 gain to keep - grids: OccupancyGrid colormap is byte-for-byte parity; GridMap speed colormap + fixed range is camp-only (bag-replay verify) Open UX decisions (GridMap speed coloring, marker fill-alpha) recorded per pair for resolution in PR5. Part of #59. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- GridMap: drop camp's situational speed colormap, adopt camp2 grayscale auto-range. Reusable selectable-colormap facility (camp::map::ColorMap, also a depth-shading consumer) split out to #63, out of #59 scope. - Markers: fill alpha becomes a config option rather than hardcoding camp's half or camp2's full; default chosen at PR5. Part of #59. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p_map_ros Core is verified ROS-free and ros/ depends on it one-directionally, so the split enforces the layering at compile time and lets pure-logic gtests link Qt/GDAL only. PR2 will create two targets (libcamp_map_ros PUBLIC-links libcamp_map). Part of #59. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split camp2's map substrate into two shared libraries that the camp2
executable (and, from PR3a, CCOMAutonomousMissionPlanner) link:
- libcamp_map — pure Qt/GDAL map core (map model, Web-Mercator scene,
OSM/WMTS tiles, reprojected rasters, tree view, tools).
Verified ROS-free: ldd shows zero ROS libraries.
- libcamp_map_ros — ROS-dependent layers (geometry, grids, markers) + node
plumbing; links libcamp_map + rclcpp/tf2/messages.
To break the core<->ros circular dependency, invert Map's node creation:
Map no longer constructs a camp::ros::Node (which lives in the ros layer).
It now stores its ToolsManager and exposes Map::toolsManager(); the app
layer (MainWindow) attaches the ROS node there — same parent, same
discovery via the item tree, same construction timing. This is what makes
a genuinely ROS-free core possible rather than just a file move.
camp2 builds unchanged and links the new libs (self-verifying); the
CCOMAutonomousMissionPlanner build is unaffected. Drops the unused
Qt5::Test from the libraries (kept on the camp2 exe, which uses
QAbstractItemModelTester) and an unused <QApplication> include in map.cpp.
Part of #59.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR advances the #59 migration plan by (1) deleting dead/disabled ROS 1-era CAMP UI/code, (2) committing the Phase 0 parity audit docs that gate later replacements, and (3) extracting the camp2 map subsystem into shared libraries with an explicit ROS-free core boundary.
Changes:
- Decouple
camp2::map::Mapfrom ROS by exposingMap::toolsManager()and moving ROSNodeconstruction to the app layer (MainWindow). - Introduce shared libraries
camp_map(Qt/GDAL, ROS-free) andcamp_map_ros(ROS-dependent layers) and relink thecamp2executable against them. - Remove unused/disabled ROS 1 remnants from
src/camp/(radar/geoviz/sonar_manager/sound_play/scaledview) and clean up associated UI actions/slots/signals; adddocs/parity/matrices.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/camp2/map/map.h |
Adds toolsManager() accessor to support ROS-free map core. |
src/camp2/map/map.cpp |
Stops constructing the ROS node inside Map; stores/returns ToolsManager. |
src/camp2/main/main_window.cpp |
Constructs and wires the ROS Node in the app layer using map_->toolsManager(). |
CMakeLists.txt |
Extracts camp_map/camp_map_ros libs and relinks camp2; removes retired CAMP sources/UIS. |
docs/parity/* |
Adds committed parity audit docs for raster/markers/grids. |
src/camp/mainwindow.{ui,h,cpp} |
Removes dead radar/sound_play UI actions and commented-out wiring. |
src/camp/* deleted files |
Deletes dead/unused ROS 1 or unbuilt components (radar/geoviz/sonar_manager/sound_play/scaledview). |
.agent/work-plans/issue-59/{plan.md,progress.md} |
Captures the work plan/progress for #59 and the PR sequencing rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Bootstrap the PR3a design before scene code: record the decisions that let deployed camp adopt camp2's Web-Mercator map framework while preserving camp's functionality. - Scene projection -> Web Mercator (EPSG:3857), owned by camp::map::Map; charts become reprojected raster layers, not the scene foundation. - Two-model split: Layer tree (Map/MapTreeView) vs Mission tree (slimmed AutonomousVehicleProject), tabbed in one dock. - Depth as a first-class layer type; getDepthRaster() -> getDepth(geo) order resolution; parity-gates retiring BackgroundRaster (PR3c). - One shared library split at the ROS boundary (libcamp_map / libcamp_map_ros); camp2 kept as sandbox. - Migration via a geoToPixel->web_mercator::geoToMap shim: the parent-offset subtraction is coordinate-agnostic, so the real PR3a work is reparenting overlay items into Map's scene, not editing the 24 call sites. Nothing deleted in PR3a. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rLayer Deployed camp's QGraphicsScene becomes the camp::map::Map Web-Mercator scene (ADR-0002). Substrate swap only — no overlays migrated yet, nothing deleted. - CMake: link CCOMAutonomousMissionPlanner against camp_map_ros (transitively camp_map + the src/camp2 include path). - AutonomousVehicleProject owns a camp::map::Map; scene() returns Map::scene(). openBackground adds a reprojected raster::RasterLayer (exact GDAL warp to EPSG:3857) for chart DISPLAY. - BackgroundRaster is demoted to a non-painting scene anchor at the origin (ItemHasNoContents + raised Z): it keeps its georeferencing + depth band as the depth oracle (getDepth) and remains the parent every overlay attaches to, so the existing overlay/findParentBackgroundRaster machinery is untouched. This keeps mission items AND the live ROS overlays (AIS, platform, grid, collision monitor, nav_source, markers, measuring tool) working without per-overlay edits. - geoToPixel shim: GeoGraphicsItem::geoToPixel now returns web_mercator::geoToMap; the parent-offset subtraction is coordinate-agnostic and the anchor sits at the origin, so it reduces to absolute Web-Mercator. The bg argument is ignored. - ProjectView mouse<->geo conversions and Waypoint drag readout switch from the background raster's pixel space to web_mercator::mapToGeo / geoToMap. Depth stays in BackgroundRaster until it becomes a first-class layer in PR3c (parity gate to retire the raster). Chart display via RasterLayer (no depth) is why both coexist for now. Build: camp package builds green (both binaries + libs). Runtime/visual verification (chart + overlay alignment on a display) still pending. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Web Mercator is Y-increasing-north; a QGraphicsView is Y-increasing-down, so the reprojected chart and overlays rendered upside down. camp::MapView compensates with a negative-Y view scale; ProjectView was missing it. - Constructor applies scale(1, -1). Uniform wheel zooms preserve the sign; fitInView multiplies the current transform so the flip survives. - updateBackground now fitInView()s the chart's Web-Mercator extent on load (preserving the flip) instead of centering at 1:1, so the chart fits the view. Overlay labels use ItemIgnoresTransformations and stay upright. Mouse<->geo is unaffected (mapToScene already inverts the view transform). Visually verified: chart 13283 renders north-up and fits the view on load. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
camp's markers (local-frame LINE_STRIP with baked Y reflection + pixel_size_ units) and grids (drawImage costmap) render flipped/mis-scaled on the PR3a-i Web-Mercator flipped-Y scene. Deferred to PR5 (Bucket-A retire-and-replace by camp2 ros/markers + ros/grids, which are scene-correct) rather than patched. Noted in progress + parity audit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…w fixes) Addresses pre-push review findings (Claude + Copilot adversarial passes, cross-confirmed): - RasterLayer lifecycle (cross-confirmed High): openBackground created a new display RasterLayer every call and nothing removed the old one, so opening a second chart stacked both and deleting a background orphaned its layer. Track m_currentRasterLayer; replace it on open and tear it down on delete (its dtor aborts/joins the async load). Restores the pre-PR3a single-displayed-chart behaviour; multi-chart management remains PR3b (layer tree). - topLevelLayers() null guard: skip creating the chart layer if the layer list is unexpectedly absent, instead of handing RasterLayer a null parent. - fitInView robustness (Copilot): build the chart's Web-Mercator extent from all four corners, not just topLeft/bottomRight, so a rotated/sheared georeferenced chart isn't under-fit on load. Build green. Lower-priority review notes (camp_map vs camp_map_ros breadth; dropped setSceneRect) left as-is with rationale recorded in progress.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…odel) Two-model split (ADR-0002). The left panel's single mission treeView becomes a QTabWidget with two tabs: - Mission: the existing treeView (AutonomousVehicleProject) — edits the plan. - Layers: a camp::map_tree_view::MapTreeView bound to project->map() (camp::map::Map) — backgrounds, OSM/OpenSeaMap/NOAA-WMTS tiles, and chart RasterLayers, with inline visibility checkboxes + the opacity delegate. This is also where the stacked-chart layers from PR3a-i become user-manageable (toggle visibility), addressing the multi-chart review note. detailsView follows the active tab: mission selection on the Mission tab, cleared on the Layers tab (layer detail widgets are future work). Built in code (reparent treeView into the tab widget, insert at its splitter slot) to avoid reworking the .ui splitter layout / Designer custom-widget promotion. Deferred (documented): background still appears as a mission-tree node — moving it out is entangled with project save/load persistence and lands with the PR3c depth-as-layer / persistence migration. Build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ire managers First PR5 step: replace camp's flipped, BackgroundRaster-parented grids/markers with camp2's scene-correct ros overlays, hosted on camp's existing ROS node (unified, not a second node). - camp::ros::Node gains an adopt constructor (tools_manager, node, buffer): uses an externally-owned rclcpp node + tf buffer instead of spawning its own thread, defers manager creation to the event loop, and skips rclcpp::shutdown() / the thread teardown in the destructor (the node belongs to the host). It also skips the geometry (PolygonStamped) manager, since camp keeps its own collision-zone renderer for those topics (avoids double-drawing). camp2's standalone path is unchanged (default spawning ctor + geometry). - MainWindow attaches a camp::ros::Node to project->map()->toolsManager() on the first ROSLink rosConnected; it auto-discovers OccupancyGrid/GridMap/Marker topics and creates layers in the Layers tab (rendered correctly, via ColorMap). - Retired camp's GridManager + MarkersManager: creation, members, the two manager-window menu actions/slots. camp's AIS, collision-monitor, platform, nav-source overlays are unchanged for now (no camp2 equivalent; re-homed later). Build green (both binaries). Runtime verification pending (needs a running ROS with costmap/marker topics). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+72
to
+76
| // Last received message, cached so a colormap change can re-render without | ||
| // waiting for the next publish (live costmaps would also pick it up). | ||
| grid_map_msgs::msg::GridMap last_msg_; | ||
| bool has_last_msg_ = false; | ||
|
|
Comment on lines
36
to
40
| void GridMap::gridMapCallback(const grid_map_msgs::msg::GridMap &data) | ||
| { | ||
| last_msg_ = data; // [camp#63] keep the latest for colormap re-render | ||
| has_last_msg_ = true; | ||
| if(!process_future_.isRunning()) |
Comment on lines
204
to
208
| void RasterLayer::imageReady() | ||
| { | ||
| auto result = future_watcher_.result(); | ||
| prepareGeometryChange(); | ||
|
|
Comment on lines
+112
to
+116
| // [camp#63] Normalised value -> colour via the selectable ramp | ||
| // (colorNormalized clamps to [0,1]); default grayscale reproduces the | ||
| // prior output. | ||
| value = (value - min_value) / (max_value - min_value); | ||
| uint8_t ival = std::min(1.0,std::max(0.0, value))*255; | ||
| grid_layer_data.grid_image.setPixelColor(QPoint(size.x()-1-iterator.getUnwrappedIndex().x(), iterator.getUnwrappedIndex().y()), QColor(ival, ival, ival, 255)); | ||
| grid_layer_data.grid_image.setPixelColor(QPoint(size.x()-1-iterator.getUnwrappedIndex().x(), iterator.getUnwrappedIndex().y()), cm.colorNormalized(value)); |
Comment on lines
68
to
72
| case MouseMode::addWaypoint: | ||
| if(bg) | ||
| { | ||
| m_project->addWaypoint(bg->pixelToGeo(mapToScene(event->pos()))); | ||
| m_project->addWaypoint(web_mercator::mapToGeo(mapToScene(event->pos()))); | ||
| } |
Comment on lines
217
to
220
| QPointF transformedMouse = mapToScene(event->pos()); | ||
| BackgroundRaster *bg = m_project->getBackgroundRaster(); | ||
| BackgroundRaster *dr = m_project->getDepthRaster(); | ||
| if(bg) | ||
| { |
Comment on lines
+291
to
+295
| install(TARGETS camp_map | ||
| LIBRARY DESTINATION lib | ||
| ARCHIVE DESTINATION lib | ||
| RUNTIME DESTINATION bin) | ||
|
|
Comment on lines
+343
to
+347
| install(TARGETS camp_map_ros | ||
| LIBRARY DESTINATION lib | ||
| ARCHIVE DESTINATION lib | ||
| RUNTIME DESTINATION bin) | ||
|
|
Comment on lines
328
to
+332
| std::vector<QGeoCoordinate> ret; | ||
| for(int i = 0; i < guidePath.size(); i++) | ||
| { | ||
| double depth = depthRaster.getDepth(guidePath[i]); | ||
| double depth = project->getDepth(guidePath[i]); | ||
| // [#59 PR3c] Skip guide points with no depth coverage rather than |
Comment on lines
+122
to
+130
| double min_value = std::numeric_limits<double>::max(); | ||
| double max_value = std::numeric_limits<double>::lowest(); | ||
| for(float v : values) | ||
| { | ||
| if(std::isnan(v) || (has_nodata && v == nodata)) | ||
| continue; | ||
| min_value = std::min(min_value, double(v)); | ||
| max_value = std::max(max_value, double(v)); | ||
| } |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s overlays) PR5 step 1 retired camp's GridManager/MarkersManager; the underlying classes are now unreferenced (confirmed: only stale comments mention them). Remove them and their now-orphaned converter test: - src/camp/grids/ (grid, grid_manager + .ui) - src/camp/markers/ (markers, markers_manager, markers_converter + .ui) - test/test_markers_converter.cpp (tested camp's retired markers converter) - CMakeLists SOURCES/UIS entries + the test_markers_converter gtest. Grids/markers are now exclusively camp2's scene-correct ros overlays. Build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+219
to
+227
| void RasterLayer::setColormap(map::ColorMap::Type type) | ||
| { | ||
| if(type == colormap_.type()) | ||
| return; | ||
| colormap_.setType(type); | ||
| writeSettings(); | ||
| if(!filename_.isEmpty()) // re-warp + re-shade with the new ramp | ||
| loadFile(filename_); | ||
| } |
Comment on lines
+83
to
+88
| float depthAt(int x, int y) const | ||
| { | ||
| if(x < 0 || y < 0 || x >= gridSize || y >= gridSize) | ||
| return unknownDepth; | ||
| return depthGrid[y*gridSize + x]; | ||
| } |
Comment on lines
+70
to
+74
| **Open decisions — RESOLVED 2026-06-02:** | ||
| - **GridMap colormap:** drop camp's situational "speed" ramp → adopt camp2 | ||
| grayscale auto-range. A reusable selectable-colormap facility | ||
| (`camp::map::ColorMap`, also a depth-shading consumer) is tracked out of #59 | ||
| scope in [#63](https://github.com/rolker/camp/issues/63). |
Comment on lines
+3
to
+6
| #include <array> | ||
| #include <cmath> | ||
| #include <vector> | ||
|
|
added 2 commits
June 5, 2026 09:15
Brings in the field-import callback-group concurrency fix (#67) merged to jazzy. The only conflict is a modify/delete on src/camp/grids/grid.cpp: #67 patched the legacy Grid class (dedicated callback group + TF wait), while this branch's PR5 retires the entire legacy grids/markers families in favour of the camp2 ROS overlays. Resolution: keep the deletion. The starvation #67 fixed cannot recur in the camp2 path: the overlay's heavy QImage build + TF lookup run on a QtConcurrent worker off the ROS executor (isRunning() frame-drop guard), so the grid can never starve overlays or realtime telemetry regardless of callback group. # Conflicts: # src/camp/grids/grid.cpp
…orm cache) The camp2 ROS layers resolved earth<-frame with a zero-timeout lookupTransform(TimePointZero) and dropped the whole frame on any miss, so a momentary TF stream hiccup blanked grids/markers (white flicker / dropped maps). The legacy Grid carried a dedicated-callback-group + 0.5s TF wait for this (#67); that file is retired by PR5, and the camp2 path already runs heavy work off the executor on a QtConcurrent worker, so a blocking wait is unnecessary here. Instead of waiting, cache the last good earth<-frame transform per frame_id and reuse it while it is younger than a 1s staleness budget. The earth<-map/odom transform is quasi-static, so holding the last placement across a brief gap is imperceptible and far better than blanking; cold start (frame never resolved) or an over-budget gap still throws so callers drop as before. Bounded so a sustained outage or a genuinely moving frame ages out rather than showing stale data. The decision is extracted as a pure function (lookupEarthTransformCached) so the fresh / reuse / expire / cold-start branches are unit-tested without a node or the system clock; Layer::lookupEarthTransform is the thin mutex-guarded wrapper (the lookup runs on QtConcurrent workers for grids and ROS callback threads for markers). All four call sites (occupancy_grid, grid_map, markers, polygon) inherit the behaviour via transformToWebMercator. - new: src/camp2/ros/transform_cache.{h,cpp} - test: test/test_transform_cache.cpp (7 cases, all passing) - docs: markers parity row updated (transform-cache, not message-buffer)
Comment on lines
+156
to
+164
| void GridMap::setColormap(map::ColorMap::Type type) | ||
| { | ||
| if(type == colormap_.type()) | ||
| return; | ||
| colormap_.setType(type); | ||
| writeSettings(); | ||
| if(has_last_msg_) // re-render the cached grid with the new ramp | ||
| gridMapCallback(last_msg_); | ||
| } |
Comment on lines
+122
to
+130
| double min_value = std::numeric_limits<double>::max(); | ||
| double max_value = std::numeric_limits<double>::lowest(); | ||
| for(float v : values) | ||
| { | ||
| if(std::isnan(v) || (has_nodata && v == nodata)) | ||
| continue; | ||
| min_value = std::min(min_value, double(v)); | ||
| max_value = std::max(max_value, double(v)); | ||
| } |
Comment on lines
77
to
79
| for(const auto & layer: grid_map.getLayers()) | ||
| { | ||
| GridMapLayerData grid_layer_data; |
Comment on lines
36
to
42
| void GridMap::gridMapCallback(const grid_map_msgs::msg::GridMap &data) | ||
| { | ||
| last_msg_ = data; // [camp#63] keep the latest for colormap re-render | ||
| has_last_msg_ = true; | ||
| if(!process_future_.isRunning()) | ||
| { | ||
| process_future_ = QtConcurrent::run(this, &GridMap::processGridMap, data); |
Comment on lines
+219
to
+227
| void RasterLayer::setColormap(map::ColorMap::Type type) | ||
| { | ||
| if(type == colormap_.type()) | ||
| return; | ||
| colormap_.setType(type); | ||
| writeSettings(); | ||
| if(!filename_.isEmpty()) // re-warp + re-shade with the new ramp | ||
| loadFile(filename_); | ||
| } |
Comment on lines
+256
to
+261
| if(type != colormap_.type()) | ||
| { | ||
| colormap_.setType(type); | ||
| if(!filename_.isEmpty()) | ||
| loadFile(filename_); | ||
| } |
added 5 commits
June 5, 2026 09:54
The camp#63 colormap feature left GridMap's shared state (colormap_, last_msg_, has_last_msg_, process_future_) accessed from three threads — the ROS subscription callback, the UI thread (setColormap / contextMenu / readSettings), and the QtConcurrent render worker — with no synchronization. That is a data race / UB (sporadic crashes). Separately, gridMapCallback() only launched a render when process_future_ was idle and nothing re-checked when the worker finished, so a colormap change made mid-render was silently dropped (the UI kept the old ramp until the next publish; for a static grid, forever). Fix: - Add a QMutex guarding all shared state. setColormap/readSettings take it for their colormap_ writes; gridMapCallback takes it for last_msg_. - The worker is now fully isolated: processGridMap takes the message and colormap by value, snapshotted under the lock at launch, so it never touches guarded state. The body moves to renderToData() returning bool; processGridMap emits on success then calls onProcessFinished(). - Replace the QFuture::isRunning() check with an explicit rendering_/render_pending_ coalescing model: a request arriving mid-render sets render_pending_, and onProcessFinished() (worker thread, under the lock) relaunches exactly once with the latest message+colormap. No lost renders, no unbounded queueing. A QFutureWatcher was unusable here — it is GUI-thread-affined but gridMapCallback runs on the ROS executor thread. - Add a destructor that sets shutdown_ (stops the worker relaunching) and joins the in-flight future, closing a latent teardown use-after-free. No unit test: GridMap is GUI/ROS-coupled with no seam. Verified by build + the existing 35-test suite; the coalescing logic is small and commented.
…nt raster) Three robustness fixes in the camp2 RasterLayer (camp#63 depth shading): - Use-after-free: setColormap() re-invokes loadFile(), which called setFuture() on a new QtConcurrent job without stopping the previous one. The watcher only tracks the latest future, so a replaced job became untracked — it kept running, raced the new job on shared state, and could outlive the layer (the dtor only joins the watched future). loadFile() now aborts and joins any in-flight load, then re-arms abort_flag_, before launching the new one. - Failed-load guard: imageReady() applied result.scale/pos from a default-constructed (zero-filled) LoadResult when a load failed (null/unreprojectable dataset), collapsing the item to zero scale at the origin and clearing the status. It now bails on an empty mipmap set and reports "(load failed)". - Constant raster vanishing: a scalar raster whose valid samples are all equal gave min_value == max_value, hitting ColorMap::color()'s degenerate max<=min guard (transparent for every pixel) — the raster disappeared. Widen the range when min==max (as grid_map already does) so the value maps to the top of the ramp; an all-nodata raster keeps min/max crossed and correctly stays transparent.
…ct guard - astar extendedPathAverageDepth(): the obstacle checks passed a double coordinate to Context::depthAt(int,int), which truncated toward zero instead of flooring. At the grid boundary a point at e.g. y=-0.3 read row 0 (a real cell) instead of being treated as out-of-grid (obstacle), so an off-grid point could read as free water. Make every conversion explicit: floor() for the cell containing a fractional coordinate (matching the existing floor_x/ceil_x straddle), round() for the nearest-cell cost sample. Removes the implicit narrowing and the major/minor-axis asymmetry between the two branches. - trackline planPath(): guard autonomousVehicleProject() before dereferencing it (avp->hasDepth()). It can return null for an item not yet attached to a project (construction/load), which would crash.
Low-risk fixes from the PR #60 triage: - color_map.cpp: include <algorithm> explicitly (std::min/max in colorNormalized were only transitively available). - grid_map.h: include <QImage>/<QPointF>/<string>/<utility>/<vector> so the GridMapLayerData/GridMapData structs don't depend on transitive includes. - surveyarea generateNextLine(): capture guidePath.size() once as int and iterate/clamp against it, removing the signed/unsigned loop comparison. - parity README: ColorMap is no longer "tracked out of #59 scope" — it is implemented in this PR and consumed by grid_map and the raster layer; update the decision note to match.
Comment on lines
+252
to
255
| AutonomousVehicleProject *project = autonomousVehicleProject(); | ||
|
|
||
| if(wps.size() > 2 && project->hasDepth()) | ||
| { |
Comment on lines
+65
to
+76
| TEST(ColorMap, NameRoundTrip) | ||
| { | ||
| for(auto type : ColorMap::allTypes()) | ||
| { | ||
| bool ok = false; | ||
| EXPECT_EQ(ColorMap::typeFromName(ColorMap::name(type), &ok), type); | ||
| EXPECT_TRUE(ok); | ||
| } | ||
| bool ok = true; | ||
| EXPECT_EQ(ColorMap::typeFromName("nope", &ok), ColorMap::Grayscale); | ||
| EXPECT_FALSE(ok); | ||
| } |
Comment on lines
+291
to
+301
| // Pre-sample depth at each cell centre (geo query, scene-independent). | ||
| astar::Context c; | ||
| c.start.x = start.x(); | ||
| c.start.y = start.y(); | ||
| c.finish.x = finish.x(); | ||
| c.finish.y = finish.y(); | ||
| c.map = depthRaster; | ||
| c.gridSize = N; | ||
| c.depthGrid.resize(static_cast<size_t>(N)*N); | ||
| for(int gy = 0; gy < N; gy++) | ||
| for(int gx = 0; gx < N; gx++) | ||
| { | ||
| const QPointF centre(originX + (gx+0.5)*cellSize, originY + (gy+0.5)*cellSize); | ||
| const float d = avp->getDepth(web_mercator::mapToGeo(centre)); | ||
| c.depthGrid[static_cast<size_t>(gy)*N + gx] = std::isnan(d) ? astar::Context::unknownDepth : d; | ||
| } |
| typedef boost::geometry::model::multi_linestring<BLineString> BMultiLineString; | ||
|
|
||
| std::vector<QGeoCoordinate> generateNextLine(std::vector<QGeoCoordinate> const &guidePath, BackgroundRaster const &depthRaster, double tanHalfSwath, int side, BPolygon const &area_poly, double stepSize, BMultiLineString const & previousLines); | ||
| std::vector<QGeoCoordinate> generateNextLine(std::vector<QGeoCoordinate> const &guidePath, AutonomousVehicleProject *project, double tanHalfSwath, int side, BPolygon const &area_poly, double stepSize, BMultiLineString const & previousLines); |
added 2 commits
June 5, 2026 10:49
…TS-only ops) ProjectView gated waypoint / trackline / survey-pattern / survey-area / avoid-area / search-pattern creation, the mouse-move cursor readout + pending-item updates, and the right-click boat-command menu (Hover/Goto/ Idle/Look Here) on a BackgroundRaster being loaded. That guard dates to when the raster defined the coordinate system; since PR3a the scene is Web Mercator and every conversion uses web_mercator::mapToGeo(), which is chart-independent. The guard now needlessly blocked all mission planning and boat commands over OSM/WMTS-only backgrounds. Remove those guards. Depth readout still queries by geo and is simply omitted (existing isnan check) when no source covers the cursor. Kept the two genuinely chart-dependent uses: saving/restoring the view center and fitting the view to the loaded chart's extent (bg->boundingRect()/pixelToGeo()). bg is also still read in mousePressEvent for the middle-button MeasuringTool, which is parented to it — that coupling belongs with the eventual BackgroundRaster retirement. Resolves PR #60 triage findings #9-#15. GUI input path, not covered by the unit suite (still 35/35) — needs a runtime check (create items / read cursor over an OSM-only background) before merge.
Comment on lines
4
to
6
| #include "../map/layer.h" | ||
| #include "../map/color_map.h" | ||
| #include <QFutureWatcher> |
Comment on lines
+248
to
+256
| void RasterLayer::setColormap(map::ColorMap::Type type) | ||
| { | ||
| if(type == colormap_.type()) | ||
| return; | ||
| colormap_.setType(type); | ||
| writeSettings(); | ||
| if(!filename_.isEmpty()) // re-warp + re-shade with the new ramp | ||
| loadFile(filename_); | ||
| } |
Comment on lines
99
to
+107
| if(!grid_map::GridMapRosConverter::fromMessage(data, grid_map)) | ||
| { | ||
| RCLCPP_WARN_STREAM_THROTTLE(node->get_logger(), clock, 2, "Unable to convert GridMap message"); | ||
| return; | ||
| return false; | ||
| } | ||
| if(grid_map.getLayers().empty()) | ||
| { | ||
| RCLCPP_WARN_STREAM_THROTTLE(node->get_logger(), clock, 2.0, "Got GridMap message with no layers"); | ||
| return; | ||
| return false; |
Comment on lines
1
to
12
| #include "grid_map.h" | ||
| #include <grid_map_ros/grid_map_ros.hpp> | ||
| #include "../../map_view/web_mercator.h" | ||
| #include <geometry_msgs/msg/pose_stamped.hpp> | ||
| #include "../node.h" | ||
| #include "marine_autonomy/gz4d_geo.h" | ||
| #include <tf2/utils.h> | ||
| #include "grid_layer.h" | ||
| #include <QMenu> | ||
| #include <QAction> | ||
| #include <QSettings> | ||
|
|
Comment on lines
57
to
+67
| Node::~Node() | ||
| { | ||
| emit shuttingDownRos(); | ||
| rclcpp::shutdown(); | ||
| node_thread_.quit(); | ||
| node_thread_.wait(); | ||
| if(owns_thread_) | ||
| { | ||
| // Only the node we spawned do we shut down; an adopted node belongs to its | ||
| // owner and may still be in use elsewhere. | ||
| rclcpp::shutdown(); | ||
| node_thread_.quit(); | ||
| node_thread_.wait(); | ||
| } |
Comment on lines
+252
to
255
| AutonomousVehicleProject *project = autonomousVehicleProject(); | ||
|
|
||
| if(wps.size() > 2 && project->hasDepth()) | ||
| { |
added 2 commits
June 5, 2026 11:18
First increment of the BackgroundRaster retirement. The deployed planner's
depth oracle rode on the BackgroundRaster (a QGraphicsItem/MissionItem):
AutonomousVehicleProject::getDepth queried m_currentDepthRaster, a
BackgroundRaster*. That welded depth data to the chart's graphics identity
and was one of the three structural blockers to deleting BackgroundRaster
(the others: overlay parent-anchoring, and the mapScale/scaledPixelSize
helpers).
Extract depth into a standalone, non-graphics DepthRaster (inherits
Georeferenced; loads only the Float32 depth band). AutonomousVehicleProject
now owns a DepthRaster, created from the chart file in openBackground() and
torn down alongside the chart's RasterLayer — mirroring the existing
m_currentRasterLayer ownership. getDepth/hasDepth query it; the public API
is unchanged.
The load mirrors BackgroundRaster's exactly (same first-Float32-band
detection, same GF_Read, same Georeferenced geo->pixel), so depth values
are unchanged. getDepth degrades safely (NaN) on a missing/invalid chart so
the planner reads "unknown" (unsafe) rather than a garbage depth.
- new: src/camp/depth_raster.{h,cpp}
- test: test/test_depth_raster.cpp (safe-degradation: invalid file -> NaN,
no crash). Real-chart depth correctness is verified in sim.
- AVP: m_currentDepthRaster (BackgroundRaster*) -> m_depthRaster
(DepthRaster*, owned); removed the now-unused getDepthRaster().
BackgroundRaster still loads its depth band (now redundant) and the full
chart image; both are removed in a later increment once this is sim-verified.
SAFETY: depth feeds shoal avoidance. Verify in sim (cursor depth readout +
planner avoiding shallow water over a real depth chart) before field use.
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.
Closes #59
Plan: Port camp2 multi-layer + OSM/WMTS map system into deployed CAMP
Issue
#59
Context
Deployed CAMP (
CCOMAutonomousMissionPlanner, built fromsrc/camp/) issingle-background and chart-centric: one GDAL raster is the
QGraphicsScene(chart-native WKT projection), every overlay is parented to that
BackgroundRasterandpositioned via
geoToPixel(), and there is no layer model.src/camp2/(a secondexecutable in the same CMake project,
CMakeLists.txt:237+) contains a self-containedmap subsystem CAMP lacks — a
Map(QAbstractItemModel) +Layer/LayerListtree,OSM/WMTS tiles with disk cache, and GDAL rasters reprojected into a global
Web-Mercator scene as ordinary layers.
The crux is the scene coordinate system: adopting the camp2 map system means adopting
Web Mercator as CAMP's scene projection, which touches every overlay's positioning
(~25 files call
geoToPixel). It is tractable because overlays already store canonicalposition as
QGeoCoordinateand derive scene pixels on demand — the core change isswapping
geoToPixel(geo, bgr)forweb_mercator::geoToMap(geo)and dropping theparent-to-background rule.
Framing: adopt the framework, preserve the functionality
This is not "replace camp with camp2." camp2's ROS layer set is only
geometry,grids,markers— it has no AIS, collision monitor, platform, nav_source, missionplanning, vector datasets, measuring tool, or orbit. So most of camp is re-homed onto
the new framework, not replaced. Three buckets:
camp::map::Layer— preserve all logic, swap only the coordinate substrateParity rule (load-bearing): never delete a camp original until its replacement is
confirmed ≥ parity — verified by test or manual check, not merely "it compiles." The
audit is bidirectional: keep camp2's improvements too (e.g. marker
CUBE).Resolved Decisions (from discussion, 2026-06-01)
libcamp_map(namespaced
camp::map::) that bothCCOMAutonomousMissionPlannerand thecamp2executable link. camp2 is NOT retired — it stays a working sandbox. No code
duplication. Lib boundary for the ROS-dependent layers (single lib vs sibling
libcamp_map_ros) is a PR2 detail.docs/parity/or.agents/) — areviewable, durable matrix per replacement pair, not just inline analysis.
gtest in
test/); use manual verification for Qt rendering / UI seams that aren'tunit-testable. Don't force tests onto framework glue.
toggled in the tree;
getDepthRaster()(single) →getDepth(geo)query over theenabled depth layers in tree order, first valid wins. Keeps depth-aware A*,
survey/trackline generation, and the cursor depth readout. (Raster pair's parity gate.)
camp(docs/decisions/, ADR-0001"adopt ADRs" + architecture ADR). Lands with PR3a.
still deleted (PR1). Not a deployment gate.
bgr->mapScale()→MapViewviewport scale; distances stay geodesic. Implementation detail.
Approach (stacked PRs on
feature/issue-59)radar/(ROS 1, disabled),geoviz/(ROS 1, 0 CMake refs),sonar_manager/(ROS 1,.cppnot built),sound_play/(commented out),scaledview.{h,cpp}(unused); cleanCMakeLists.txt.true-replacement pair (raster, grids, markers) commit a matrix
camp feature | camp2 feature | delta | resolution; confirm the carryover inventory.Known deltas to capture: raster→depth band (camp-only) + color tables (parity ✓);
markers→DELETE/DELETEALL actions (camp) vs CUBE + expiry timer (camp2); grids→
one-class-both-types (camp) vs split classes (camp2), colormap/range parity.
libcamp_map: move camp2's map substrate (+ shared ROS layerframework) into a shared library; repoint the
camp2executable to link it andconfirm camp2 still builds and runs unchanged (self-verifying). No camp changes yet.
Mapowns the scene;AutonomousVehicleProjectshrinks toward a mission model contributing a missionlayer; charts shown via reprojected
RasterLayer; add OSM tile layer; add ageoToPixel→web_mercator::geoToMapshim inGeoGraphicsItemso not-yet-migratedoverlays still compile/run. Bootstrap
docs/decisions/+ write the architecture ADR.Nothing deleted yet.
MapTreeView;Mission = existing
treeView); background stops being a mission-tree node.getDepth(geo)query → satisfies the raster pair's parity gate.Waypoint,TrackLine,SurveyPattern,SurveyArea,AvoidArea,SearchPattern, vectorPoint/LineString/Polygon) off the shim toweb_mercator::geoToMap; repointdepth-aware survey/trackline + A* at the
getDepth(geo)query. Note: A* iterates apixel grid today — it needs a defined planning grid/resolution under the multi-depth
model (more than a query repoint; may be its own sub-PR).
camp-only overlays (
NavSource,Markers-carryover bits,Platform/ShipTrack,CollisionMonitor,AISContact) onto the framework. For the replacement pairs(grids, markers): confirm ≥ parity first (port
DELETE/DELETEALLinto shared markersif expiry doesn't cover it; verify grid colormap/range), then switch camp to the shared
layers and retire camp's
grids/grid.*+ markers. Delete the four overlay managerwindows (
GridManager,AISManager,MarkersManager,CollisionMonitorManager) +menu actions — visibility moves to inline layer-tree checkboxes.
backgroundraster.*,backgrounddetails.*,georeferenced.*; remove thegeoToPixelshim +updateBackground/updateProjectedPointsplumbing; add themissing
.agents/README.md(architecture section).Each PR builds and runs on its own; PR1, Phase 0, and PR2 are low-risk and land first.
Testing (where practicable)
web_mercatorgeo↔map round-trip +metersPerUnitat latitudegetDepth(geo)order resolution (overlap → top enabled wins)DELETE/DELETEALLparityFiles to Change (representative; full tables in the issue)
CMakeLists.txtlibcamp_map+ relink camp2 (PR2); link camp (PR3a); drop manager UIs (PR5)src/camp/{radar,geoviz,sonar_manager,sound_play}/,scaledview.*docs/parity/*.mdsrc/camp2/{map,map_view,map_tiles,wmts,map_tree_view,background,raster,util}/(+ sharedros/)libcamp_map(PR2)docs/decisions/0001-*.md,docs/decisions/000X-web-mercator-scene-and-layer-model.mdgetDepth(geo)resolution (PR3c)autonomousvehicleproject.{h,cpp}getDepthRaster()→ depth query (PR3a/PR3c)mainwindow.{ui,cpp}Map; remove managershow()actions (PR3b/PR5)geographicsitem.{h,cpp},projectview.{h,cpp}geoToPixelshim →geoToMap; cursor depth via query (PR3a/PR4)astar.cpp,surveyarea.cpp,trackline.cppgetDepth(geo); A* planning grid (PR4)waypoint,survey*,vector/*,ais/*,markers/*,platform_manager/*,collision_monitor/*,nav_source,grids/grid)geoToMap; re-home onto Layer framework; parity-gated retirement of replaced ones (PR4/PR5)backgroundraster.*,backgrounddetails.*,georeferenced.*,grids/grid.*.agents/README.md,test/Principles Self-Check
ADR Compliance
feature/issue-59worktree; PRs tojazzy## Plan Authored/## Plan Reviewentries recordeddocs/decisions/+ architecture ADR (Web-Mercator scene, two-model split, depth-as-layer, shared lib)Consequences
libcamp_mapgetDepthRaster()→getDepth(geo)astar.cpp,surveyarea.cpp,trackline.cpp,projectview.cpp, menu gatingmainwindowmenu + action wiringOpen Questions
libcamp_mapvs splitting ROS-dependent layers intolibcamp_map_ros— settle in PR2.Estimated Scope
Multiple PRs (PR1, Phase 0, PR2, PR3a/b/c, PR4, PR5, PR6 — ~9 units), stacked on
feature/issue-59. PR1/Phase 0/PR2 are low-risk; PR3a is the foundational scene swap;PR3b–PR6 are incremental, parity-gated migration and cleanup.