Sanitize airport definition files #2114

inferiorhumanorgans · 2023-10-15T03:33:29Z

As part of another project I wrote a parser/linter for the airport definition files and ran it on everything in the repository. The resulting changes are listed in the commit message, roughly:

Fixing invalid routes (e.g. speed restriction of S21O instead of S210)
Filling in some missing but easy to find data (e.g. field elevation)
Ensuring numeric values are numbers and not strings (unless units need to be specified)
Cleaning up the coordinates, specifically if minutes are specified with a decimal component, don't allow the seconds to be specified (dNN.NNm0 was used haphazardly) – this makes parsing a bit simpler and more predictable
Removed the 'empty' maps where the shape wouldn't have parsed in the first place
Reshaped old maps into the format the current code uses (array vs dict)
Rewrote the elevation parser and enforced a space between value and units
Removed name_offset as it's not referenced in code anywhere
Updated the documentation

Known issues:

Didn't update as I couldn't find info: SBGR and OSDI
Map line segments are invalid: EGNM

One potentially intrusive change I've made is to change missing values to null instead of empty strings for arrival and departure routes. This is accompanied by checking for null in a few places with the ? operator. This makes parsing easier and semantically seems to make more sense to me.

All the tests appear to pass.

With altitude and speed fields, use null to represent missing values instead of overloading an empty string.

EGKK * Ensure speed restrictions are prefixed with S and not K * Ensure altitude restrictions are specced in 100s of feet EKCH * Add missing `has_terrain` field EDDF * Remove array around simple instruction in TOBA9S SID KCLT Cleanup: * Remove array around simple instruction in BTSEY2 STAR * Replace capital O with zero in MAJIC4 STAR KEWR * Spawn defs use integer literals for altitude and speed not strings KPHL * Spawn defs use integer literals for altitude and speed not strings KOKC * Spawn defs use integer literals for altitude and speed not strings KMEM * Spawn defs use integer literals for altitude and speed not strings KGSO * Spawn defs use integer literals for altitude and speed not strings KSAT * Spawn defs use integer literals for altitude and speed not strings LKPR * Spawn defs use integer literals for altitude and speed not strings WIMM * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format * Add runway elevation ZSPD * Add airport elevation * Add runway elevation KDFW * Clean up routing on BRDJE3 STAR VHHH * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format WMKK * Add field elevation * Add runway elevation WIII * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format * Add field elevation * Add runway elevation VOBL * Add field elevation * Add runway elevation VABB * Add field elevation * Add runway elevation TJSJ * Cleanup routing on BEANO3 and JOSHE3 STARs SBGL * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format SAWH * Add runway elevation RKSI * Add field elevation * Add 16R/34L * Add runway elevation Note: SIDs and STARs should be updated as well. RJTT * Add field elevation * Add runway elevation RJBB * Add field elevation RJAA * Add field elevation * Add runway elevation * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format PHNL * Add units to R-3110B restricted airspace LOWW * Clean up routing on ARSIN1AR and UNGUT2C SIDs GCRR * Add field elevation * Add runway elevation EDDT * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format EGLC * Clean up routing on CPT and BPK SIDs KSEA * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format KSAN * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format * ° → d * Add runway elevation KORD * Add runway elevation KMSP * Add field elevation * Add runway elevation KMIA * Add field elevation * Add runway elevation KJFK * Remove trailing "m0" from coordinates. The minutes were already specified in floating point format KIAD * Ensure airlines have a numeric factor in spawn entries

n8rzz

I have questions about how the app will behave with some of these properties moving to null instead of an empty string. I think it will be okay, but we tend to lean on strict equals a lot and null is not ''.

Any chance you could add a few tests to support some of the spots I called out, or just verify those spots are already covered by tests, then I think we'll be good to go!

Thanks for the effort and your contribution on this! I'd be interested to see the external tool you mention. At one time, I attempted an airport validator using a js library called tcomb. It kinda worked but I wasn't very happy with it. I'd love to see what someone else put together!

src/assets/scripts/client/aircraft/AircraftModel.js

n8rzz · 2023-10-17T01:28:44Z

src/assets/scripts/client/aircraft/AircraftModel.js

@@ -707,8 +707,8 @@ export default class AircraftModel {
            icaoWithWeightClass: this.model.icaoWithWeightClass,
            assignedAltitude,
            flightPlanAltitude,
-            arrivalAirportId: this.destination.toUpperCase(),
-            departureAirportId: this.origin.toUpperCase(),
+            arrivalAirportId: this.destination?.toUpperCase(),


How do things behave when destination is null?

n8rzz · 2023-10-17T01:28:54Z

src/assets/scripts/client/aircraft/AircraftModel.js

-            arrivalAirportId: this.destination.toUpperCase(),
-            departureAirportId: this.origin.toUpperCase(),
+            arrivalAirportId: this.destination?.toUpperCase(),
+            departureAirportId: this.origin?.toUpperCase(),


How do things behave when origin is null?

The ? operator only calls the following function if the value is not null or undefined, so the code is equivalent to e.g.:

this.origin ? this.origin.toUpperCase() : null

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

Yes, I know. I'm asking because many places in the app use strict === equals and this property was formerly a string. An "invalid" or "empty" state before would have been ''. Now it's null. Using strict equals, what was once true is no longer.

All I'm asking is: when origin or destination are [now] null, does everything behave the same way? And, if so, can we verify that with a test or two?

Sure, I'll take a look over the weekend.

I've changed the only place that was comparing the origin and/or destination fields to an empty string in b4fb948, and corrected the defaults to null. I'll take a look at the test coverage later.

In terms of test coverage creating objects with default (empty string, now null) origin and/or destination is covered both by some of the mocks and some explicit checks for behavior e.g. "_initializeAirportsAndRunways() does not make calls to initialize departure airport+runway when origin ICAO is null". I updated this in 23dc216 and at first blush I think that should manage expectations within the code appropriately.

Awesome! Thanks for looking into that, I appreciate it.

I actually caught one more instance of empty strings in the spawn patterns that I missed. I'll push that tomorrow.

src/assets/scripts/client/aircraft/FlightManagementSystem/Fms.js

inferiorhumanorgans · 2023-10-17T18:01:38Z

https://git.sr.ht/~az1/openscope-data

So it's mostly intended as a parser (and it's still under development), but adding proper validation would be pretty easy.

