Better H1 Parsing

[makrsmark]

• Deprecate slash parser and add support in Label_H1 plugin
• add support for REJ/REQ/RES messages

Summary by CodeRabbit

• New Features

  • Added support for decoding messages with slash-prefixed format (/).
  • Extended decoding for REJ, REQ, and RES message types.
  • Added support for flight plan, route status, and ground address information extraction.

• Bug Fixes

  • Improved position report and timestamp parsing accuracy.

• Refactor

  • Consolidated slash-prefixed message handling into core decoder for better consistency.

[coderabbitai]

Walkthrough

The PR consolidates the specialized Label_H1_Slash plugin into the generic Label_H1 plugin, removing the standalone Label_H1_Slash class entirely. Label_H1 gains new logic to handle slash-prefixed messages. Supporting utilities (h1_helper, result_formatter, flight_plan_utils) are extended with new field parsing and formatting capabilities for additional H1 message types (REJ, REQ, RES, POS variants).

Changes

Cohort / File(s)	Summary
Plugin Consolidation & Removal lib/MessageDecoder.ts, lib/plugins/Label_H1_Slash.ts, lib/plugins/official.ts	Removed Label_H1_Slash plugin registration from MessageDecoder constructor and deleted the entire Label_H1_Slash class file. Removed corresponding export from official plugin registry.
Label_H1 Core Enhancement lib/plugins/Label_H1.ts	Added early path for slash-prefixed messages that splits by dot and decodes the H1 header, marking results as partial decoding and returning early. Preserved existing logic for non-slash messages.
Label_H1_Slash Migration & Tests lib/plugins/Label_H1_Slash.test.ts	Migrated test suite from Label_H1_Slash to Label_H1. Updated assertions to reflect Label_H1's output format with raw data checks replacing formatted item assumptions. Adjusted expected remaining text to include slash prefixes.
H1 Message Type Test Suites lib/plugins/Label_H1_POS.test.ts, lib/plugins/Label_H1_REJ.test.ts, lib/plugins/Label_H1_REQ.test.ts, lib/plugins/Label_H1_RES.test.ts	Added comprehensive test coverage for H1 preamble variants (POS, REJ, REQ, RES). Tests validate successful partial/full decoding, raw field extraction (checksum, altitudes, route data), and remaining text after decoding.
H1 Field Parsing Enhancement lib/utils/h1_helper.ts	Added handling for eight new H1 field keys (AK, DI, DQ, GA, RN, SP, WQ, RS). Refactored message type interpretation to support composite 6-character types via recursive processing. Centralized position, route, and address formatting through ResultFormatter.
Formatting & Flight Plan Utilities lib/utils/result_formatter.ts, lib/utils/flight_plan_utils.ts	Added six new static methods to ResultFormatter: requestedAltitudes, desiredAltitude, startPoint, routeNumber, flightPlan, groundAddress. Extended flight_plan_utils to handle FP and RS keys with appropriate formatting.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• using generic H1 decoder instead of specific #235 — Both PRs consolidate specialized H1 plugins into generic Label_H1 and remove/migrate plugin variant files.
• Fix H1 parsing #310 — Both PRs modify h1_helper and extend ResultFormatter with new field-parsing and formatting methods for H1 messages.
• Adding Label H2 header 02E parser #294 — Both PRs extensively modify the same H1 helper and ResultFormatter code paths for enhanced field handling.

Suggested reviewers

• kevinelliott
• fredclausen

Poem

🐰 A Slash marks the path, the Label_H1 now sees,
With plugins consolidated, logic flows with ease,
REJ, REQ, RES—new friends find their way,
The helpers format all, and tests ensure they stay! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Better H1 Parsing' accurately captures the main objective of the PR, which involves refactoring H1 message parsing to support additional message types (REJ, REQ, RES) and consolidating slash parser functionality.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

lib/plugins/Label_H1.ts (1)

22-35: Variable shadowing: const decoded on line 24 shadows the outer let decoded from line 21.

