Skip to content

Add Label A6 ADS-C Periodic Position decoder plugin#409

Open
thepacket wants to merge 1 commit intoairframesio:masterfrom
thepacket:plugin/label-a6
Open

Add Label A6 ADS-C Periodic Position decoder plugin#409
thepacket wants to merge 1 commit intoairframesio:masterfrom
thepacket:plugin/label-a6

Conversation

@thepacket
Copy link
Copy Markdown

Adds a decoder for FANS 1/A ADS-C periodic position reports carried under label A6. Ground + tail + contract-request-number surfaced; binary payload preserved as hex for downstream ARINC-745 decoding.

npm run build passes.

New plugin registered in official.ts and MessageDecoder.ts.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

Warning

Rate limit exceeded

@thepacket has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 55 minutes and 29 seconds before requesting another review.

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 29 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7bca81dc-79cd-47b9-acee-34ec2b6d6d60

📥 Commits

Reviewing files that changed from the base of the PR and between 10f9f69 and 0fd3f63.

📒 Files selected for processing (3)
  • lib/MessageDecoder.ts
  • lib/plugins/Label_A6_ADS.ts
  • lib/plugins/official.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@kevinelliott kevinelliott left a comment

Choose a reason for hiding this comment

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

Summary

Adds Label_A6_ADS to surface ARINC-622 envelope fields (ground address, IMI, tail, hex payload, optional CRC) for label A6. Plugin is registered in both lib/MessageDecoder.ts and lib/plugins/official.ts. Build passes per the description. Thoughtful inline ASCII art makes the wire format easy to follow.

Verdict

Needs changes before merge. The decode logic is reasonable for an unparsed-payload pass-through, but a few correctness concerns and the lack of tests are blockers.

Must Fix

  • No tests. Every other plugin under lib/plugins/ ships a companion *.test.ts. Please add one with at least one happy-path message and one malformed input that hits the early-return branches. Use lib/plugins/Label_15.test.ts as a template.
  • PR title vs. content mismatch. The PR title says "ADS-C Periodic Position" but A6 in ARINC 620 is "Request ADS Reports" (RAR), and the plugin description correctly reflects that ("Request ADS Report (RAR) — ADS-C contract"). Either rename the PR/file to match the RAR semantics, or clarify why "Periodic Position" is the intended product name. Right now a downstream consumer searching for "A6" gets contradictory descriptors.
  • Speculative 2-char CRC branch. ARINC-622 ATS messages use a 16-bit CRC encoded as 4 ASCII hex digits. The else if (adjAfter.length >= 6) branch (line 111-114) splits the trailing 2 chars and labels them crc_or_tail_byte ("short — possibly payload byte"). This is guessing. Please either (a) only emit a CRC when there are exactly 4 trailing hex chars, or (b) leave those bytes inside ads_payload_hex and skip the CRC field entirely. Surfacing an "uncertain" CRC under the same crc_hex raw key as the real CRC will confuse downstream code.

Should Fix

  • Tail-extraction heuristic is brittle. The "look at the first 8 chars and find the last non-hex char" rule will misidentify any tail composed entirely of hex characters (e.g. JA788A, B12345, A6CDEF, …). The cascade approach used in PR #411 (Label_AA_CPDLC.ts) — try a list of known registration patterns and accept the first whose remainder is even-length hex — would be more robust here too. Consider extracting that into a shared utility so all FANS 1/A plugins use the same logic.
  • Non-standard raw keys. air_data, ads_payload_hex, crc_hex, imi aren't part of the canonical RawFields set defined in lib/DecoderPluginInterface.ts. The interface allows extension via the index signature, but please check with maintainers whether you should add the well-known ones (imi, crc) to that interface for consistency across A6/AA/B0 plugins.
  • payload_hex value contains the byte count. value: '${hex} (${hex.length / 2} bytes)' — putting the byte count into the human-readable value is fine, but storing the byte count separately as a raw field (the way PR #411 does with cpdlc_payload_bytes) is more downstream-friendly.

Nits

  • The "Direction" item is hardcoded to "Uplink (ground → aircraft)" but the docstring concedes A6 can carry RAR or other ADS-C protocol messages depending on IMI. Consider deriving direction from the IMI rather than asserting uplink unconditionally.
  • decodeResult.raw.imi = imi; — also surface this in formatted.items only if it's something other than ADS, otherwise it's redundant with the Message Type row.

Tests

Missing entirely. Suggested fixtures:

  • Happy path: /NYCODYA.ADS.N183AM 0804140A28C4 01 (matches the wire-format example).
  • Hex-only tail: a JA… registration that's all hex chars, to confirm the cascade can still find a sensible split (this will require the cascade fix above to pass).
  • Malformed: missing IMI dot, missing payload, non-hex payload — should set decoded: false and decodeLevel: 'none'.

Notes

  • Registration: confirmed in both lib/MessageDecoder.ts (line 81) and lib/plugins/official.ts (line 67) at the PR HEAD.
  • FANS 1/A consistency: please coordinate the tail-extraction logic and crc raw-key naming with PR #411 (Label AA) and PR #412 (Label B0) so the three plugins behave the same on overlapping fields.

Thanks @thepacket!

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