Add Label H2 AMDAR Meteorological decoder plugin#418
Add Label H2 AMDAR Meteorological decoder plugin#418thepacket wants to merge 1 commit intoairframesio:masterfrom
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 12 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
There was a problem hiding this comment.
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_02Eis fragile. AMDAR declaresqualifiers: { labels: ['H2'] }with nopreambles, so it runs against every H2 message. The dispatcher triesH2_02Efirst (registered earlier, line 59 vs your line 81), andH2_02Eonly succeeds when the message starts with'02E'and the trailing chunk is'Q'. For an02E-prefixed message that doesn't satisfyparts[parts.length - 1] !== 'Q',H2_02Ereturnsdecoded: falseand the dispatcher falls through to AMDAR. AMDAR's ownfirstRecregex won't match an02E-shaped weather report, so it bails — but that's silent, not deliberate.Two cleaner options:
- Add
preambles: ['N', 'S', '0G', '1G', /* etc, the leading-frag patterns you actually observe */]so the dispatcher can short-circuit. - Or front-validate explicitly: if
text.startsWith('02E'), returndecoded: falseearly 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.
- Add
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 = 999will be reported verbatim. Reject onwindDir > 360,windSpd > 999(or whatever the documented max is) to surface clearly malformed records. ResultFormatter.temperature(decodeResult, String(first.temperatureC))— the formatter expectsM/Pprefixes for sign ('M55.2'), not a raw signed decimal ('-55.2'). Readresult_formatter.ts:331— the implementation doesNumber(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 asM55.2/P55.2to align with the formatter's intent, or store directly viaraw.outside_air_temperatureand skip the formatter for this field.- Anchor data is the first record only. The headline
position/altitude/temperaturecome fromrecords[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 torecords[records.length - 1]. raw.leading_fragment = fragment || undefined;— fine, but the formatted item is only pushed whenfragmentis non-empty. Make sure the test for the no-fragment case asserts the absence of thecode: 'FRAG'row.- Item codes
REC1,REC2, … — using a per-record code/type that varies (record_${idx}) makes programmatic filtering harder than a stabletype: 'amdar_record', code: 'REC', value: ...pattern. Consumers that index bytypewill see N distinct types per message.
Nits
text.search(/\.../)returns-1for no match. YourfirstRec > 0check handles> 0and< 0but not=== 0(perfectly aligned, no fragment) — that's fine becausebody = textis already correct andfragment = ''from initialization. Tests should still cover both branches.- The doc-comment ASCII art is excellent.
Math.round(lat * 1e5) / 1e5to 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_02Eis at line 59,Label_H2_AMDARat 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
firstRecregex 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.
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
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 buildpasses.