fix(hid): correct IcomRC28Parser byte offsets and report size (#1896)

[NF0T]

Summary

The Icom RC-28 parser has never worked correctly. The knob did not tune
the radio, and turning it caused spurious transmit toggles. This fixes
the root cause: completely wrong byte-offset assumptions in
IcomRC28Parser::parse().

Root cause

The parser comment claimed "64-byte reports, byte 0 = encoder counter,
byte 1 = button bitmask." The RC-28 actually sends 32-byte reports
with this layout (0-indexed, after the hidapi report ID byte):

Offset	Meaning
0	Sequence counter: 0x01 = first report of step, 0x02 = second
1	Unused (0x00)
2	Direction: 0x01 = CW, 0x02 = CCW, 0x00 = none
3	Unused (0x00)
4	Button state: 0x07 = idle, 0x05 = F1, 0x03 = F2, 0x06 = TX bar

On Windows/Linux, hidapi prepends the report ID (0x01) as buf[0],
so the old parser read:

• buf[0] = 0x01 (constant) → encoder position never changed → no tuning
• buf[1] = sequence counter alternating 0x01/0x02 per knob tick →
  interpreted as button bitmask bit-0 toggling → ToggleMox fired on
  every knob movement

On macOS, hidapi strips the report ID, producing different but
equally broken behaviour. The user confirmed failure on both platforms.

Fix

• Strip report ID byte when buf[0] == 0x01 (Windows/Linux) before
  parsing; otherwise use buf directly (macOS).
• Read direction from data[2]; emit HidEvent::Rotate only on
  seq == 0x01 (first report of each pair) — the RC-28 sends two
  reports per detent, so without this guard each click tunes two steps.
• Read button state from data[4] as an absolute enum, not a bitmask.
• Expand from 2 to 3 buttons: F1 (button 1), F2 (button 2),
  TX bar (button 3).
• Fix reportSize(): 64 → 32.

Raw HID captures and platform confirmation provided by @bobbyrmanson in
the issue thread. Root cause analysis from AetherClaude (2026-05-01)
validated against source and independently confirmed here.

Testing

• Knob CW tunes up by one step per detent (no double-stepping)
• Knob CCW tunes down by one step per detent
• F1 press/release fires button 1 action
• F2 press/release fires button 2 action
• TX bar press/release fires button 3 action
• No spurious transmit toggles on knob movement
• Works on macOS (report ID stripped path)
• Works on Windows/Linux (report ID included path)

@bobbyrmanson has offered to test a patched build on macOS and Windows 11.

Fixes #1896

[aethersdr-agent]

Thanks @NF0T — solid bit of detective work. The byte-layout table in the description matches the diff exactly and the failure modes you describe (buf[0]==0x01 constant → no tuning, buf[1] alternating → spurious ToggleMox) drop out cleanly from the old code, so the root-cause story is convincing. Scope is clean: only the two files claimed in the title.

One substantive concern, plus a couple of smaller ones.

The buf[0] == 0x01 heuristic collides with macOS seq=0x01 reports

const uint8_t* data = (buf[0] == 0x01) ? buf + 1 : buf;

On Windows/Linux, buf[0] is the report ID (always 0x01) — strip is correct.
On macOS (report ID stripped by hidapi), buf[0] is the seq counter, which is 0x01 on the first report of every detent. So on macOS:

• seq=0x01 report: parser sees buf[0]==0x01, incorrectly strips a byte. data[0] becomes the unused byte (0x00), data[2] becomes the byte after direction (0x00). The seq == 0x01 rotation guard fails, no rotate event fires.
• seq=0x02 report: parser correctly doesn't strip. data[0]==0x02, but the guard requires seq == 0x01, so no rotate event fires.

Net effect: rotation never fires on macOS with this fix — the exact platform the PR description calls out as broken. Buttons would only work half the time (on seq=0x02 reports), since the button state byte position shifts on the falsely-stripped seq=0x01 reports.

Robust disambiguation can use buf[1]: on Windows/Linux buf[1] is the seq counter (0x01 or 0x02); on macOS buf[1] is the unused byte (0x00). So:

const bool hasReportId = (buf[0] == 0x01) && (buf[1] == 0x01 || buf[1] == 0x02);
const uint8_t* data = hasReportId ? buf + 1 : buf;

That cleanly distinguishes "report ID 0x01 followed by seq 0x01/0x02" from "seq 0x01 followed by unused 0x00" and works without platform #ifdefs. Worth confirming against a raw macOS capture from @bobbyrmanson before merging.

Minor — reportSize() = 32 truncates the report ID byte on Windows/Linux

HidEncoderManager calls hid_read(device, buf, parser->reportSize()). On Windows/Linux the wire report is 33 bytes (1 report ID + 32 data), so capping at 32 means we lose the last data byte. The parser only reads bytes 0–4 so this doesn't matter functionally today, but if anyone ever adds a check on a later byte it will silently fail on Windows/Linux but not macOS. Consider reportSize() = 33 — it lets you use len to detect the platform path unambiguously (33 = report-ID prefixed, 32 = stripped) and dodges the buf[0]/buf[1] heuristic entirely.

Small — m_prevButtonState{0x07} first-report behaviour

Initialising to 0x07 (idle) is correct for the common case. If AetherSDR starts with a button physically held, the first non-0x07 report will fire a press event — which is the right behaviour, just worth noting in case anyone wonders why a stuck button generates an event on startup.

Looks good

• Removing the encoder-wrap math and the m_firstReport gate is correct given the new format isn't a counter.
• len < 6 minimum covers data[4] access in both strip / no-strip cases — defensive and right.
• qCDebug/qCWarning aren't relevant here since the parser is silent by design; the manager logs disconnect, which is the right boundary.

Happy to re-review once the macOS path is sorted. The Windows/Linux side of this fix looks right and the description's analysis is convincing — please get @bobbyrmanson to confirm CW/CCW rotation works on macOS before merging.