Skip to content

Add Label H2 AMDAR Meteorological decoder plugin#418

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

Add Label H2 AMDAR Meteorological decoder plugin#418
thepacket wants to merge 1 commit intoairframesio:masterfrom
thepacket:plugin/label-h2

Conversation

@thepacket
Copy link
Copy Markdown

Adds a decoder for AMDAR (Aircraft Meteorological Data Relay) waypoint reports on label H2 — a stream of 28-char position+altitude+temperature+wind records terminated by G.

Record format

N 60230 E 008174 3528 M 552 125 021 G

Handles leading fragment detection (multi-part transmissions). Coordinates: DDMMT (degrees+minutes+tenths), altitude in tens of feet, temperature in tenths of °C with P/M sign, wind direction + speed.

Coexists with existing Label_H2_02E.

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 12 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 12 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: d731358c-200a-40d6-a4b7-ab87181e5794

📥 Commits

Reviewing files that changed from the base of the PR and between 10f9f69 and 5dd533f.

📒 Files selected for processing (3)
  • lib/MessageDecoder.ts
  • lib/plugins/Label_H2_AMDAR.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_H2_AMDAR for label H2 AMDAR (Aircraft Meteorological Data Relay) waypoint reports — a stream of 28-char position+altitude+temperature+wind records terminated by G. Registered last in MessageDecoder.ts/official.ts. Coexists with the existing Label_H2_02E. No companion test file.

Verdict

Comment-only review — the per-record decoding looks careful and correct. Two structural concerns: missing tests, and a missing preamble qualifier that makes coexistence with Label_H2_02E work only because of how 02E's internal validation happens to fail-soft. Please address before merge.

Must Fix

  • Add a Label_H2_AMDAR.test.ts. No tests at all today. At minimum: qualifiers() shape, a multi-record happy path (2–3 records), the leading-fragment branch (0G... prefix), the no-records bail, and a non-matching string.

  • Coexistence with Label_H2_02E is fragile. AMDAR declares qualifiers: { labels: ['H2'] } with no preambles, so it runs against every H2 message. The dispatcher tries H2_02E first (registered earlier, line 59 vs your line 81), and H2_02E only succeeds when the message starts with '02E' and the trailing chunk is 'Q'. For an 02E-prefixed message that doesn't satisfy parts[parts.length - 1] !== 'Q', H2_02E returns decoded: false and the dispatcher falls through to AMDAR. AMDAR's own firstRec regex won't match an 02E-shaped weather report, so it bails — but that's silent, not deliberate.

    Two cleaner options:

    1. Add preambles: ['N', 'S', '0G', '1G', /* etc, the leading-frag patterns you actually observe */] so the dispatcher can short-circuit.
    2. Or front-validate explicitly: if text.startsWith('02E'), return decoded: false early without running the regex.

    Option 1 is cheaper; option 2 is more defensive. Either is fine — please pick one and add a test confirming AMDAR does not claim a 02E-prefixed message.

Should Fix

  • text.replace(/\s+/g, '') strips ALL whitespace before parsing. That works for the documented format, but if a multi-part transmission concatenates two records without their separating space, you'd already be fine; if a record contains an embedded space-separated subfield in some real-world variant, you'd silently fold it in. Consider replacing only the documented inter-field spaces explicitly, or document this as a deliberate normalization step.
  • HHMMSS / temperature validation gap. Wind direction up to 360°, wind speed unbounded, temperature unbounded — the regex enforces digits but not ranges. wind direction = 999 will be reported verbatim. Reject on windDir > 360, windSpd > 999 (or whatever the documented max is) to surface clearly malformed records.
  • ResultFormatter.temperature(decodeResult, String(first.temperatureC)) — the formatter expects M/P prefixes for sign ('M55.2'), not a raw signed decimal ('-55.2'). Read result_formatter.ts:331 — the implementation does Number(value.replace('M','-').replace('P','+')). For '-55.2', neither replace fires, Number('-55.2') returns -55.2, so it happens to work — but that's a coincidence, not the contract. Either pass it as M55.2/P55.2 to align with the formatter's intent, or store directly via raw.outside_air_temperature and skip the formatter for this field.
  • Anchor data is the first record only. The headline position/altitude/temperature come from records[0], but for a multi-record AMDAR transmission the last (most recent) sample is usually the more useful headline. Worth a comment explaining the choice or switching to records[records.length - 1].
  • raw.leading_fragment = fragment || undefined; — fine, but the formatted item is only pushed when fragment is non-empty. Make sure the test for the no-fragment case asserts the absence of the code: 'FRAG' row.
  • Item codes REC1, REC2, … — using a per-record code/type that varies (record_${idx}) makes programmatic filtering harder than a stable type: 'amdar_record', code: 'REC', value: ... pattern. Consumers that index by type will see N distinct types per message.

Nits

  • text.search(/\.../) returns -1 for no match. Your firstRec > 0 check handles > 0 and < 0 but not === 0 (perfectly aligned, no fragment) — that's fine because body = text is already correct and fragment = '' from initialization. Tests should still cover both branches.
  • The doc-comment ASCII art is excellent.
  • Math.round(lat * 1e5) / 1e5 to 5 decimals (~1m precision) is reasonable for AMDAR.

Tests

None added. Please add Label_H2_AMDAR.test.ts. See "Must Fix" for coverage.

Notes — Registration & Coexistence

  • Registration order: Label_H2_02E is at line 59, Label_H2_AMDAR at line 81. So 02E gets first crack at every H2 message — correct, given AMDAR has no preamble qualifier and would otherwise grab everything.
  • Coexistence: Today AMDAR doesn't actually claim 02E-prefixed messages because its firstRec regex won't match that shape. Works in practice; please make it explicit (see "Must Fix" #2).
  • Preambles: None set on AMDAR. Recommended to add — see above.

Thanks @thepacket! The numeric decoding (DDMMT coords, tens-of-feet altitude, signed-tenths-of-degrees temp) reads cleanly and is well-documented.

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