Adding label 16 AUTPOS decoding#340
Conversation
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (1)
WalkthroughAdds a new Label_16 AUTPOS decoder plugin and test suite, plus two ResultFormatter helpers (totalAirTemp and timestamp) to parse and format AUTPOS position reports, coordinates, timestamps, temperatures, wind, airspeed, and fuel fields. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@SoTi-AF - here you go |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lib/plugins/Label_16_AUTPOS.test.ts (1)
41-60: Consider adding assertion foroutside_air_temperature.The test verifies
total_air_temperature(TAT) but doesn't assert theoutside_air_temperature(SAT) field. The message containsSAT -057which should parse to-57.💡 Suggested addition
expect(decodeResult.raw.total_air_temperature).toBe(-27); + expect(decodeResult.raw.outside_air_temperature).toBe(-57); expect(decodeResult.raw.airspeed).toBe(476);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/plugins/Label_16_AUTPOS.test.ts` around lines 41 - 60, Add an assertion checking that the SAT value is parsed into outside_air_temperature: after calling plugin.decode(message) and before checking formatted.items length, assert that decodeResult.raw.outside_air_temperature equals -57 (parsed from "SAT -057"); update the test block in Label_16_AUTPOS.test.ts where decodeResult is used (the test('matches all values') function) to include this new expectation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/plugins/Label_16_AUTPOS.ts`:
- Around line 111-121: The comment above the DAT/TIM parsing is wrong: DAT is
actually in YYMMDD (e.g., 260228 -> 2026-02-28), but the code slices dat into yy
= dat.slice(0,2), mm = dat.slice(2,4), dd = dat.slice(4,6) and then builds
mmddyy for DateTimeUtils.convertDateTimeToEpoch; update the comment to state
"DAT is YYMMDD, TIM is HHMMSS" (or similar) so it matches the parsing logic in
this block that calls ResultFormatter.timestamp and
DateTimeUtils.convertDateTimeToEpoch(tim, mmddyy). Ensure the comment explicitly
notes convertDateTimeToEpoch expects MMDDYY.
- Around line 7-14: The plugin class Label_16_AUTPOS is not exported from the
central plugins export module, so MessageDecoder never registers it; fix this by
adding an export for the Label_16_AUTPOS plugin in the same central plugins
export file alongside the other Label_16 variants so it follows the same export
pattern and will be picked up by MessageDecoder.decode().
In `@lib/utils/result_formatter.ts`:
- Around line 309-318: The change renaming the formatted item type from
'outside_air_temperature' to 'temperature' breaks existing tests that expect
decodeResult.formatted.items[*].type === 'outside_air_temperature'; revert the
type value in the object pushed in result_formatter.ts so it remains
'outside_air_temperature', or if you intentionally want the new value, update
all test expectations (e.g., in Label_H1_POS.test.ts and Label_21_POS.test.ts)
and any other code that matches on decodeResult.formatted.items[].type to use
'temperature' instead; ensure you update every occurrence of the type string and
run the test suite to confirm no other consumers rely on the old value.
---
Nitpick comments:
In `@lib/plugins/Label_16_AUTPOS.test.ts`:
- Around line 41-60: Add an assertion checking that the SAT value is parsed into
outside_air_temperature: after calling plugin.decode(message) and before
checking formatted.items length, assert that
decodeResult.raw.outside_air_temperature equals -57 (parsed from "SAT -057");
update the test block in Label_16_AUTPOS.test.ts where decodeResult is used (the
test('matches all values') function) to include this new expectation.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/plugins/Label_16_AUTPOS.test.tslib/plugins/Label_16_AUTPOS.tslib/utils/result_formatter.ts
| export class Label_16_AUTPOS extends DecoderPlugin { | ||
| name = 'label-16-autpos'; | ||
| qualifiers() { | ||
| return { | ||
| labels: ['16'], | ||
| preambles: [''], | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Label_16_AUTPOS is registered in MessageDecoder
rg -n "Label_16_AUTPOS" lib/MessageDecoder.ts
# List all registered Label_16 plugins for comparison
rg -n "Label_16" lib/MessageDecoder.tsRepository: airframesio/acars-decoder-typescript
Length of output: 331
🏁 Script executed:
#!/bin/bash
# Check if Label_16_AUTPOS.ts file exists
ls -la lib/plugins/Label_16_AUTPOS.ts
# Check if it's exported from the Plugins index/module
rg -n "Label_16_AUTPOS" lib/plugins/Repository: airframesio/acars-decoder-typescript
Length of output: 487
🏁 Script executed:
#!/bin/bash
# Check how plugins are imported in MessageDecoder.ts
head -20 lib/MessageDecoder.ts
# Check if there's an index/barrel export in plugins directory
cat lib/plugins/index.ts 2>/dev/null || ls -la lib/plugins/*.ts | grep -i indexRepository: airframesio/acars-decoder-typescript
Length of output: 611
🏁 Script executed:
#!/bin/bash
# Check plugins/official.ts for Label_16_AUTPOS export
rg -n "Label_16_AUTPOS|export" lib/plugins/official.ts | head -50Repository: airframesio/acars-decoder-typescript
Length of output: 1836
🏁 Script executed:
#!/bin/bash
# Get the full plugins/official.ts file to check for Label_16_AUTPOS
wc -l lib/plugins/official.ts
# Check if Label_16_AUTPOS appears anywhere in the file
rg "Label_16_AUTPOS" lib/plugins/official.tsRepository: airframesio/acars-decoder-typescript
Length of output: 106
Plugin not exported from official plugins module.
The Label_16_AUTPOS plugin is not exported from lib/plugins/official.ts and therefore not registered in MessageDecoder.ts. The plugin class exists but won't be invoked when decoding messages through the main MessageDecoder.decode() API. Add the export line to lib/plugins/official.ts alongside the other Label_16 variants (lines 11-14):
export * from './Label_16_AUTPOS';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/plugins/Label_16_AUTPOS.ts` around lines 7 - 14, The plugin class
Label_16_AUTPOS is not exported from the central plugins export module, so
MessageDecoder never registers it; fix this by adding an export for the
Label_16_AUTPOS plugin in the same central plugins export file alongside the
other Label_16 variants so it follows the same export pattern and will be picked
up by MessageDecoder.decode().
| if (!dat.includes('*') && !tim.includes('*')) { | ||
| // DAT is DDMMYY, TIM is HHMMSS | ||
| // DateTimeUtils.convertDateTimeToEpoch expects MMDDYY | ||
| const yy = dat.slice(0, 2); | ||
| const mm = dat.slice(2, 4); | ||
| const dd = dat.slice(4, 6); | ||
| const mmddyy = `${mm}${dd}${yy}`; | ||
| ResultFormatter.timestamp( | ||
| decodeResult, | ||
| DateTimeUtils.convertDateTimeToEpoch(tim, mmddyy), | ||
| ); |
There was a problem hiding this comment.
Date format comment is incorrect; actual format appears to be YYMMDD.
The comment states DAT is DDMMYY, but the code extracts yy = dat.slice(0, 2), mm = dat.slice(2, 4), dd = dat.slice(4, 6), which treats the format as YYMMDD. Based on the test case where 260228 produces a timestamp for Feb 28, 2026, the actual format is YYMMDD.
📝 Proposed fix
if (!dat.includes('*') && !tim.includes('*')) {
- // DAT is DDMMYY, TIM is HHMMSS
+ // DAT is YYMMDD, TIM is HHMMSS
// DateTimeUtils.convertDateTimeToEpoch expects MMDDYY
const yy = dat.slice(0, 2);
const mm = dat.slice(2, 4);
const dd = dat.slice(4, 6);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!dat.includes('*') && !tim.includes('*')) { | |
| // DAT is DDMMYY, TIM is HHMMSS | |
| // DateTimeUtils.convertDateTimeToEpoch expects MMDDYY | |
| const yy = dat.slice(0, 2); | |
| const mm = dat.slice(2, 4); | |
| const dd = dat.slice(4, 6); | |
| const mmddyy = `${mm}${dd}${yy}`; | |
| ResultFormatter.timestamp( | |
| decodeResult, | |
| DateTimeUtils.convertDateTimeToEpoch(tim, mmddyy), | |
| ); | |
| if (!dat.includes('*') && !tim.includes('*')) { | |
| // DAT is YYMMDD, TIM is HHMMSS | |
| // DateTimeUtils.convertDateTimeToEpoch expects MMDDYY | |
| const yy = dat.slice(0, 2); | |
| const mm = dat.slice(2, 4); | |
| const dd = dat.slice(4, 6); | |
| const mmddyy = `${mm}${dd}${yy}`; | |
| ResultFormatter.timestamp( | |
| decodeResult, | |
| DateTimeUtils.convertDateTimeToEpoch(tim, mmddyy), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/plugins/Label_16_AUTPOS.ts` around lines 111 - 121, The comment above the
DAT/TIM parsing is wrong: DAT is actually in YYMMDD (e.g., 260228 ->
2026-02-28), but the code slices dat into yy = dat.slice(0,2), mm =
dat.slice(2,4), dd = dat.slice(4,6) and then builds mmddyy for
DateTimeUtils.convertDateTimeToEpoch; update the comment to state "DAT is
YYMMDD, TIM is HHMMSS" (or similar) so it matches the parsing logic in this
block that calls ResultFormatter.timestamp and
DateTimeUtils.convertDateTimeToEpoch(tim, mmddyy). Ensure the comment explicitly
notes convertDateTimeToEpoch expects MMDDYY.
There was a problem hiding this comment.
this is incorrect. the code is corect
There was a problem hiding this comment.
@makrsmark, I apologize for any confusion. To clarify, I was pointing out that the comment text on line 111 states // DAT is DDMMYY, but the code logic parses it as YYMMDD (year-month-day). The code itself is correct—I was suggesting updating only the comment to match what the code does.
However, if you feel the comment is fine as-is or prefer to keep it, that's perfectly okay. I'll defer to your judgment on this.
🧠 Learnings used
Learnt from: makrsmark
Repo: airframesio/acars-decoder-typescript PR: 332
File: lib/plugins/Label_H1.ts:39-40
Timestamp: 2026-02-21T01:32:05.328Z
Learning: In lib/plugins/Label_H1.ts, when parsing two-part H1 messages (split by '#'), the offset logic `parts[0] === '- ' || isNaN(parseInt(parts[1][1])) ? 3 : 4` is correct. The `parts[0] === '- '` condition ensures that messages like '- `#M1FPN/`...' use offset=3 to skip 3 characters after '#' ('M1' + next char), whereas messages like 'F37AKL0767#M1BFPN/...' use offset=4 to skip 4 characters ('M1B' + '/'). Removing the `parts[0] === '- '` check would break '- `#M1`...' messages where parts[1][1] is numeric.
1 participant
Summary by CodeRabbit
New Features
Improvements
Tests