pull up c-band prefix decoding #174
Conversation
|
|
||
| // https://app.airframes.io/messages/3452310240 | ||
| const text = '101606,1910,.N317FR,,KMDW,----,1,1016,RT0,LT1,'; | ||
| const decodeResult = decoderPlugin.decode({ text: text }); | ||
| const decodeResult = decoder.decode({ label: "4A", text: text }); |
There was a problem hiding this comment.
this isn't a bad idea, but it's going to make the PR huge. Do we want a separate PR that just makes this change? Or do we want to move the c-band messages to the MessageDecoder suite? I'm leaning towards moving the c-band tests
There was a problem hiding this comment.
Moving the c band message tests makes sense with two drawbacks:
- Need to remember to add tests in a second location for types with this prefix
- More importantly, there's no way to check if the cband prefix accidentally catches non cband messages and incorrectly truncates the message that it passes into a plugin.
There was a problem hiding this comment.
do we have messages (minus the prefix) that are unique to c-band? If not, i don't think we need to have c-band label-specific tests other than a couple to test the prefix extraction.
As for the second point, idk. Is it worth slowing down the test suite by making every test run through the plugin selection code just so we can verify this edge case that we don't even know exists?
There was a problem hiding this comment.
not sure about any cband specific messages, but the regex for the prefix is pretty permissive. i would be surprised if it never matched any non cband messages. maybe it would be better to send more metadata about the source of the message into the decoders and just tell it explicitly if it's cband?
or we could try to decode as cband if the prefix is detected then retry with the prefix still attached if it fails to decode. that wouldn't necessarily catch every edge case though
There was a problem hiding this comment.
@kevinelliott and @andermatt64 - Do you have any opinions here?
lib/MessageDecoder.ts
Outdated
| // C-Band puts a 10 char header in front of some message types | ||
| // First 4 chars are some kind of message number | ||
| // Last 6 chars are the flight number | ||
| let cband = message.text.match(/^(?<msgno>[A-Z]\d{2}[A-Z])(?<airline>[A-Z]{2})(?<number>[0-9]{4})/); |
There was a problem hiding this comment.
For airline capture group, we might have to be a bit more forgiving in terms of what is allowed since the following are valid IATA callsigns: EK05RQ, B60001, 9E0001, MCEY42
There was a problem hiding this comment.
seems like there's a possible full format described here: https://en.wikipedia.org/wiki/Airline_codes#IATA_airline_designator
It makes sense to let the airline be letters or digits, but the C-Band prefix is a fixed 10 characters as far as I've seen, and it pads out the numeric part to 4 digits, so there's no room for the third airline designator character or optional operational suffix. I could change it anyway if you like but I think it makes sense to try and keep it as tight as possible to try to avoid false matches.
rpatel3001
force-pushed
the
cband_prefix
branch
from
beabfab
to
84e959e
Compare
rpatel3001
force-pushed
the
cband_prefix
branch
from
84e959e
to
cf15587
Compare
doing it this way means using top level MessageDecoder instance in all the tests instead of an instance of the specific plugin being tested. if that's ok i can update the rest to match (and check if the C-Band prefix test matches any messages it shouldn't).