n8rzz · 2023-10-18T15:46:40Z

https://git.sr.ht/~az1/openscope-data

So it's mostly intended as a parser (and it's still under development), but adding proper validation would be pretty easy.

That's pretty freaking cool! Nice work!!

e.g. 100ft vs 100 ft This makes parsing in Rust easier and matches generally accepted SI conventions.

Remove empty map definition that wouldn't parse properly: EBBR, EDDT, EGKK, EGLC, EGLL, EKCH, GCRR, VHHH, Reshape map dict → array to match the current code: KDFW, KDTW, KMSP, KPHX, KPVD, KSDF, KTPA, KTUS, MDSD, PANC Repair coordinates: LTBA

As far as I can tell `name_offset` isn't referenced or used anywhere.

inferiorhumanorgans · 2023-10-24T21:49:51Z

https://git.sr.ht/~az1/openscope-data
So it's mostly intended as a parser (and it's still under development), but adding proper validation would be pretty easy.

That's pretty freaking cool! Nice work!!

Welp it's intended first and foremost as a parser so as a linter it's not great. Namely it bails on the first error. Certainly it could be massaged to look like a proper linter, but that's not something I'm terribly focused on right now.

Meanwhile I've snuck in a few more changes:

nuked the 'empty' maps where the shape wouldn't have parsed in the first place
reshaped old maps into the format the current code uses
rewrote the elevation parser and enforced a space between value and units
removed name_offset as it's not referenced in code anywhere (I've not looked at the label drawing code to see if it should be smarter than it is)
updated the documentation

At this point nothing should be generating empty strings for these fields.

Alex Zepeda added 3 commits October 14, 2023 00:19

SpawnPattern: Missing values → null

e0f89ee

With altitude and speed fields, use null to represent missing values instead of overloading an empty string.

Client: handle null instead of empty strings

ec31976

n8rzz approved these changes Oct 17, 2023

View reviewed changes

Alex Zepeda added 6 commits October 20, 2023 16:41

Enforce a space between value and units

877a23d

e.g. 100ft vs 100 ft This makes parsing in Rust easier and matches generally accepted SI conventions.

Remove trailing m0 from mock coordinates and docs

e95efee

Cleanup airport map definitions

9992ebe

Remove empty map definition that wouldn't parse properly: EBBR, EDDT, EGKK, EGLC, EGLL, EKCH, GCRR, VHHH, Reshape map dict → array to match the current code: KDFW, KDTW, KMSP, KPHX, KPVD, KSDF, KTPA, KTUS, MDSD, PANC Repair coordinates: LTBA

Update airport docs to reflect current reality

1f2303d

Remove name_offset from runway definitions

f1ccea5

As far as I can tell `name_offset` isn't referenced or used anywhere.

Fix typo.

1c31c57

Alex Zepeda added 4 commits October 25, 2023 13:15

AircraftModel: Init origin and destination w/ null

5915e5c

At this point nothing should be generating empty strings for these fields.

AircraftModel: check against null in isOverflight

c65838b

Tests and mocks now use null origin/destination

23dc216

SpawnPatternModel: use null origin/destination

a6c7845

inferiorhumanorgans force-pushed the iho-sanitize-airports branch from b4fb948 to a6c7845 Compare October 25, 2023 23:32

Alex Zepeda added 3 commits October 29, 2023 15:07

Update forgotten SpawnPattern origins "" → null

82722b5

Update forgotten SpawnPattern mocks "" → null

800e0f2

UnitConverters: make lint-diff happy

7dcc5f0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sanitize airport definition files #2114

Sanitize airport definition files #2114

inferiorhumanorgans commented Oct 15, 2023 •

edited

n8rzz left a comment

n8rzz Oct 17, 2023

n8rzz Oct 17, 2023

inferiorhumanorgans Oct 17, 2023 •

edited

n8rzz Oct 18, 2023

inferiorhumanorgans Oct 20, 2023

inferiorhumanorgans Oct 24, 2023

inferiorhumanorgans Oct 26, 2023

n8rzz Oct 29, 2023

inferiorhumanorgans Oct 29, 2023

inferiorhumanorgans commented Oct 17, 2023

n8rzz commented Oct 18, 2023

inferiorhumanorgans commented Oct 24, 2023

Sanitize airport definition files #2114

Are you sure you want to change the base?

Sanitize airport definition files #2114

Conversation

inferiorhumanorgans commented Oct 15, 2023 • edited

n8rzz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inferiorhumanorgans Oct 17, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inferiorhumanorgans commented Oct 17, 2023

n8rzz commented Oct 18, 2023

inferiorhumanorgans commented Oct 24, 2023

inferiorhumanorgans commented Oct 15, 2023 •

edited

inferiorhumanorgans Oct 17, 2023 •

edited