This works only because of the early return on line 35. If someone later refactors the early return away, the outer decoded will remain false and break the non-slash codepath. Rename the inner variable (e.g., slashDecoded) to avoid confusion.

Also, the comment on line 27 ("skip up to # and then a little more") is copied from the # handling branch and doesn't apply to the slash path.

Proposed fix

     if (msg.startsWith('/')) {
       const headerData = msg.split('.');
-      const decoded = H1Helper.decodeH1Message(
+      const slashDecoded = H1Helper.decodeH1Message(
         decodeResult,
         headerData.slice(1).join('.'),
-      ); // skip up to # and then a little more
-      if (decoded) {
+      );
+      if (slashDecoded) {
         decodeResult.remaining.text =
           headerData[0] + '.' + decodeResult.remaining.text;
-        decodeResult.decoded = decoded;
+        decodeResult.decoded = slashDecoded;
         decodeResult.decoder.decodeLevel = 'partial';
       }
 
       return decodeResult;

lib/utils/h1_helper.ts (2)

324-356: The 6-char composite type branch only handles exact length 6—verify this covers all expected composite types.

The messageType.length === 6 condition on line 333 correctly handles REJPWI, REJPOS, REQPWI, REQFPN, RESPWI, RESPOS etc. (all 3+3 composites). However, any composite type where the parts aren't exactly 3 chars each (or where messageParts[0] exceeds 6 chars due to trailing data like position coordinates) would fall through to the bottom processMessageType call and fail.

This appears safe given the current message grammar, but consider adding a comment clarifying the 3+3 assumption so future maintainers recognize the constraint.

---

35-38: Commented-out code (processAK, processDI) left as placeholders.

These cases currently route everything to ResultFormatter.unknown. The commented-out calls hint at future work. Consider adding TODO comments (like the existing one for PR) to make the intent discoverable, or remove the comments if there's no near-term plan.

Also applies to: 45-48

lib/plugins/Label_H1_REJ.test.ts (1)

4-11: Shared mutable message object could cause test coupling.

message is declared once and mutated in each test. If a test fails mid-assertion, the mutated .text leaks into subsequent tests. This matches the existing convention in Label_H1_POS.test.ts, so it's consistent across the codebase—but worth being aware of if flaky tests appear later. A safer pattern is to declare message inside each test.

lib/plugins/Label_H1_Slash.test.ts (2)

1-11: Consider renaming the test file to reflect the new plugin under test.

The file still bears the Label_H1_Slash name but now imports and exercises Label_H1. If the intent is to test the slash-path handling within Label_H1, a name like Label_H1_Slash.test.ts is arguably fine as a sub-suite, but worth confirming it won't confuse contributors looking for Label_H1_Slash plugin code that no longer exists.

---

41-59: Variant 2 test now mixes raw-data assertions with a formatted item count check — consider asserting key formatted labels too.

Lines 50–56 verify raw.* fields (position, route, altitude, OAT, ground_address, checksum), which is good. Line 57 asserts formatted.items.length === 6, but none of the six formatted labels are verified. If the formatter output ever regresses (e.g. wrong label or value), the test would still pass.

Variants 1 and 3 already assert both raw and formatted content, so variant 2 is less robust by comparison. Adding a couple of representative formatted-item assertions would keep the coverage consistent.

lib/utils/result_formatter.ts (1)

546-564: Missing NaN/empty-array guards for altitude methods, unlike altitude().

altitude() (line 77) returns early on isNaN(value), but desiredAltitude and requestedAltitudes don't. If an upstream parser passes NaN or an empty array, the formatted output will show "NaN" or an empty string respectively. Consider adding the same defensive checks for consistency.

Proposed guards

  static requestedAltitudes(decodeResult: DecodeResult, values: number[]) {
+   if (!values || values.length === 0) {
+     return;
+   }
    decodeResult.raw.requested_alts = values;
    ...
  }

  static desiredAltitude(decodeResult: DecodeResult, value: number) {
+   if (isNaN(value)) {
+     return;
+   }
    decodeResult.raw.desired_alt = value;
    ...
  }

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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