Repository navigation
Conversation
📝 WalkthroughWalkthroughVersion bump to 0.2.0 introducing dynamic feed URL templating with coordinate/range placeholders, radar zoom/pan/directional navigation with runtime feed updates, quit confirmation modal, improved selected-aircraft rendering, refactored config/fetcher wiring for runtime URL updates, and UI enhancements including scrollable help and theme adjustments. Changes
Sequence DiagramsequenceDiagram
participant User
participant Runtime
participant App
participant Config
participant FeedUpdateCh as Feed Update<br/>Channel
participant Fetcher
participant Sources
User->>Runtime: Press SHIFT+Arrow (Pan)<br/>or +/- (Zoom)
Runtime->>App: pan_radar() or zoom_radar()
App->>App: Update radar_center
App->>Config: render_url_templates(...,<br/>new_lat, new_lon, new_range)
Config-->>App: Vec<String> (rendered URLs)
App->>App: refresh_feed_urls()
App->>FeedUpdateCh: Send new URL list
Fetcher->>FeedUpdateCh: Receive update
Fetcher->>Sources: build_sources(new URLs)
Sources-->>Fetcher: Updated source list
Fetcher->>Fetcher: Reset backoff, fetch immediately
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 184-186: The zoom keybinding table row ("Zoom radar/feed range")
omits the `=` key; update that table entry to list both `+` and `=` (e.g., "`+`
/ `=` / `-`" or similar) so it matches CHANGELOG.md's advertised `+`/`=` zoom-in
binding and covers layouts where `+` requires Shift. Ensure the table cell text
for the Zoom row includes the `=` symbol alongside `+` to make the unshifted
binding discoverable.
In `@src/app.rs`:
- Around line 1431-1451: The aircraft_centroid_for function computes longitude
by simple averaging which breaks at the antimeridian; replace the raw lon sum
with a circular mean: accumulate sum_sin += sin(lon_rad) and sum_cos +=
cos(lon_rad) for each ac.lon, compute mean_lon_rad = atan2(sum_sin / count,
sum_cos / count), convert back to degrees, and normalize into the expected range
before returning RadarCenter; keep the latitude as the linear average and ensure
downstream callers like ensure_radar_center() and refresh_feed_urls() receive
the corrected RadarCenter.
In `@src/config.rs`:
- Around line 389-390: The new array setting url_templates (and the duplicate at
lines 454-455) can't round-trip because src/app.rs treats non-scalar TOML as
ConfigKind::Str and re-serializes as a quoted string; either add array-aware
handling or exclude the key from the editor: modify the ConfigKind enum (e.g.,
add ConfigKind::Array or ConfigKind::List) and update the loader/serializer in
src/app.rs to detect toml::Value::Array when reading url_templates (and
serialize it back as a TOML array), or alternatively add url_templates (and
url_template if needed) to the editor blacklist so they are not loaded/rewritten
by the editor; adjust code paths that construct/serialize ConfigKind values
accordingly so array values round-trip unchanged.
In `@src/radar.rs`:
- Around line 170-177: The code currently derives a single center
(center_lat/center_lon) from app.radar_center() or app.site() and uses it for
both plotting and metric readouts; change this to compute two distinct centers:
a plot_center (e.g., plot_center_lat/plot_center_lon) used for the radar
projection as currently chosen by app.radar_center() / pan logic, and a
metric_center (e.g., metric_center_lat/metric_center_lon) that is always
anchored to app.site() (fallback to the averaged sum_lat/sum_lon only if
app.site() is None). Keep the existing selection of plot center logic but ensure
metric_center is derived from app.site(), then update the call site of
selected_aircraft() to accept both centers so position stays radar-relative
while RNG/BRG calculations use metric_center.
In `@src/ui.rs`:
- Around line 1073-1077: Update the confirmation dialog text to use formal
language by replacing the colloquial "Are you sure you wanna quit?" in the lines
vector (the Line::from(...) entry) with "Are you sure you want to quit?" so the
dialog displays "Are you sure you want to quit?" while leaving the other
Line::from entries unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 654e6161-a8e7-4f61-8be2-51eee498413f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.github/ISSUE_TEMPLATE/bug-report.yml.github/ISSUE_TEMPLATE/security.ymlCHANGELOG.mdCargo.tomlREADME.mddocs/CONFIGURATION.mddocs/DATA_SOURCES.mdsrc/app.rssrc/config.rssrc/main.rssrc/net.rssrc/radar.rssrc/runtime.rssrc/ui.rs
| | `+` / `-` | Zoom radar/feed range | | ||
| | `Shift` + `↑/↓/←/→` | Pan radar/feed center | | ||
| | `↑/↓/←/→` in radar view | Select nearest aircraft in that direction | |
There was a problem hiding this comment.
List = as a zoom-in binding here too.
CHANGELOG.md Line 13 advertises +/= for zoom-in, but this table only mentions +. On many layouts + requires Shift, so omitting = makes the unshifted binding easy to miss.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 184 - 186, The zoom keybinding table row ("Zoom
radar/feed range") omits the `=` key; update that table entry to list both `+`
and `=` (e.g., "`+` / `=` / `-`" or similar) so it matches CHANGELOG.md's
advertised `+`/`=` zoom-in binding and covers layouts where `+` requires Shift.
Ensure the table cell text for the Zoom row includes the `=` symbol alongside
`+` to make the unshifted binding discoverable.
| fn aircraft_centroid_for(&self, indices: &[usize]) -> Option<RadarCenter> { | ||
| let mut sum_lat = 0.0; | ||
| let mut sum_lon = 0.0; | ||
| let mut count = 0usize; | ||
| for idx in indices { | ||
| let Some(ac) = self.data.aircraft.get(*idx) else { | ||
| continue; | ||
| }; | ||
| if let (Some(lat), Some(lon)) = (ac.lat, ac.lon) { | ||
| sum_lat += lat; | ||
| sum_lon += lon; | ||
| count += 1; | ||
| } | ||
| } | ||
| if count == 0 { | ||
| None | ||
| } else { | ||
| Some(RadarCenter { | ||
| lat: sum_lat / count as f64, | ||
| lon: sum_lon / count as f64, | ||
| }) |
There was a problem hiding this comment.
Antimeridian centroiding is incorrect.
This averages raw longitudes, so traffic clustered around 179°E and 179°W produces a center near 0°. Once ensure_radar_center() falls back to this centroid, radar navigation and refresh_feed_urls() can jump the viewport/feed request to the wrong side of the world. Use a circular mean for longitude (atan2(mean_sin, mean_cos)) instead of sum_lon / count.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app.rs` around lines 1431 - 1451, The aircraft_centroid_for function
computes longitude by simple averaging which breaks at the antimeridian; replace
the raw lon sum with a circular mean: accumulate sum_sin += sin(lon_rad) and
sum_cos += cos(lon_rad) for each ac.lon, compute mean_lon_rad = atan2(sum_sin /
count, sum_cos / count), convert back to degrees, and normalize into the
expected range before returning RadarCenter; keep the latitude as the linear
average and ensure downstream callers like ensure_radar_center() and
refresh_feed_urls() receive the corrected RadarCenter.
| pub url_template: Option<String>, | ||
| pub url_templates: Vec<String>, |
There was a problem hiding this comment.
url_templates won't round-trip through the config editor.
This adds an array-valued setting, but src/app.rs still loads non-scalar TOML values as ConfigKind::Str and saves Str back as a quoted string. A config containing url_templates = ["..."] will therefore be corrupted after an in-app save. Please add array-aware config kinds/serialization or exclude this key from the editor until that exists.
Also applies to: 454-455
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/config.rs` around lines 389 - 390, The new array setting url_templates
(and the duplicate at lines 454-455) can't round-trip because src/app.rs treats
non-scalar TOML as ConfigKind::Str and re-serializes as a quoted string; either
add array-aware handling or exclude the key from the editor: modify the
ConfigKind enum (e.g., add ConfigKind::Array or ConfigKind::List) and update the
loader/serializer in src/app.rs to detect toml::Value::Array when reading
url_templates (and serialize it back as a TOML array), or alternatively add
url_templates (and url_template if needed) to the editor blacklist so they are
not loaded/rewritten by the editor; adjust code paths that construct/serialize
ConfigKind values accordingly so array values round-trip unchanged.
| pub fn active_url_templates(config: &Config) -> Vec<String> { | ||
| let templates = if config.url_templates.is_empty() { | ||
| config | ||
| .url_template | ||
| .as_ref() | ||
| .map(|template| vec![template.clone()]) | ||
| .unwrap_or_default() | ||
| } else { | ||
| config.url_templates.clone() | ||
| }; | ||
| templates | ||
| .into_iter() | ||
| .map(|template| template.trim().to_string()) | ||
| .filter(|template| !template.is_empty()) | ||
| .collect() | ||
| } | ||
|
|
||
| pub fn initial_fetch_urls(config: &Config) -> Result<Vec<String>> { | ||
| let templates = active_url_templates(config); | ||
| if !templates.is_empty() { | ||
| let (Some(lat), Some(lon)) = (config.site_lat, config.site_lon) else { | ||
| return Err(anyhow!( | ||
| "url_template/url_templates require site_lat and site_lon" | ||
| )); | ||
| }; | ||
| return Ok(render_url_templates( | ||
| &templates, | ||
| lat, | ||
| lon, | ||
| config.radar_range_nm.max(1.0), | ||
| )); | ||
| } | ||
|
|
||
| Ok(static_urls(config)) | ||
| } |
There was a problem hiding this comment.
Template mode currently masks explicit static URL overrides.
initial_fetch_urls() always prefers url_template/url_templates whenever either is set. If a template comes from the config file, later ADSB_URL, ADSB_URLS, or --url values never take effect, so users can't temporarily switch back to a fixed feed without editing the file. This needs an explicit override path or a way for env/CLI inputs to clear template mode.
| let (center_lat, center_lon) = app | ||
| .radar_center() | ||
| .map(|center| (center.lat, center.lon)) | ||
| .or_else(|| app.site().map(|site| (site.lat, site.lon))) | ||
| .unwrap_or(( | ||
| sum_lat / current_points as f64, | ||
| sum_lon / current_points as f64, | ||
| ), | ||
| }; | ||
| )); |
There was a problem hiding this comment.
Keep target-panel range/bearing anchored to the configured site.
This center now prefers app.radar_center(), and those coordinates are later reused by selected_aircraft() for the RNG/BRG lines. After a pan, the selected-target panel will report range/bearing from the viewport center instead of the receiver/home site, which contradicts CHANGELOG.md Line 20. Split plot-center from metric-center here so panning only affects projection, not site-relative readouts.
Possible direction for the fix
- let (center_lat, center_lon) = app
+ let (plot_center_lat, plot_center_lon) = app
.radar_center()
.map(|center| (center.lat, center.lon))
.or_else(|| app.site().map(|site| (site.lat, site.lon)))
.unwrap_or((
sum_lat / current_points as f64,
sum_lon / current_points as f64,
));
+
+ let (metric_center_lat, metric_center_lon) = app
+ .site()
+ .map(|site| (site.lat, site.lon))
+ .unwrap_or((plot_center_lat, plot_center_lon));Then use plot_center_* for radar projection and pass both centers into selected_aircraft() so:
positionstays radar-relativeRNG/BRGstays site-relative
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against the current code and only fix it if needed.
In `@src/radar.rs` around lines 170 - 177, The code currently derives a single
center (center_lat/center_lon) from app.radar_center() or app.site() and uses it
for both plotting and metric readouts; change this to compute two distinct
centers: a plot_center (e.g., plot_center_lat/plot_center_lon) used for the
radar projection as currently chosen by app.radar_center() / pan logic, and a
metric_center (e.g., metric_center_lat/metric_center_lon) that is always
anchored to app.site() (fallback to the averaged sum_lat/sum_lon only if
app.site() is None). Keep the existing selection of plot center logic but ensure
metric_center is derived from app.site(), then update the call site of
selected_aircraft() to accept both centers so position stays radar-relative
while RNG/BRG calculations use metric_center.
```
</details>
<!-- fingerprinting:phantom:medusa:grasshopper:327f2e9c-e4c6-420d-b02f-b487855268b7 -->
<!-- 4e71b3a2 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| let lines = vec![ | ||
| Line::from("Are you sure you wanna quit?"), | ||
| Line::from(""), | ||
| Line::from("Enter/y quit Esc/n cancel"), | ||
| ]; |
There was a problem hiding this comment.
Consider using formal language in the confirmation dialog.
"wanna" is colloquial. Consider "want to" for a more professional tone in the UI.
Suggested fix
let lines = vec![
- Line::from("Are you sure you wanna quit?"),
+ Line::from("Are you sure you want to quit?"),
Line::from(""),
Line::from("Enter/y quit Esc/n cancel"),
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let lines = vec![ | |
| Line::from("Are you sure you wanna quit?"), | |
| Line::from(""), | |
| Line::from("Enter/y quit Esc/n cancel"), | |
| ]; | |
| let lines = vec![ | |
| Line::from("Are you sure you want to quit?"), | |
| Line::from(""), | |
| Line::from("Enter/y quit Esc/n cancel"), | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ui.rs` around lines 1073 - 1077, Update the confirmation dialog text to
use formal language by replacing the colloquial "Are you sure you wanna quit?"
in the lines vector (the Line::from(...) entry) with "Are you sure you want to
quit?" so the dialog displays "Are you sure you want to quit?" while leaving the
other Line::from entries unchanged.
Summary
cargo audit.Local validation
cargo fmt -- --checkpassedcargo clippy --all-targets -- -D warningspassedcargo test --all-targetspassed: 57 unit tests and 4 integration testscargo auditpassed with only allowed advisory warningsgit diff --checkandgit diff --cached --checkpassedGitHub validation
devCI passed for7e884bb94ef348800fb7f01c7c15f1afbe694b17: https://github.com/j4v3l/ADS-B_TUI/actions/runs/25118230743Summary by CodeRabbit
New Features
Bug Fixes
Documentation