Add Label A0 AFN (ATS Facilities Notification) decoder plugin#407
Add Label A0 AFN (ATS Facilities Notification) decoder plugin#407thepacket wants to merge 1 commit into
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 31 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 a Label_A0_AFN plugin for FANS 1/A AFN logon downlinks. Handles three observed envelope variants. Registered correctly.
Verdict
Changes requested — useful coverage, but the implementation diverges from project conventions in several places, doesn't use ResultFormatter, has unstable raw keys, and ships without tests.
Must Fix
- No tests. Please add coverage for each of the three documented variants from the PR description, plus a malformed envelope and an empty-body case.
- Plugin bypasses
ResultFormatteralmost entirely. Where helpers exist (tail,groundAddress,arrivalAirport,timestamp) they should be used so downstream consumers get the sameraw.*keys andformatted.itemsshape as every other plugin. Currently you writedecodeResult.raw.tail = …directly, push your owncode:'TAIL'row, etc. — equivalent, but inconsistent. raw.message_timestampis set to aHH:MM:SSZstring. PerRawFields,message_timestampis numeric (seconds since midnight or epoch). This will type-mismatch downstream consumers. UseResultFormatter.timestamp(decodeResult, DateTimeUtils.convertHHMMSSToTod(timeTok))and validatehh<24, mm<60, ss<60first.
Should Fix
- Sequence-detection heuristic is brittle.
aircraft.match(/^([A-Z0-9]+?)(\d{4})$/)strips trailing 4 digits from any aircraft token of length ≥8 — so a tail likeN12340105would have0105peeled off as a "sequence". Variants 2 and 3 have no sequence — relying on length alone to disambiguate is risky. Drive this off the variant context (whether the post-envelope CSV block exists) instead. - Dead code / no-op. Line 76:
const consumed = env[0].length + (seq ? -0 : 0);—+ -0is a no-op and the comment "env[0] already included seq digits" contradicts it. Delete the conditional, just useenv[0].length. Also lines 78–80:const remainder = remainderRaw.startsWith(',') ? remainderRaw : remainderRaw;— the two branches are identical. Remove. - Non-standard raw keys.
application,aircraft_address,message_sequence,reason_code,header_extra,capability_blocks,capability_code,atc_center_icao,ground_facility_full,ground_facility_address,ground_facility_versionare bespoke. Consider namespacing underafn: { … }so these don't collide with future plugins, and document them inRawFieldsif they’re intended to be public. raw.arrival_icaois set to the ATC centre ICAO (line 138). The ATC centre is not the flight’s arrival airport. Drop this — it pollutes the standard arrival_icao slot.- Ground-facility "version" parser (
/^([A-Z0-9]+?)(F\d{2,4})$/) over-fits a single observed example (LPAFAYAF015). Without source documentation this should be exposed verbatim, not synthesised. - Capability-block multi-shape rendering is asymmetric: when there is one block you emit individual rows; when there are multiple you collapse to
BLK1/BLK2with·-joined values. That makes consumers branch on count. Pick one shape (recommend always per-token rows with an index suffix). - HHMMSS validation.
if (timeTok && /^\d{6}$/.test(timeTok))accepts999999. Validate ranges.
Nits
appl === 'AFN' ? 'AFN (ATS Facilities Notification)' : appl— given this plugin'squalifiers().labels = ['A0']and the regex anchorsAFNsemantically, you can probably hard-fail on non-AFNapplications.decodeResult.raw.tailis set as an assignment, then later rendered viaformatted.items.push({code:'TAIL'…})rather than viaResultFormatter.tail(...).- The leading
let { ground, appl, aircraft } = env.groups;mutatesaircraft. Consider naming the originalaircraftRawfor clarity.
Tests
Missing. See Must Fix.
Notes
- Registration LGTM (MessageDecoder pluginClasses + official.ts re-export).
- CodeRabbit rate-limited; that comment can be ignored.
Thanks @thepacket!
There was a problem hiding this comment.
Pull request overview
Adds support for decoding FANS 1/A AFN (ATS Facilities Notification / CPDLC logon context) downlink messages carried under ACARS label A0, integrating it into the official plugin set and decoder plugin registry.
Changes:
- Introduces a new
Label_A0_AFNdecoder plugin that parses AFN envelope/header fields and capability blocks across multiple observed variants. - Exports the new plugin from the official plugin barrel.
- Registers the new plugin in
MessageDecoderso it participates in decoding.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/plugins/official.ts | Exports the new Label_A0_AFN plugin. |
| lib/plugins/Label_A0_AFN.ts | Implements the AFN (label A0) parsing/formatting logic. |
| lib/MessageDecoder.ts | Adds Label_A0_AFN to the ordered plugin registry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (timeTok && /^\d{6}$/.test(timeTok)) { | ||
| decodeResult.raw.message_timestamp = `${timeTok.substring(0, 2)}:${timeTok.substring(2, 4)}:${timeTok.substring(4, 6)}Z`; | ||
| } else if (timeTok) { | ||
| decodeResult.raw.message_timestamp = timeTok; | ||
| } |
| // If second token looks like a 4-char ICAO, treat it as the ATC center | ||
| if (first.tokens[0] && /^[A-Z]{4}$/.test(first.tokens[0])) { | ||
| decodeResult.raw.atc_center_icao = first.tokens[0]; | ||
| decodeResult.raw.arrival_icao = first.tokens[0]; |
| // Remainder after the envelope (+ seq digits if we peeled them off) | ||
| const consumed = env[0].length + (seq ? -0 : 0); // env[0] already included seq digits | ||
| const remainderRaw = text.substring(consumed); | ||
| const remainder = remainderRaw.startsWith(',') | ||
| ? remainderRaw | ||
| : remainderRaw; // may or may not start with comma |
| decode(message: Message, options: Options = {}): DecodeResult { | ||
| const decodeResult = this.initResult( | ||
| message, | ||
| 'AFN — ATS Facilities Notification (FANS 1/A data-link logon)', | ||
| ); |
Adds a decoder for FANS 1/A AFN (Context Management / CPDLC logon) downlinks carried under label A0.
Handles all three observed envelope variants:
npm run buildpasses.