Skip to content
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.

Sign up for GitHub

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

Jump to bottom

OpenAir extended format: use AY indication for colouring #1349

Merged
merged 7 commits into from
May 30, 2024
Merged

OpenAir extended format: use AY indication for colouring #1349

merged 7 commits into from
May 30, 2024
+333 −40

Conversation

ubx
Copy link
Contributor

@ubx ubx commented Feb 15, 2024

The PR implements airspace coloring, if present and allowed, according to the AY-tag.

Described in #1340

@ubx ubx requested a review from a team as a code owner February 15, 2024 10:16
@lordfolken lordfolken linked an issue Feb 15, 2024 that may be closed by this pull request
OpenAir extended format: use AY indication for colouring with AC UNCLASSIFIED #1340
Closed
@llauner llauner mentioned this pull request Feb 16, 2024
Closed
@lordfolken lordfolken added this to the v.7.42 milestone Feb 18, 2024
@lordfolken
Copy link
Contributor

The map elements dialog does not show the type in case of class uncasslified with valid type.
image

@lordfolken
Copy link
Contributor

The Airspace Details dialog, just shows empty for unclassified. While this is ok, it should be consistent across all dialogs
image

@lordfolken
Copy link
Contributor

Restircted should be red according to the coloring information. Yet it remains magenta.
image

image

lordfolken
lordfolken requested changes Feb 22, 2024
View reviewed changes
Copy link
Contributor

@lordfolken lordfolken left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the work. Please see my comments.

src/Renderer/AirspaceRendererSettings.cpp Outdated Show resolved Hide resolved
src/Renderer/AirspaceRendererSettings.cpp Outdated Show resolved Hide resolved
@ubx
Copy link
Contributor Author

ubx commented Feb 26, 2024 •
edited

The Airspace Details dialog, just shows empty for unclassified. While this is ok, it should be consistent across all dialogs

UNCLASSIFIED class/type is now displayed as Unclassified

@ubx ubx force-pushed the colouring-with-AC-UNCLASSIFIED branch 2 times, most recently from da67148 to faebf34 Compare February 26, 2024 13:23
MaxKellermann
MaxKellermann reviewed Feb 26, 2024
View reviewed changes
src/Airspace/AirspaceParser.cpp Outdated
@@ -179,7 +183,7 @@ struct TempAirspace
name.clear();
radio_frequency = RadioFrequency::Null();
asclass = OTHER;
astype.clear();
astype = OTHER;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why OTHER?

@MaxKellermann MaxKellermann added the needs-work this closed pull request requires some action to be merged label Feb 27, 2024
@lordfolken lordfolken modified the milestones: v.7.42, v7.43 Mar 10, 2024
@ubx ubx force-pushed the colouring-with-AC-UNCLASSIFIED branch from faebf34 to 8b5c67e Compare March 11, 2024 09:14
@ubx
Copy link
Contributor Author

ubx commented Mar 11, 2024

Changes have been made according to the notes.

I am not satisfied with this pull request. It is not based on a specification, but rather on what has been implemented here and there. However, it is now here.

@lordfolken
Copy link
Contributor

Max has stopped working on xcsoar.

Ill compile and review it.

@ubx ubx force-pushed the colouring-with-AC-UNCLASSIFIED branch from 8b5c67e to 75d172f Compare May 22, 2024 14:47
@lordfolken lordfolken removed the needs-work this closed pull request requires some action to be merged label May 22, 2024
@ubx ubx force-pushed the colouring-with-AC-UNCLASSIFIED branch from b0d195c to 7437435 Compare May 27, 2024 16:18
@ubx ubx force-pushed the colouring-with-AC-UNCLASSIFIED branch from 7437435 to b0cfe13 Compare May 28, 2024 01:05
@lordfolken
Copy link
Contributor

Restricted airspaces don´t seem to work.

image

AC UNCLASSIFIED
AY RESTRICTED
AN LSR3 SPEER 124.700 126.350
AI 641728500f96e34b4791e206
AL FL100
AH FL130
DP 47:02:53.0000 N 009:04:10.0000 E
DP 47:02:57.0000 N 009:28:37.0000 E
DP 47:03:55.0000 N 009:28:21.0000 E
DP 47:04:36.0000 N 009:29:49.0000 E
DP 47:05:14.0000 N 009:30:55.0000 E
DP 47:06:16.0000 N 009:31:7.0000 E
DP 47:06:54.0000 N 009:30:59.0000 E

