Skip to content

Add Label H1 APM (ACMF Snapshot) decoder plugin#417

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

Add Label H1 APM (ACMF Snapshot) decoder plugin#417
thepacket wants to merge 1 commit intoairframesio:masterfrom
thepacket:plugin/label-h1

Conversation

@thepacket
Copy link
Copy Markdown

Adds a decoder for H1 channel ACMF (Airplane Condition Monitoring Function) cruise performance snapshots, matching the #CFBAPM_REPORT pattern.

Wire format

APM 6 HL8573 KAL075 RKSI CYVR 200426221336
,ACMF0021,.837,,,285.2,,,-28.81,,,35001,,,432443,,,5681,5697,,, 84.72, 84.72,,,  5.495,535.0,,, 66.7, 66.7, 95.5, 95.5,,,…

Surfaces tail, flight, origin, destination, YYMMDDHHMMSS timestamp, ACMF version, Mach, OAT, pressure altitude, dual-engine N1/N2/EGT. Wild-guess fields (fuel flows, gross weight, flag soup) intentionally not surfaced.

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 14 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 14 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: 0f90ec2c-a6b9-4104-916a-2b17f29663d3

📥 Commits

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

📒 Files selected for processing (3)
  • lib/MessageDecoder.ts
  • lib/plugins/Label_H1_APM.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_H1_APM (Aircraft Performance Monitoring / ACMF Snapshot) for H1 messages matching APM <seq> <tail> <flight> <origin> <dest> <YYMMDDHHMMSS> followed by a comma-delimited ACMF body. Registered last in MessageDecoder.ts/official.ts. No companion test file.

Verdict

Comment-only review — there's a real, demonstrable header-regex bug that breaks the headline #CFBAPM use case from the PR description. Please address before merge. There are also a handful of registration/coexistence and convention concerns specific to adding a second H1 plugin to a label that already has Label_H1_OHMA.

Must Fix

  • Header regex does not match #CFBAPM ... — the very prefix the PR description and qualifiers().preambles advertises.

    The regex is:

    /(?:^|\b)APM\s+(?<seq>\d+)\s+.../

    For input #CFBAPM 6 HL8573 KAL075 RKSI CYVR 200426221336:

    • ^ alternative fails — the string starts with #, not APM.
    • \b alternative fails — \b is a transition between a word char and a non-word char. In …BAPM, both B and A are word chars, so there is no word boundary between them. APM is not at a \b here.

    Verified empirically:

    IN : "APM 6 HL8573 KAL075 RKSI CYVR 200426221336"            → match @ 0
    IN : "#CFBAPM 6 HL8573 KAL075 RKSI CYVR 200426221336"        → NO MATCH
    IN : "#CFBAPM_REPORTAPM 6 HL8573 KAL075 RKSI CYVR 200426..." → NO MATCH
    IN : "#CFBAPM\nAPM 6 HL8573 KAL075 RKSI CYVR 200426221336"   → match @ 8
    

    So the plugin currently only decodes messages that literally start with APM (the first sample) or have the literal APM token preceded by a non-word character such as \n (the fourth sample). The middle two — which are the patterns the doc-comment and qualifiers().preambles: ['#CFBAPM'] actually claim to support — fail.

    Recommended fix: drop the \b and explicitly skip any leading prefix. For example:

    const m = text.match(/^(?:#?CFB)?APM\s+(?<seq>\d+)\s+(?<tail>...)/);

    or strip a #CFBAPM_REPORT / #CFBAPM lead before applying the existing regex. Either way, please add explicit test cases for the prefixed forms — that's what the qualifier preambles and the PR description both advertise.

  • Add a Label_H1_APM.test.ts. No tests at all. Required minimum: qualifiers() shape (preambles included), happy path with bare APM ..., #CFBAPM ..., #CFBAPM_REPORT..., malformed timestamp, and a non-matching string.

Should Fix

  • Coexistence with Label_H1_OHMA — preamble check passes (OHMA uses OHMA, /RTNBOCR.OHMA, #T1B/RTNBOCR.OHMA; APM uses APM, #CFBAPM). No string-prefix collision. Confirmed by inspection. However, several existing H1 plugins (Label_H1_M_POS, others) declare no preambles and run for every H1 message. Their internal regexes (^M\d{2}A...) won't grab APM input, but please be explicit and assert that in tests so future refactors don't regress.

  • HHMMSS validation gap. hh, mm, ss are sliced and shoved straight into convertHHMMSSToTod without bounds checks. Garbage like 999999999999 would parse without complaint. Reject when hh > 23, mm > 59, ss > 59 (and ideally mo/dd likewise).

  • Date is dropped from the canonical timestamp. ResultFormatter.timestamp(decodeResult, DateTimeUtils.convertHHMMSSToTod(...)) produces a "seconds since midnight" value that loses the YYMMDD part. You preserve the date in raw.report_date and the UI shows it via the custom code: 'DATE' item, but downstream consumers reading raw.message_timestamp will see a time-of-day-only number that doesn't agree with the date. Either:

    1. Use DateTimeUtils.convertDateTimeToEpoch(hh+mm+ss, 'DD MM YY') (or the right helper — see Label_15.ts line 47) so message_timestamp is a real epoch, or
    2. Skip ResultFormatter.timestamp and stick with the explicit report_date / report_time items.

    As-is, this hits the project's "Both ResultFormatter.timestamp() AND explicit {type:'time'} push" antipattern (you push report_date/report_time yourself in addition to the formatter call).

  • ResultFormatter.temperature(decodeResult, oat) for OAT is appropriate semantically, but the formatter's value-string ('-28.81 degrees') is OAT, while ACMF snapshots also include TAT in nearby fields. Consider documenting why you intentionally only surface OAT.

  • **ResultFormatter.altitude(decodeResult, Number(alt)) for 35001 ft is correct but ACMF altitude is typically pressure altitude — worth surfacing that as a label suffix ('Pressure Altitude') since other plugins use plain "Altitude" for GPS/baro. Optional.

Nits

  • decodeResult.formatted.items.unshift({...}) for the message-type row is fine, but the rest of the items use push. Mixing the two is a small readability cost; consider building items in order.
  • The Mach validation regex /^\.\d+$|^\d\.\d+$/ is fine but excludes 0.837 (with leading zero) and 1.0 (M1.0+ for fighters/test). Loosen to /^-?\d*\.?\d+$/ if you want to be defensive.
  • decodeResult.raw.acmf_version = acmfVer; — string is fine, but if every observed value is ACMF\d{4}, consider parsing the digits separately for easier filtering.
  • Doc comment mentions #CFBAPM_REPORT explicitly — make sure your final regex actually handles that form (current one doesn't).

Tests

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

Notes — Registration & Coexistence

  • Registration order in MessageDecoder.ts: APM is appended last (line 81), after Label_H1_OHMA (line 64). For label-specific dispatch this is fine; the labelIndex keys by label and order within the label only matters when multiple plugins could match.
  • OHMA preambles vs APM preambles: ['OHMA', '/RTNBOCR.OHMA', '#T1B/RTNBOCR.OHMA'] vs ['APM', '#CFBAPM']. No prefix collision. ✓
  • Preamble-less H1 plugins (M_POS, OHMA does have preambles, etc.): their internal regexes won't match an APM message. Confirmed by inspection. ✓
  • **The qualifier preambles: ['APM', '#CFBAPM'] is correct as written, but the dispatcher's message.text.startsWith(p) short-circuits on these. As long as the wire really starts with APM or #CFBAPM, the plugin will be invoked — but then the header regex still has to actually match, which is where the bug above bites.

Thanks @thepacket! The ACMF snapshot decoding logic itself is solid — it's just the entry-point regex and missing tests that need attention.

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