-
-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update airspaces, fixes CO #340
Conversation
- Add support for AC = P, W - Add support for DA arcs - Improve coordinates decoding
Reviewer's Guide by SourceryThis pull request updates the airspaces data and fixes a problem with Colombian airspaces. The main changes include updating the coordinate parsing logic to handle decimal seconds, adding support for "DA" arc definitions, refactoring the arc-drawing logic, adding Colombian airspaces from a new source, and filtering out outdated Colombian airspaces from the openaip.net dataset. Sequence diagram for updated airspace processing flowsequenceDiagram
participant Script
participant OpenAIP
participant Colombia
participant Ukraine
participant Parser
participant GeoJSON
Script->>OpenAIP: Load OpenAIP airspaces
Script->>Parser: Parse OpenAIP data
Script->>Script: Process airspaces
Script->>Script: Filter (including CO filter)
Script->>Colombia: Load Colombia airspaces
Script->>Parser: Parse Colombia data
Script->>Script: Process airspaces
Script->>Script: Filter
Script->>Ukraine: Load Ukraine airspaces
Script->>Parser: Parse Ukraine data
Script->>Script: Process airspaces
Script->>Script: Filter
Script->>GeoJSON: Combine all airspaces
Script->>GeoJSON: Convert to GeoJSON
GeoJSON-->>Script: Final GeoJSON output
Class diagram for airspace parsing componentsclassDiagram
class OpenAirParser {
+parseAll(openair: string, country: string): Airspace[]
-decodeCoordinates(coords: string)
-addArc(center, radius, direction, startAngle, endAngle, coords)
-decodeACField(ac: string, country: string)
}
class Airspace {
+polygon: Coordinate[]
+country: string
+icaoClass: Class
+type: Type
+ignoreAirspace: boolean
}
class Direction {
<<enumeration>>
Clockwise
CounterClockwise
}
OpenAirParser ..> Airspace : creates
OpenAirParser ..> Direction : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe pull request introduces comprehensive changes to the Changes
Sequence DiagramsequenceDiagram
participant Build as Build Process
participant Secrets as Secrets Build
participant Tiles as FXC Tiles
participant Parser as OpenAir Parser
participant Airspaces as Airspace Data
Build->>Secrets: Build secrets first
Secrets-->>Build: Secrets built successfully
Build->>Tiles: Build FXC Tiles
Tiles->>Parser: Parse airspace data
Parser->>Airspaces: Process Colombian airspaces
Airspaces-->>Parser: Return processed airspaces
Parser-->>Tiles: Provide parsed airspace data
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @vicb - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/fxc-tiles/src/app/parser/openair.ts (3)
25-26
: Consider clarifying the coordinates format in the comment.
The comment at line 25 references a sexagesimal format with colons, but it is only partially illustrative. Consider providing a clearer quick reference for new contributors.
163-174
: Ensure consistency in error messages.
The error message at line 169 refers to "DB" while we’re within the “DA” case block. Consider synchronizing the error text so it references the correct field (DA
).- throw new Error(`No center for DB ${line}`); + throw new Error(`No center for DA ${line}`);
179-179
: Double-check the “Invalid DB arc” message.
This exception message is helpful but consider adding context about what was expected, such as specifying that the coordinates could not be parsed or that the arc format is incomplete.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/fxc-tiles/README.md
(1 hunks)apps/fxc-tiles/src/app/airspaces/create-geojson.ts
(3 hunks)apps/fxc-tiles/src/app/parser/openair.ts
(4 hunks)apps/fxc-tiles/src/assets/airspaces/colombia.openair
(1 hunks)apps/fxc-tiles/src/assets/airspaces/download.sh
(1 hunks)package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/fxc-tiles/src/assets/airspaces/download.sh
🧰 Additional context used
🪛 Biome (1.9.4)
apps/fxc-tiles/src/app/parser/openair.ts
[error] 235-235: Unexpected constant condition.
(lint/correctness/noConstantCondition)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (20.x)
🔇 Additional comments (10)
apps/fxc-tiles/src/app/parser/openair.ts (3)
184-187
: Arc consistency check.
Lines 184-187 calculate startAngle, endAngle, and radius, then calladdArc
. This logic is correct and improves maintainability by reusing theaddArc
helper. Reviewed and approved.
278-278
: Approve the prohibitions logic.
Extending recognition of both ‘P’ and ‘PROHIBITED’ is consistent and enhances parser flexibility.
286-293
: Validate newly addedW
airspace type.
Support forWarningArea
is consistent with other airspace types. Ensure that any referencing code handlesType.WarningArea
gracefully.✅ Verification successful
Type.WarningArea
implementation verified and consistent
The implementation is complete with proper enum definition, string representation, and OpenAir format parsing, following the same pattern as other airspace types.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Confirm usage of `Type.WarningArea` across the codebase. rg -A 5 'Type\.WarningArea'Length of output: 730
Script:
#!/bin/bash # Find Type enum definition ast-grep --pattern 'enum Type { $$$ }' # Find any UI components or processing logic handling airspace types rg -l 'type.*:.*Type[^.]' --type typescriptLength of output: 2003
apps/fxc-tiles/src/assets/airspaces/colombia.openair (1)
1-1484
: Large data addition for Colombian airspaces.
This file looks to be a comprehensive set of new OpenAir definitions. The altitudes and coordinates appear syntactically correct for OpenAir. No immediate red flags are visible regarding syntax or structure.✅ Verification successful
OpenAir format verification successful
All 109 airspace definitions contain the required elements (class, name, upper/lower limits) with correct syntax. Altitude specifications and coordinates follow standard OpenAir format.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify each "AC" record has a corresponding "AL" and "AH" within the same block. # This is a rudimentary structural check on the file's new lines. rg -A 10 '^AC ' | gawk '/^AC /{block=""} {block = block $0 ORS} /AL / && /AH /{print block; block=""}'Length of output: 104
Script:
#!/bin/bash # Count essential OpenAir records and verify altitude formats echo "=== Record counts ===" echo "AC records: $(rg -c '^AC ' apps/fxc-tiles/src/assets/airspaces/colombia.openair)" echo "AN records: $(rg -c '^AN ' apps/fxc-tiles/src/assets/airspaces/colombia.openair)" echo "AH records: $(rg -c '^AH ' apps/fxc-tiles/src/assets/airspaces/colombia.openair)" echo "AL records: $(rg -c '^AL ' apps/fxc-tiles/src/assets/airspaces/colombia.openair)" echo -e "\n=== Checking altitude formats ===" # Look for any altitude specifications that don't match expected patterns rg '^A[HL] ' apps/fxc-tiles/src/assets/airspaces/colombia.openair | rg -v '(GND|FL\d+|\d+ft AMSL|UNLTD)' echo -e "\n=== Checking coordinate formats ===" # Look for any coordinate lines that don't match expected DD:MM:SS.SS format rg '^DP ' apps/fxc-tiles/src/assets/airspaces/colombia.openair | rg -v '^\DP \d{2}:\d{2}:\d{2}\.\d{2} [NS] \d{3}:\d{2}:\d{2}\.\d{2} [EW]'Length of output: 909
apps/fxc-tiles/src/app/airspaces/create-geojson.ts (4)
41-41
: Order of filters.
The new filtering call tocreateFilterOutColombia
precedes the final logs. Ensure that the order of filters does not hide other logs or complicate debugging.
45-57
: Colombian airspace parsing workflows.
This block cleanly integrates Colombian OpenAir data with the existing pipeline. Good use of distinct log headers aids troubleshooting.
85-85
: Combine airspaces from multiple sources.
AddingcoAirspaces
to the final array is correct. Ensure that the merged set does not inadvertently duplicate airspaces.
112-123
: Intuitive naming:createFilterOutColombia
.
This function adheres to a direct naming scheme, clarifying its intention. Logic is straightforward and meets the PR objectives to exclude stale CO data.apps/fxc-tiles/README.md (1)
7-7
: Build step clarification.
Emphasizingnx build secrets
first is crucial, as missing secrets can break subsequent builds. This clarity helps avoid partial or failed builds.package.json (1)
6-6
: Synchronize the dev script with README instructions.
Ensuringnx build secrets
runs before serving the projects is consistent with the updated readme. Good improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/fxc-tiles/src/app/airspaces/create-geojson.ts (1)
112-123
: LGTM! Well-documented filter function.The function effectively filters out outdated Colombian airspaces from OpenAIP with a clear explanation via the comment link.
Consider extracting the country code to a constant for better maintainability:
+const COLOMBIA_COUNTRY_CODE = 'CO'; function createFilterOutColombia(logs: Map<string, number>) { return (airspace: Airspace, index: number) => { - if (airspace.country.toUpperCase() === 'CO') { + if (airspace.country.toUpperCase() === COLOMBIA_COUNTRY_CODE) { incMapKey(logs, 'CO'); return false; } return true; }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/fxc-tiles/README.md
(1 hunks)apps/fxc-tiles/src/app/airspaces/create-geojson.ts
(3 hunks)apps/fxc-tiles/src/app/parser/openair.ts
(4 hunks)apps/fxc-tiles/src/assets/airspaces/download.sh
(1 hunks)package.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/fxc-tiles/src/assets/airspaces/download.sh
- apps/fxc-tiles/README.md
- package.json
🧰 Additional context used
🪛 Biome (1.9.4)
apps/fxc-tiles/src/app/parser/openair.ts
[error] 235-235: Unexpected constant condition.
(lint/correctness/noConstantCondition)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (20.x)
🔇 Additional comments (8)
apps/fxc-tiles/src/app/airspaces/create-geojson.ts (3)
45-57
: LGTM! Colombia airspaces processing follows established patterns.The implementation maintains consistency with other regions by using the same processing and filtering pipeline.
85-85
: LGTM! Airspaces array correctly updated.The Colombian airspaces are properly integrated into the final array using the spread operator.
41-41
: LGTM! Verify the filtering of Colombian airspaces.The filter is correctly placed after the main filter but before logging, ensuring proper counting of filtered airspaces.
Let's verify the impact:
✅ Verification successful
✅ Colombian airspace filtering verified and correctly implemented
The filter properly removes outdated Colombian airspaces from OpenAIP data while tracking the filtered count, making way for up-to-date XContest data as documented at paltakats.com/airspace-piedechinche.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check how many Colombian airspaces are being filtered out from OpenAIP rg -l "country.*CO" $(fd -t f "openaip.json" -x echo "{}")Length of output: 109
Script:
#!/bin/bash # Check the implementation of createFilterOutColombia and surrounding context rg -A 5 -B 5 "createFilterOutColombia" apps/fxc-tiles/src/app/airspaces/create-geojson.ts # Look for any Colombian airspace related patterns rg -i "colombia.*airspace|airspace.*colombia"Length of output: 1097
apps/fxc-tiles/src/app/parser/openair.ts (5)
Line range hint
25-34
: LGTM! Good addition of sexagesimal coordinate parsing.The implementation correctly handles the new coordinate format while maintaining consistent error handling.
163-174
: LGTM! Robust implementation of DA arc handling.The implementation correctly processes radius and angles according to the OpenAir format specification, with proper error handling and parameter validation.
179-187
: LGTM! Good refactoring of DB arc handling.The code reuse through addArc function and improved error messages enhance maintainability.
219-251
: Improve the loop structure in addArc function.The static analysis tool flags the constant condition within the loop. Consider using a while loop for better clarity.
- for (let i = 0; true; i++) { + let i = 0; + while (true) { const { latitude, longitude } = computeDestinationPoint(center, radius, startAngle); coords.push([longitude, latitude]); startAngle += angleStep; if (direction === Direction.Clockwise && startAngle > endAngle) { break; } if (direction === Direction.CounterClockwise && startAngle < endAngle) { break; } if (i > 100) { throw new Error(`Angle error`); } + i++; }🧰 Tools
🪛 Biome (1.9.4)
[error] 235-235: Unexpected constant condition.
(lint/correctness/noConstantCondition)
278-293
: LGTM! Good addition of airspace type handlers.The new handlers for 'P' (Prohibited) and 'W' (Warning Area) types follow the established pattern and maintain consistency.
Summary by Sourcery
Add support for Colombia airspaces using an external source.
New Features:
Tests:
Summary by CodeRabbit
Release Notes
New Features
Build Process
Parsing Improvements
Script Modifications