Skip to content

Update Label 16 POSA1 — fix altitude field, surface track/ETA/OAT#419

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

Update Label 16 POSA1 — fix altitude field, surface track/ETA/OAT#419
thepacket wants to merge 1 commit intoairframesio:masterfrom
thepacket:plugin/label-16-update

Conversation

@thepacket
Copy link
Copy Markdown

Corrects the field-3 interpretation in Label_16_POSA1:

  • Previously treated field[3] (349) as altitude × 100 = 34,900 ft
  • Per analyst review of documented examples (POSA1N52969E 16393,RANOK ,080055,330,KORUP ,083244,,-53, 54, 93,828), this field is ground track in degrees; altitude is not present

Also surfaces OAT, report timestamp, and ETA as their own rows instead of burying them inside unknownArr. Wild-guess fields at positions 8–10 remain ignored.

Decode level bumped from partial to full.

npm run build passes.

Extended to handle additional sub-formats and/or bug fixes.
@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: 816de829-a58e-4bcc-a669-fd1141cfabbe

📥 Commits

Reviewing files that changed from the base of the PR and between 10f9f69 and 53a10df.

📒 Files selected for processing (1)
  • lib/plugins/Label_16_POSA1.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

Reinterprets Label_16_POSA1 field 3 as ground track (degrees) instead of altitude × 100, and surfaces the report timestamp, ETA at next waypoint, and OAT as their own labelled rows instead of dumping them into unknownArr. Decode level is bumped from partial to full. Excellent ASCII field-map docblock — very helpful for future maintainers.

Verdict

Comment-only review. Direction is good, the field reinterpretation is plausible, and the new structured output is a net improvement. However, this PR cannot merge as-is — it breaks two of the existing four unit tests because the test file is unchanged.

Must Fix

  1. Existing tests fail. Label_16_POSA1.test.ts is not updated. Both decodes variant 1 and decodes redacted fail:

    • assert decodeLevel === 'partial' — now 'full'
    • assert formatted.items.length === 3 — now ~6
    • assert items[1].label === 'Altitude' and value === '37000 feet' — Altitude row is removed
    • assert remaining.text === ',-61,139,1174,829' and ',*****,*****, 744, 0'unknownArr is no longer called for fields 6+

    Please update both fixtures to assert the new structure (Track, Timestamp, ETA, OAT rows) and the new decodeLevel: 'full'. Without this, CI will go red.

  2. Verify field 3 really is ground track, not altitude. The PR body says "Per analyst review of documented examples (POSA1N52969E 16393,RANOK ,080055,330,KORUP ,083244,...), this field is ground track in degrees; altitude is not present." The existing fixture is 370 and the test labelled it 37000 feet, which would be plausible for an airliner but the value 349 (the example you cite from RANOK) is an extremely on-the-nose track value. I am inclined to trust your analyst review, but please add a brief reference to the document/source in a comment so future maintainers can verify. If altitude is genuinely absent from POSA1, that is a meaningful claim worth a citation.

Should Fix

  • The redacted fixture has field 7 = ***** (5 stars), which fails your /^-?\d{1,3}$/ test for OAT — fine, it just gets skipped. But field 3 in the redacted fixture is 390, which still parses as track. That is probably wrong (390° is invalid for a track 0–359). Consider tightening the track regex to /^(0|[1-9]\d?|[12]\d{2}|3[0-5]\d)$/ or just Number(x) >= 0 && Number(x) <= 359. As-is the plugin will happily emit 390° from 370 (the original variant 1 fixture).

Nits

  • The "wild guess" trailing-fields comment is great, but consider a // TODO(label-16-posa1): trailing fields 8–10 are undocumented to make it greppable.

Backward Compat / Regression Risk

  • Breaking output change: consumers reading decodeResult.raw.altitude from this plugin will see it disappear. If altitude was actually correct (rare), this is a real regression. If altitude was always wrong (your claim), this is the right fix. Either way, the change is breaking and should be noted in the changelog.
  • The unknownArr body is now empty, so decodeResult.remaining.text will be '' instead of the trailing fields. Consumers parsing remaining.text for fields 6–10 will see them disappear. Reasonable, given the analyst guidance to ignore wild guesses.
  • Decode level partial → full is an upgrade — fine if all knowable fields are now surfaced.

Test Coverage

Currently strictly narrower because tests aren't updated. Once tests are rewritten:

  • New rows (Timestamp, Track, ETA, OAT) need explicit assertions.
  • A negative-OAT case is already covered by -61 in variant 1 and -53 from your example — please assert the formatted OAT value in the test.
  • A track-out-of-range fixture (e.g. 390, 400) would be a useful guard rail given the regex is permissive.

Outstanding Existing Comments

None. Only an auto-generated CodeRabbit rate-limit notice on the PR.

Thanks @thepacket! The structured output is a clear improvement and the field-map docblock is genuinely useful — this just needs the test file updated and ideally a citation for the altitude claim before merge.

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