@ubx
Copy link
Contributor Author

ubx commented May 28, 2024

Restricted airspaces don´t seem to work.

Are you sure ?
With airspace file CH-ASP-National-OpenAIP.txt
Screenshot_20240528_100021

@lordfolken
Copy link
Contributor

@ubx Argh. I'm sorry, correct screenshot but wrong text.

What doesn't work is danger areas. They are parsed as Unclassified/Unknown.

AC UNCLASSIFIED
AY DANGER
AN LSD5 ERISWIL
AI 641728500f96e34b4791e217
AL 3000ft MSL
AH 5000ft MSL
DP 47:05:27.0000 N 008:04:47.0000 E
DP 47:06:55.0000 N 007:58:47.0000 E
DP 47:08:8.0000 N 007:53:47.0000 E
DP 47:07:35.0000 N 007:47:47.0000 E
DP 47:06:59.0000 N 007:41:17.0000 E
DP 47:03:35.0000 N 007:38:25.0000 E
DP 46:56:17.0000 N 007:46:56.0000 E
DP 46:56:38.0000 N 007:52:56.0000 E
DP 46:57:7.0000 N 008:01:18.0000 E
DP 47:05:27.0000 N 008:04:47.0000 E

Parses as Unclassifed/Unkown.

The way i see it, is that DANGER is missing from airspace_class_strings[]

lordfolken
lordfolken reviewed May 28, 2024
View reviewed changes
src/Dialogs/Airspace/AirspaceList.cpp Outdated
@@ -167,7 +167,35 @@ static constexpr StaticEnumChoice type_filter_list[] = {
{ CLASSG, _T("Class G") },
{ MATZ, _T("MATZ") },
{ RMZ, _T("RMZ") },
nullptr
//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this empty comment please?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or give it meaning)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty comment removed.

lordfolken
lordfolken reviewed May 28, 2024
View reviewed changes
src/Airspace/AirspaceParser.cpp
@@ -63,6 +63,34 @@ static constexpr AirspaceClassStringCouple airspace_class_strings[] = {
{ "RMZ", RMZ },
{ "MATZ", MATZ },
{ "GSEC", WAVE },
{ "UNCLASSIFIED", UNCLASSIFIED },
{ "RESTRICTED", RESTRICTED },
{ "TMA", TMA },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DANGER is missing

Copy link
Contributor Author

@ubx ubx May 29, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was already defined here:

XCSoar/src/Airspace/AirspaceParser.cpp

Line 50 in 303cb6e

{ "Q", DANGER },

Rename it to { "DANGER", DANGER } ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, add. That would break classic openair files. https://pyopenair.readthedocs.io/en/latest/openair.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

{ "DANGER", DANGER2 },

lordfolken
lordfolken requested changes May 28, 2024
View reviewed changes
Copy link
Contributor

@lordfolken lordfolken left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid work. Please see the comment concerning danger zones.

What remains to be done, is that in all the dialogs where AirspaceFormatter::GetClass(airspace) is being called, that these get updated and also reflect the type. At least that where the class is unclassified, that the text reflects the airspace type.

src/Dialogs/Airspace/dlgAirspace.cpp
src/Dialogs/Airspace/dlgAirspaceWarnings.cpp
src/Renderer/AirspaceListRenderer.cpp

image
image
image

@ubx ubx force-pushed the colouring-with-AC-UNCLASSIFIED branch from b0cfe13 to 48bdfe1 Compare May 29, 2024 20:58
@ubx ubx force-pushed the colouring-with-AC-UNCLASSIFIED branch 2 times, most recently from f436342 to 02db012 Compare May 30, 2024 07:45
@lordfolken lordfolken merged commit 7a68178 into XCSoar:master May 30, 2024
10 checks passed
@ubx ubx deleted the colouring-with-AC-UNCLASSIFIED branch May 31, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxKellermann MaxKellermann MaxKellermann left review comments

@lordfolken lordfolken lordfolken approved these changes

Assignees
No one assigned
Labels
documentation enhancement pr-provided Provided a pr
Projects
Features
Status: Done
Milestone
v7.43
Development

Successfully merging this pull request may close these issues.

OpenAir extended format: use AY indication for colouring with AC UNCLASSIFIED
3 participants
@ubx @lordfolken @MaxKellermann