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.

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

Various improvements and fixes #70

Merged
merged 12 commits into from
Oct 1, 2024
Merged

Various improvements and fixes #70

merged 12 commits into from
Oct 1, 2024

Conversation

reskume
Copy link
Contributor

@reskume reskume commented Jul 13, 2024

This commit fixes several issues:

  • fixes naming convention for names (prior to that change files contained a mix of names like LF-R12/R12/ R 12/LF R12 ...)
  • phone numbers moved to comments
  • frequencies moved from name to AF tag
  • AG added where applicable
  • FIRs appended to the end of the file

Problem is that it took quite a considerable amount of time to update the file. Recent additions are NOT included! Open for comments/discussion.

This commit fixes several issues:

- fixes naming convention for names (prior to that change files contained a mix of names like LF-R12/R12/ R 12/LF R12 ...)
- phone numbers moved to comments
- frequencies moved from name to AF tag
- AG added where applicable
- FIRs appended to the end of the file

Problem is that it took quite a considerable amount of time to update the file. Recent additions are NOT included!
Copy link
Contributor

@llauner llauner left a comment

Choose a reason for hiding this comment

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

Error found at line 36234: Failed to read line 36234. Unknown syntax.

image

This commit fixes several issues:

- fixes naming convention for names (prior to that change files contained a mix of names like LF-R12/R12/ R 12/LF R12 ...)
- phone numbers moved to comments
- frequencies moved from name to AF tag
- AG added where applicable
- FIRs appended to the end of the file

Problem is that it took quite a considerable amount of time to update the file. Recent additions are NOT included!
@llauner
Copy link
Contributor

llauner commented Jul 16, 2024

Error found at line 3347: Geometry of airspace 'TMA BALE 1' starting on line 3347 is invalid. First and last Position are not equivalent.

Do you think you could run openaip-openair-parser on your change to make sure the file is valid ?
Thanks

@llauner
Copy link
Contributor

llauner commented Jul 17, 2024

I've started looking at the PR. Which reference are you using for the OpenAir Extended format ?
If I look at:
Documentation format OpenAir
or
Documentation format OpenAir + OpenAir Extended - Naviter
The Restricted Area is AY R, and not AY RESTRICTED

Danger is defined as
AY Q and not AY DANGER

I'm not sure it's right to classify a RMZ as G (uncontrolled), when it is controlled this you need to have a Radio contact.

The RMZ Le Versoud turned from temporary to fixed, but it's true that the date wasn't modified in the file. It should be kept, I'm not sure if others were deleted from your file ?

Let me know what you think...

@reskume
Copy link
Contributor Author

reskume commented Jul 17, 2024 via email

@reskume
Copy link
Contributor Author

reskume commented Jul 18, 2024

I pushed an updated version of the file that includes several fixes to geometries that had self-intersections. I also added a JS script to validate the openair text file using the openAIP openair parser. This requires the package.json etc to be able to easily install the required dependencies to the folder.

Regarding the RMZ: we mapped UNCLASSIFIED to G as defined in the official AIP AD. I re-added the previously removed RMZ for Le Havre.

@llauner The configuration for the parser should be adjusted to match the one you are using to test your builds. I simply copied over the one from our README file.

@llauner
Copy link
Contributor

llauner commented Jul 19, 2024

I'll look at iy over the wee-end.
Regarding the parser, I have github action checking out this parser:
https://github.com/openAIP/openaip-openair-parser.git
I think it's better to check out the file rather than copying it locally...isn't that the same parser as the one you're using ?

@reskume
Copy link
Contributor Author

reskume commented Jul 19, 2024

Yes, the parser is the same. But I didn't know the exact configuration that you are using in the Github actions. So I left it on the default from our repository. I added the parser because it adds a convenient way to validate the openAIR file locally when by running the script validate-openairdefined in package.json. Otherwise each contributor would have to download the parser individually and then configure it depending on your config (which isn't documented anywhere I suppose?). If not using the parser, the validation process only takes place when there is a PR. And then issues have to be fixed by "pushing->waiting->reviewing check output->fix->push->wait..." which is quite tedious to do. But feel free to remove it if you think it doesn't fit in the repo.

@llauner
Copy link
Contributor

llauner commented Aug 11, 2024

I was trying to test your file with SeeYou, SeeYou Navigator and XCSoar, and I had an error with XCSoar:
image

image

@reskume
Copy link
Contributor Author

reskume commented Aug 11, 2024

Typo is fixed

@llauner
Copy link
Contributor

llauner commented Aug 11, 2024

Thanks !
I'm not sure about the FIR region. There are several reasons for that:

  • I don't think they're really useful when flying a glider
  • They appear as active straight away and you have to disable them when you start XCSoar for instance.

@reskume
Copy link
Contributor Author

reskume commented Aug 11, 2024

I can't comment on how useful they are in France, but for Germany, they are very useful. Especially when you are around restricted areas that may be deactivated during specific times. Normally, the frequency associated with the airspace is the one that can be called and knows about activation hours. I just noticed that we didn't add additional AF command to them. I can try to find the specific frequency for each FIR - this would add more useful context to it. If this is not possible, I admit that they are of minimal use and can be removed.

@llauner
Copy link
Contributor

llauner commented Aug 11, 2024

I think for France we should remove them as they're not really useful.
Is there any way that you could also put back all of the airspaces that you considered obsolete as we were past the expiration date ? Most of them are actually still active as they have been transformed into permanent area. My home airfield for instance:
RMZ in LFLG - Grenoble Le Versoud
And then I'll go through them and delete the ones that really are obsolete.
Otherwise there are missing airpsaces...

@reskume
Copy link
Contributor Author

reskume commented Aug 11, 2024

I added the missing airspaces back in from the current version. THere are two TMAs that I cannot find in the AIP:

** TMA AGEN Partie 1 (29 NOV 2001) **
** TMA AGEN Partie 2 (29 NOV 2001) **

I suppose they have been removed but they are still in the current version of the file (not in the PR).

@llauner
Copy link
Contributor

llauner commented Sep 23, 2024

Just to inform: this is still in progress, I think I should be able to merge the PR shortly now.
Now that the season is (almost) over, it's now a good time to adopt a better syntax in the file...

@reskume
Copy link
Contributor Author

reskume commented Sep 23, 2024 via email

@llauner llauner merged commit 3dad449 into planeur-net:main Oct 1, 2024
2 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 1, 2024
github-actions bot pushed a commit that referenced this pull request Oct 1, 2024
github-actions bot pushed a commit that referenced this pull request Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants