Add Label BA FANS 1/A CPDLC Downlink decoder plugin#415
Conversation
New plugin registered in official.ts and MessageDecoder.ts.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 18 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kevinelliott
left a comment
kevinelliott
left a comment
There was a problem hiding this comment.
Summary
Adds Label_BA_CPDLC for FANS-1/A CPDLC downlinks (/GROUND.QUAL.TAILPAYLOADCRC). Registered last in MessageDecoder.ts/official.ts. No companion test file.
Verdict
Comment-only review — the tail/payload split heuristic has a real correctness bug that will mis-decode a wide range of non-US tails. Please address before merge.
Must Fix
-
Tail-split heuristic is broken for hyphenated registrations. The window is
rest.substring(0, Math.min(8, rest.length))and the search is/[G-Zg-z]/(non-hex letters). For real samples this misfires:input current output (tail / payload+CRC) likely correct B-2345A0CAFB-2345/A0CAFB-2345A/0CAFC-FRSE0CAFC-FRS/E0CAFC-FRSE/0CAFD-ABYA0CAFD-ABY/A0CAFD-ABYA/0CAFF-GZNT0CAFF-G/ZNT0CAFF-GZNT/0CAFThe heuristic treats any non-hex letter as a tail terminator, but real tails routinely contain non-hex letters internally (G/Z/N/Y/...). The hyphen also defeats the "≥2 chars" guard for one-letter prefix codes. Two reasonable fixes:
- Anchor on the trailing CRC instead of leading-tail length. The CRC is always exactly 4 hex chars at the very end. Strip those, then everything between
.QUAL.and the CRC istail||payload. Tails are typically 5–7 chars; if the next chunk aftertail||payloadis empty, treat the whole thing as the tail (keep-alive case). If it has structure (length divisible by 2, hex), treat the leading 5–7 as tail. - Use a positive tail regex. ICAO tails follow a small set of country-prefix patterns (
[A-Z]-[A-Z0-9]{3,5},N\d{1,5}[A-Z]{0,2},JA\d{4},B-\d{4}[A-Z]?, etc.). Match-then-fallback is more accurate than non-hex sniffing.
Either way, please add explicit tests for at least one US tail, one hyphenated European tail, one Asian tail (
JA/B-), and the keep-alive (empty payload) case. - Anchor on the trailing CRC instead of leading-tail length. The CRC is always exactly 4 hex chars at the very end. Strip those, then everything between
-
Add a
Label_BA_CPDLC.test.ts. No tests at all today. Please cover:qualifiers(), multiple tail formats per the table above, the malformed/short branch, and a non-matching string.
Should Fix
- Add a
preamblesqualifier. Every observed example begins with/, sopreambles: ['/']lets the dispatcher skip this plugin for non-CPDLC BA traffic and keepsmeetsStateRequirements/short-circuit semantics consistent with siblings. raw.crcis a string ("0CAF") andraw.checksumis typednumber. You're usingcrc, which sidesteps the typed field, but consider also pushingparseInt(crc, 16)intoraw.checksumor document the new field. Same comment as #414.decodeResult.formatted.items.unshift(...)is fine but the malformed branch unshifts onto an empty array — consider.pushfor consistency, since order doesn't matter when there's nothing else there yet.- The malformed-payload branch (
payloadAndCrc.length < 4) silently exposes the truncated payload but still flagsdecoded: true, decodeLevel: 'partial'. Reasonable, but the formatteditemsin that branch never include a CRC field at all — downstream consumers may want a sentinel like{ code: 'CRC', value: '(missing)' }to know it was tried.
Nits
- The doc-comment ASCII art is great, thanks.
decodeResult.raw.direction = 'aircraft-to-ground (downlink)'is duplicated in bothrawandformatted.items; that's fine but the value is constant — aconstwould express the intent better.- The doc says "tail (typ. 6 chars)" but the heuristic also accepts shorter; align the doc with whatever fix you settle on.
Tests
None added. Please add Label_BA_CPDLC.test.ts covering at minimum:
qualifiers()(after adding the/preamble)- Happy path with N-tail (
/USADCXA.DR1.N887QSDEADBEEF0CAF) - Hyphenated tail (
C-FRSE...,D-ABYA...) - Keep-alive / short payload (
/USADCXA.DR1.N887QS0CAF) - Malformed (length < 4 after tail) returns partial
- Non-matching returns
decoded: false
Notes — Registration & Coexistence
- Registered at the tail of
pluginClassesand exported fromofficial.ts. No other plugin claims label BA, so order is irrelevant. - No coexistence concerns.
Thanks @thepacket!
There was a problem hiding this comment.
Pull request overview
Adds a new official decoder plugin for ACARS label BA to parse FANS 1/A CPDLC downlink (aircraft → ground) ARINC 622-style routed messages and surface key metadata (ground address, service qualifier, tail, payload hex, CRC).
Changes:
- Introduces
Label_BA_CPDLCplugin with envelope parsing and payload/CRC extraction. - Exports the new plugin via
lib/plugins/official.ts. - Registers the plugin in
MessageDecoderso BA messages are dispatched to it.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/plugins/official.ts | Exposes the new BA CPDLC plugin from the official plugin barrel. |
| lib/plugins/Label_BA_CPDLC.ts | Implements BA (FANS 1/A CPDLC downlink) decoding and formatting. |
| lib/MessageDecoder.ts | Registers the new plugin so it can be executed for label BA. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Tail/payload split heuristic: tails on label BA can be all-hex | ||
| // (e.g. C-XXXXX) so a strict "find the first non-hex char" approach | ||
| // fails. We scan the leading 8 chars for a non-hex character to mark | ||
| // the end of the tail; if none, default to the first 6 chars. | ||
| let tail = ''; | ||
| let payloadAndCrc = ''; | ||
| const window = rest.substring(0, Math.min(8, rest.length)); | ||
| const nonHexIdx = window.search(/[G-Zg-z]/); | ||
| if (nonHexIdx >= 2) { | ||
| // Tail is rest[0 .. nonHexIdx], payload+CRC follows | ||
| tail = rest.substring(0, nonHexIdx + 1); | ||
| payloadAndCrc = rest.substring(nonHexIdx + 1); | ||
| } else { | ||
| // All-hex window — assume 6-char tail | ||
| tail = rest.substring(0, 6); | ||
| payloadAndCrc = rest.substring(6); | ||
| } |
| { | ||
| type: 'tail', | ||
| code: 'TAIL', | ||
| label: 'Tail', | ||
| value: tail, | ||
| }, |
| decode(message: Message, options: Options = {}): DecodeResult { | ||
| const decodeResult = this.initResult( | ||
| message, | ||
| 'FANS-1/A CPDLC Downlink — Controller–Pilot Data Link Communications', | ||
| ); |
Adds a decoder for FANS 1/A CPDLC downlinks (the companion to label AA uplinks).
Wire format
Surfaces ground address, service qualifier (DR1/CC1/etc.), tail (registration-pattern cascade for all-hex tails), ASN.1 PER payload hex, and trailing 4-char CRC. Short payloads (≤8 hex) flagged as likely contract/keep-alive messages.
npm run buildpasses.