feat(radar): add radar minimap overlay to the Distance/Bearings frame - #10224
feat(radar): add radar minimap overlay to the Distance/Bearings frame#10224egorsiniaev wants to merge 58 commits into
Conversation
|
|
github-actions
Bot
added
needs-review
I have 500 Internal Server Error after signing it. |
|
We have 3 nodelist displays with different additonal things. This adds a 4th list with a graphical representation. Also overlaps with the functionality of the GPS Compass. Maybe we can have this panel as an option instead of the GPS Compass or even merge it with the compass. |
The CLA server seems to have issues. Can you try again maybe tomorrow? |
I thought about it, but because this is my first PR and contribution - to suggest such merge without long term community vision is a strange step. That's why I decided to have clear separate section that easy to review + easy to disable if people don't need it. |
|
Problem is, we rather would like to make the interface more clear, not pile on it. I see the value in your contribution, i just think i t would really benefit the existing screens rather than open up a new one. |
|
This is cool :) I want to see how it looks on a larger screen, like the sensecap indicator - will try when I get time. Also, appreciate your collaborative approach. |
egorsiniaev
changed the title
I updated. Removed separate Radar view and add option to switch Compass mode Radar mode. Plus changed a bit orientation (nodes on the left, radar on the right) to match compass layout. Please take a look. Should I need to update screenshots? They are still showing old layout. |
Will appreciate for checking it. I have no such devices yet. |
|
Hey @caveman99 Thanks for your suggestion. I'v made changes and pushed them. Today made photos and record video for better representation. In short - I extended Compass, add long-press menu to switch to the radar view + changed a bit layout to match compass. Please take a look and let me know if anything else is missing. |
|
Hey @caveman99 @fifieldt |
|
Reviewing the video - I do like the idea and concept. It's well executed and provides a display not so dissimilar to what is in InkHUD. Need to load it up on a couple devices to give it a run through. |
Xaositek
left a comment
Xaositek
left a comment
There was a problem hiding this comment.
Tested it out on my Cardputer and I can't see it actually working. I tried Zooming In and Out, verified I have a GPS lock, and nodes are pretty close by.
I feel like we need some refinements like "What is my current Zoom level?", "Do I have GPS lock"?
Also the toggle between Compass and Radar likely needs to be similar to what we have on the Clock screen where you click on "Clock Face" and then choose Digital or Analog.
IMG_3250.movVideo of my attempt |
|
I had a similar idea while creating the distance and bearing screen, I ultimately opted to not implement due to the following:
|
…stance Adds a new RadarRenderer that draws a circular radar view with three range rings. The user's node sits at the centre; other nodes with valid GPS positions are plotted as 3×3 squares at their true bearing and proportional distance. A "N" indicator marks north, the info panel to the right shows the current scale, total node count, and the name and distance of the closest node. The screen is registered for non-E-Ink builds that have GPS (HAS_GPS) and can be hidden via the existing hiddenFrames mechanism.
When BMX160 (RAK12034) is connected, Screen::setHeading() is updated by BMX160Sensor::runOnce() after tilt-compensated compass fusion. The radar now reads that heading via screen->hasHeading()/getHeading() and rotates all node bearings and the "N" indicator accordingly, so the direction the device faces is always at the top of the display (heading-up mode). Falls back to GPS-estimated track heading, then north-up when neither is available. The info panel shows "HDG-UP" or "N-UP" so the current mode is always visible.
- Centre the radar circle horizontally, filling the full content height (radius=27 on 128×64, scales up on larger displays) - Replace the wide info panel with a compact ring legend to the right: three stacked distance labels (outer/middle/inner ring scale) plus a small "IMU" indicator when the BMX160 is active - Remove per-screen closest-node text — node positions on the rings now carry the distance information visually
… long-press menu 1. Scale table — all values now multiples of 3 so ring labels (scale/3 and scale*2/3) are always whole numbers (e.g. 30/60/90m, 100/200/300m). 2. Layout — radar shifted to the left edge; right panel shows three ring scale labels, closest node short name, distance, and a node-count + orientation badge (HDG / N^) in the bottom row. 3. Icon — replaced shared icon_compass with a new icon_radar (concentric rings with centre dot) so the radar has its own menu indicator. 4. Menu toggle — "Show/Hide Radar" added to the Show/Hide Frames menu via the existing toggleFrameVisibility / isFrameHidden pattern. 5. Long-press menu — pressing SELECT on the radar screen opens "Radar Options" with: Switch N-UP / HDG-UP (overrides IMU), Zoom In, Zoom Out. Zoom is clamped to ±2 steps from auto-scale; state is held in module- static variables in RadarRenderer.
Problem: on 128×64 OLED with FONT_SMALL≈13px, the 3 ring-label rows
left no space for node info — name/distance were clipped off screen.
Fix:
- Info panel reduced to 4 rows:
Row 0 outer ring scale (scale reference)
Row 1 closest node: name (left) + distance (right-aligned)
Row 2 2nd closest node: name + distance
Row 3 node count + HDG/N^ badge
- Entries sorted by distance ascending before drawing so rows 1/2
always show the two nearest nodes.
- Closest node rendered as a + cross on the radar; all others keep
the 3×3 filled-square marker. The cross lets the user match the
top panel row to the correct dot without cluttering the radar with
text labels.
Each node is assigned a persistent marker shape based on nodeNum % 5: 0 ■ filled square 1 + axis-aligned cross 2 × diagonal X 3 □ hollow square 4 ◆ diamond The same symbol appears on the radar dot AND in the right-panel list, so the user can visually match any dot on the map to its panel entry without text labels cluttering the radar. Info panel: Row 0 outer ring scale Row 1 [sym] closest node name (left) distance (right) Row 2 [sym] 2nd closest (left) distance (right) Row 3 node count (left) orientation badge (right)
Instead of a separate screen slot in the rotation, the radar now lives inside the existing Position/Compass screen. - Remove standalone Radar frame and icon from screen rotation - Add 'Radar View' entry to the position long-press menu (positionBaseMenu) - Long-press in radar mode opens radarPositionMenu with: Compass View, N-UP/HDG-UP toggle, Zoom In, Zoom Out - UIRenderer::drawCompassAndLocationScreen delegates to RadarRenderer::drawRadarOverlay when uiconfig.radar_mode is true - Radar layout: node list (left) + radar circle (right, 2 px padding) - radar_mode persisted as bool field 20 in DeviceUIConfig (proto-compatible) - Removes nodelist_radar from hiddenFrames, framesetInfo.positions, and all frame-toggle machinery
When no nodes have been heard within NUM_ONLINE_SECS, the radar drew empty rings with a blank list and no explanation. Render a centered 'No active nodes' message in the list-panel column so the empty state is intentional and clear, leaving the radar rings intact.
Firmware Size Report22 targets | vs
Show 17 more target(s)
Updated for 065053d |
Fixes Trunk Check (clang-format) failure - file was authored with 2-space/attached-brace style; reformat to repo Linux-brace 4-space style.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a radar overlay and menu-driven tracking-view controls for the node list location screen. It also moves the online-node timeout constant from ChangesRadar Overlay Feature
Node Online Timeout Constant Relocation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant NodeListRenderer
participant RadarRenderer
participant NodeDB
participant OLEDDisplay
User->>NodeListRenderer: open location screen
NodeListRenderer->>RadarRenderer: drawRadarOverlay(display, x, y)
RadarRenderer->>NodeDB: read positioned remote nodes
RadarRenderer->>RadarRenderer: filter, sort, scale, and plot nodes
RadarRenderer->>OLEDDisplay: draw radar, list, labels, and icon
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/graphics/draw/RadarRenderer.cpp (1)
194-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffIcon-drawing logic duplicated from
SharedUIDisplay::drawCommonFooter.The comment already acknowledges this replicates half of
drawCommonFooter's behavior. Since both copies must stay in sync (icon glyph format, scale handling, XBM bit layout), consider extracting a shareddrawConnectionIcon(display, x, y, scale)helper inSharedUIDisplaythat both call, so future icon-format changes don't need to be applied twice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graphics/draw/RadarRenderer.cpp` around lines 194 - 229, The connection icon drawing logic in drawConnectionIconNoWipe is duplicated from SharedUIDisplay::drawCommonFooter, so extract the shared icon-rendering behavior into a reusable helper such as drawConnectionIcon(display, x, y, scale) and have both drawCommonFooter and drawConnectionIconNoWipe call it. Keep the helper responsible for the icon glyph, resolution scaling, and XBM bit layout so changes stay synchronized in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/graphics/draw/RadarRenderer.cpp`:
- Around line 285-293: The early return in RadarRenderer::drawRadar skips the
connection icon, causing it to disappear in the “No GPS fix” state. Update this
branch to preserve the same UI behavior as the other exits by calling
drawConnectionIconNoWipe(display) before returning, matching the rest of the
function’s icon-preservation flow.
- Around line 298-311: The heading fallback in RadarRenderer::draw is using
screen->estimatedHeading() directly as if it were radians, but it returns
degrees and can also yield the -1.0f sentinel when unavailable. Update the
headingRad selection so the GPS fallback branch converts estimatedHeading(myLat,
myLon) with DEG_TO_RAD, and preserve the north-up fallback when no real heading
is available; keep the change localized to the heading/headingUp logic in
RadarRenderer.cpp.
---
Nitpick comments:
In `@src/graphics/draw/RadarRenderer.cpp`:
- Around line 194-229: The connection icon drawing logic in
drawConnectionIconNoWipe is duplicated from SharedUIDisplay::drawCommonFooter,
so extract the shared icon-rendering behavior into a reusable helper such as
drawConnectionIcon(display, x, y, scale) and have both drawCommonFooter and
drawConnectionIconNoWipe call it. Keep the helper responsible for the icon
glyph, resolution scaling, and XBM bit layout so changes stay synchronized in
one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c08e9e0-dff6-475a-a602-b2c64eabe4d1
⛔ Files ignored due to path filters (1)
src/mesh/generated/meshtastic/device_ui.pb.his excluded by!**/generated/**
📒 Files selected for processing (9)
src/graphics/Screen.cppsrc/graphics/draw/MenuHandler.cppsrc/graphics/draw/MenuHandler.hsrc/graphics/draw/NodeListRenderer.cppsrc/graphics/draw/RadarRenderer.cppsrc/graphics/draw/RadarRenderer.hsrc/graphics/draw/UIRenderer.cppsrc/mesh/NodeDB.cppsrc/mesh/NodeDB.h
💤 Files with no reviewable changes (1)
- src/mesh/NodeDB.cpp
- Preserve connection icon in 'No GPS fix' early return (was skipped, causing the icon to disappear until a fix was acquired). - Convert GPS estimatedHeading() fallback from degrees to radians and honor its -1.0f sentinel, falling back to north-up until a real heading exists (matches CompassRenderer).
|
Sorry for the ping again, @Xaositek could you trigger CI again? I got some feedback from CodeRabbit and fixed them. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/graphics/draw/NodeListRenderer.cpp (1)
829-835: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReset the list-mode timer when leaving radar mode.
This early return skips
modeStartTimeupdates. After radar mode is disabled, the stale timestamp can immediately advance the restored view from distance to bearing (or vice versa).Proposed fix
if (uiconfig.bearings_view_radar) { + modeStartTime = now; graphics::RadarRenderer::drawRadarOverlay(display, x, y); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graphics/draw/NodeListRenderer.cpp` around lines 829 - 835, Reset modeStartTime before the early return in the bearings_view_radar branch of NodeListRenderer so leaving radar mode starts the restored list view with a fresh timer. Keep the existing RadarRenderer::drawRadarOverlay call and return behavior unchanged.
🧹 Nitpick comments (1)
src/graphics/draw/NodeListRenderer.cpp (1)
825-832: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the radar-mode comments.
This seven-line block exceeds the project’s one- or two-line comment limit and partly restates the code. Keep only the non-obvious rationale.
- // === Radar overlay mode === - // When bearings_view_radar is enabled (toggled via long-press menu → - // Tracking View), the bearings/distance frame is replaced by the - // circular radar minimap. + // Radar replaces the distance/bearings render path. ... - // RadarRenderer draws its own BT/API icon at the end of the overlay - // (without the full-width black wipe drawCommonFooter performs), so - // the radar arc and last list row stay intact when BT is connected. + // RadarRenderer owns the BT/API icon to avoid wiping radar content.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graphics/draw/NodeListRenderer.cpp` around lines 825 - 832, Condense the comments above the uiconfig.bearings_view_radar check to one or two lines, retaining only the non-obvious rationale about RadarRenderer’s icon handling and avoiding repetition of the setting, menu path, or overlay behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/graphics/draw/NodeListRenderer.cpp`:
- Around line 829-835: Reset modeStartTime before the early return in the
bearings_view_radar branch of NodeListRenderer so leaving radar mode starts the
restored list view with a fresh timer. Keep the existing
RadarRenderer::drawRadarOverlay call and return behavior unchanged.
---
Nitpick comments:
In `@src/graphics/draw/NodeListRenderer.cpp`:
- Around line 825-832: Condense the comments above the
uiconfig.bearings_view_radar check to one or two lines, retaining only the
non-obvious rationale about RadarRenderer’s icon handling and avoiding
repetition of the setting, menu path, or overlay behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ff0ff97f-e06b-4c9d-9033-c59049779278
📒 Files selected for processing (6)
src/graphics/Screen.cppsrc/graphics/draw/MenuHandler.cppsrc/graphics/draw/NodeListRenderer.cppsrc/graphics/draw/UIRenderer.cppsrc/mesh/NodeDB.cppsrc/mesh/NodeDB.h
🚧 Files skipped from review as they are similar to previous changes (5)
- src/graphics/draw/UIRenderer.cpp
- src/mesh/NodeDB.h
- src/graphics/Screen.cpp
- src/mesh/NodeDB.cpp
- src/graphics/draw/MenuHandler.cpp
Replace the raw-pointer redeclaration (extern graphics::Screen *screen) with an include of main.h, which declares the real std::unique_ptr<graphics::Screen> screen. The raw redeclaration compiled only because main.h was not visible in this TU; it was a latent conflicting-declaration bug of the same kind fixed in MPU9250Sensor. screen->hasHeading()/getHeading()/estimatedHeading() work unchanged.
|
Hey @Xaositek @fifieldt @caveman99 @HarukiToreda could you please trigger CI? |
98ef7bc to
75c5849
Compare
|
Hey @Xaositek @fifieldt @caveman99 @HarukiToreda FYI. This PR meshtastic/protobufs#935 need to be merged before this. |
|
we don't merge PRs ourselves, we can only approve it, which I believe all have by now, looks good. Merging decision is usually done by @thebentern and other admin maintainers. I do believe the need for Protos is what has delayed this decision, even my PRs get delayed when protos are involved. |
|
Ok. Thank you @HarukiToreda for explaining this. Could you take a look on Protos PR too? =) |
Conflict: src/mesh/NodeDB.h. Adjacent independent additions - this branch hoisted NUM_ONLINE_SECS from NodeDB.cpp into the header (so RadarRenderer can use it); develop added the SINCE_UNKNOWN sentinel in the same spot. Kept both. NodeDB.cpp auto-merged with the old local #define removed, so there is no duplicate definition.
6 participants
What this PR does
Adds a radar minimap view as an alternative rendering of the existing Distance/Bearings frame — no new screen slot in the rotation.
From the node-list long-press menu, a new "Tracking View" picker (clock-face style) lets the user switch between the regular Bearings list and the Radar overlay. While in radar mode, long-pressing opens a radar-specific menu (Tracking View, N-UP/HDG-UP toggle, Favorites Only toggle, Zoom In/Out). Both choices are persistent across reboots (
bearings_view_radarandradar_favorites_onlyinDeviceUIConfig, proto fields 20 and 21).Radar layout
Radar 5km/Radar 1.5mi(units followdisplay.units)#ifndef USE_EINKblock inNodeListRenderer.cpp, so the radar code isn't compiled in for e-ink buildsFiles changed
src/graphics/draw/RadarRenderer.cpp/.h— radar overlay renderer; owns its own header so the title can carry the current rangesrc/graphics/draw/NodeListRenderer.cpp— delegates toRadarRenderer::drawRadarOverlaywhenbearings_view_radaris setsrc/graphics/draw/MenuHandler.cpp/.h— addstrackingViewPicker()andradarBearingsMenu(); wires "Tracking View" into the node-list menusrc/graphics/Screen.cpp— routes long-press on the bearings frame toradarBearingsMenuwhen radar is activesrc/mesh/generated/meshtastic/device_ui.pb.h— addsbool bearings_view_radar(field 20) andbool radar_favorites_only(field 21), both backwards-compatible🤝 Attestations
Note: Happy to get help testing on other hardware from the community.
Video: https://photos.app.goo.gl/Tj3y2PKypFPwczE58
Summary by CodeRabbit