hondavert-dev/docs/FIX_2026-10-12.md
metatroncubeswdev 9a56baef61 Rework BT poller and S300 decoding after client feedback
Client reported crashes, wrong/frozen data and that the app floods the
adapter MCU. See docs/FIX_2026-10-12.md for the full analysis.

- Poller: strictly one request outstanding; next request only after a valid
  128-byte reply or a 500 ms timeout, then a quiet period + buffer flush.
  Default 100 ms (10 Hz), minimum 80 ms. DTC clear/probe share the same lock.
- BtNotifier: filter the adapter's default (no-ECU-data) frame instead of
  holding last values; EcuDataStatus + status banner; auto-reconnect on link
  loss, stalls or long default-only runs (rate-limited). A reconnect still in
  flight when the user disconnects is discarded.
- S300 offsets corrected by column counting (trims, PA, ERR00-03, Eth, SW5,
  AF); INJ/AIN little-endian; fan = SW5 FANC only. Default-frame detection
  requires bytes 0-16 and 32-36.
- KPro RPM/MAP2/AIN little-endian (RPM still needs a bench check).
- Crash hardening: zone/Flutter/platform error handlers to an in-app log
  (copyable from Debug); permissions requested before listing devices.
- Remove temporary debug prints; graph redraws live; polling options
  100-300 ms, persisted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-12 21:02:54 -04:00

210 lines
11 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# HV BT Dashboard — Fix pass 2026-10-12 (client feedback round)
**Status:** `flutter analyze` (0 errors) and `flutter test` pass. APKs not yet
built; the bench checklist in §6.2 must pass before anything is sent to the
client.
**History:** the earlier uncommitted work and this pass are two commits on top
of `77a47eb`: first the earlier work (fuel pump, ECT/IAT/BAT, ECU-type
persistence), then this rework. To see this pass alone: `git show HEAD~1..HEAD`
(or `git diff HEAD~2 HEAD~1` for the earlier work). The original line-ending-heavy
snapshot commit is kept on branch `backup/snapshot-fb87e79`.
Review follow-ups applied in this pass: a reconnect in flight is discarded if
the user disconnects meanwhile; default-frame detection also requires bytes
32-36; the friendly error widget applies to release builds only.
---
## 1. Client feedback
1. Phone 1 — app crashes (all permissions allowed).
2. Phone 2 — connects to S300, shows nothing correct.
3. Phone 3 — some data correct, some obviously wrong; RPM gauge steps and freezes.
4. "Too many requests from the app — use a timeout for the answer before sending a
new request; it affects our device MCU usage."
## 2. Root causes found
### 2.1 "Template frames" are the adapter's DEFAULT frame
The frozen frame the team has been working around (bytes 0-16
`1B 00 14 00 00 FF FF F2 03 18 00 00 10 00 00 00 C4`, bytes 32-36
`93 A0 A0 DC 7D`) is **byte-for-byte the example row printed in
`hvbt_protocol.pdf`**. It is the adapter's built-in default buffer, returned when
it has no fresh ECU data. Combined with the client's remark about MCU load, the
most likely cause is that the app asks again before the adapter has finished
reading the ECU. Holding "last good values" during these frames is what made the
RPM gauge freeze and then jump, and a session that only ever gets default frames
(seen live in HANDOFF.md) is exactly phone 2.
### 2.2 The poller flooded the adapter
Old `bt_poller.dart`: a 50 ms periodic timer, plus an *extra* request whenever a
reply took longer than 200 ms (no waiting for the outstanding one), plus a request
that was never cleared after a bad-checksum reply (re-fired after 200 ms). Settings
even offered 20 ms (50 req/s).
### 2.3 Wrong byte offsets
The PDF's column labels (`+26 +27 +28 +29 +2A … +2F +30 +31`) are **not hex**.
Real offsets are plain column counting 0..127. Proof: the four fields already
confirmed live (Gear 27, ECT 33, IAT 34, BAT 36) all match column counting, and
the default frame decodes to sensible defaults with it (MAP 101.0 kPa, ECT/IAT
20 °C, BAT 12.0 V, trims 0 %, gear 1). The code read several fields as hex:
| Field | Old byte | New byte | Old symptom |
|---|---|---|---|
| Short trim | 41 (0x29) | 29 | showed ethanol byte |
| Long trim | 43 (0x2B) | 31 | wrong |
| PA (baro) | 44 (0x2C) | 32 | wrong |
| ERR00–ERR03 (DTCs) | 49–52 | 37–40 | phantom DTCs (e.g. SW6=0x60 → P0505/P0350) |
| Ethanol | 56 (0x38) | 41 | wrong |
| SW5 (FANC) | 67 (0x43) | 49 | always 0 (reserved area) |
| AF | 52 (0x34) | 55 | wrong |
Plus byte-order: INJ and AIN were read big-endian; the adapter is little-endian
(RPM/VSS/MAP confirmed live; KPro table labels MAP2 / AFCMD as L/H pairs).
### 2.4 Phone 1 crash — not proven yet
A Dart exception cannot close a Flutter app, so it is a native crash. Main
suspects: `flutter_bluetooth_serial 0.4.0` (unmaintained since ~2021, predates
Android 12 Bluetooth permissions, has a known "Reply already submitted" crash on a
failed connect) and the old picker screen calling `getBondedDevices` before the
Bluetooth permission was granted. **Need from client: phone model + Android
version, and a logcat of the crash.**
---
## 3. Changes, file by file
### Bluetooth / polling
- **`lib/core/bluetooth/bt_poller.dart` — rewritten.**
- Exactly one request outstanding. Next request only after a complete,
checksum-valid 128-byte reply **or** a 500 ms timeout.
- After a timeout: 150 ms quiet period + receive buffer flush, so a late reply
is never taken as the answer to the next request.
- Pacing: one request per polling interval (default **100 ms = 10 Hz**, minimum
80 ms enforced), never less than 20 ms after a reply.
- Frame sync now requires the 2-byte header (`1B 00` S300 / `1B 01` KPro) + NEG8.
- 6 consecutive timeouts → `onLinkStalled` callback.
- DTC-reset and protocol-probe commands go through the same lock, so they never
overlap a datalog request (`sendCommandAwaitReply`, `sendCommandCollect`).
- Stats: requestsSent, validFrames, droppedFrames, timeouts, lastResponseTime.
- **`lib/core/bluetooth/bt_service.dart`** — connect guarded against double
attempts + 15 s timeout; write throws when not connected (so the poller counts
it); close wrapped in timeout/try; logging.
- **`lib/core/providers/bt_provider.dart` — rewritten.**
- New `BtStatus.reconnecting`; new `EcuDataStatus` (none / live /
adapterDefaults / noResponse).
- **Adapter default frames are filtered out here** — they never reach the
gauges, graph or datalog; they are counted and shown on the Debug screen.
- Auto-reconnect: on BT link loss (up to 5 attempts, 1-5 s backoff); on 6
consecutive timeouts or >8 s of default-only frames, at most once per 45 s.
- Settings changes (poll interval) apply to a running poller.
- `clearDtcs()` / `probeOfficialProtocol()` reimplemented via the poller lock;
same return types as before.
### Decoding
- **`lib/core/protocol/s300_parser.dart`** — offsets/formulas per §2.3; full offset
map in the file header; `isS300DefaultFrame()` (old name
`isS300TemplateFrame()` kept as an alias); fan output = SW5 bit0 FANC only
(SW2 bit7 is ALTC = alternator control).
- **`lib/core/protocol/kpro_parser.dart`** — RPM, MAP2, AIN read little-endian.
⚠ KPro RPM endianness must be checked on the bench (2000 rpm must read 2000).
- **`lib/core/providers/sensor_provider.dart`** — removed hold-last-value logic and
the TEMP debug `print`s; decode errors go to the app log.
- **`lib/core/models/sensor_defs.dart`** — O2, AFR, AIN0-7 shown as `raw` (scales
not verified) instead of fake V/λ ranges.
### UI / robustness
- **`lib/main.dart`** — `runZonedGuarded` + `FlutterError.onError` +
`PlatformDispatcher.onError` → app log; friendly error widget.
- **`lib/core/logging/app_log.dart` (new)** — 400-line in-memory log.
- **`lib/ui/screens/debug/raw_frame_debug_screen.dart`** — "Copy app log" button;
new link stats (requests, live vs default frames, timeouts, reply time,
reconnects); raw-candidate rows use the corrected offsets.
- **`lib/ui/widgets/data_status_banner.dart` (new)** + `main_nav.dart` — banner
under the app bar when data is not live ("Adapter connected, but no data from
the ECU", "Adapter is not responding", "Reconnecting…").
- **`lib/ui/widgets/bt_status_chip.dart`** — reconnecting state.
- **`lib/ui/screens/bt_picker/bt_picker_screen.dart`** — requests permissions
**before** touching the paired-device list; only BLUETOOTH_CONNECT is required
(scan/location optional); try/catch around plugin calls; double-tap guard.
- **`lib/ui/screens/dashboard/widgets/analog_gauge.dart`** — 110 ms linear needle
animation (was 150 ms ease-out) so the needle glides between 10 Hz readings.
- **`lib/core/providers/settings_provider.dart` + settings screen** — interval
options 100/150/200/300 ms (removed 20 and 50), default 100, persisted.
- **`lib/core/providers/sensor_history_provider.dart`** — graph now actually
redraws live (`state = state` was swallowed by StateNotifier).
### Tests
- `test/s300_parser_test.dart` — INJ builder LE; new tests for trims, DTC bytes,
no phantom DTC from SW6, FANC, Eth/AF/PA offsets, INJ/AIN LE, and decoding the
full PDF default frame.
- `test/kpro_parser_test.dart` — builder writes RPM and MAP2 little-endian.
---
## 4. Still UNVERIFIED (need one bench check against the USB tool)
| Item | Current assumption |
|---|---|
| INJ scale | raw (LE) / 1000 → ms (assumes µs) |
| IGN formula | PDF `(raw+120)/2` — one earlier live check disagreed |
| PA formula | `raw*0.716 - 5` kPa (default 0x93 → 100.3 kPa) |
| O2, AF, AIN | shown raw |
| KPro RPM byte order | little-endian |
| DTC clear ack | sent between polls now; ack still unconfirmed |
## 5. Questions for the client (device maker)
1. Max response time of the adapter to `1B 00 E5`? (sets our 500 ms timeout)
2. Recommended request interval? (we use 100 ms; DLrate byte = 0x14 = 20 — is that
20 Hz?)
3. Confirm: the PDF example frame = "no fresh ECU data" default frame?
4. Phone models + Android versions; logcat from the crashing phone.
---
## 6. Verification checklist
### 6.1 Static (must all pass before building)
- [ ] `flutter pub get`
- [ ] `flutter analyze` — no errors (fix any compile errors in changed files only)
- [ ] `flutter test` — all tests green
- [ ] `flutter build apk --release` (and `--debug` for the bench)
- [ ] Release build: if it crashes on connect but debug doesn't, add R8 keep rule
`-keep class io.github.edufolly.flutterbluetoothserial.** { *; }`
### 6.2 Bench (S300 + USB reference tool)
- [ ] Debug screen: *Requests sent* rises ≈10/s, never faster; *Timeouts* ≈ 0.
- [ ] *Adapter default frames* stays ≈ 0 while the ECU is on (main proof the
flooding was the cause). If it is still high → report to client with log.
- [ ] RPM sweep up/down: needle moves smoothly, no freezes/steps.
- [ ] Compare against USB tool: RPM, MAP, TPS, ECT, IAT, BAT, gear (already
confirmed) + trims, PA, INJ, IGN, ETH (new).
- [ ] DTC tab: no phantom codes; a forced fault shows the right code.
- [ ] Unplug adapter power → banner "Reconnecting…", then error; replug +
reconnect works.
- [ ] Ignition off with adapter powered → banner "no data from the ECU".
- [ ] Debug → "Copy app log" pastes a readable log.
- [ ] Fresh install on Android 12+: permission prompt first, device list after,
no crash. Deny "Nearby devices" → clear message, no crash.
---
## 7. Prompt for Claude Code (paste as-is)
```
Read docs/FIX_2026-10-12.md fully. Then:
1. Run `git diff fb87e79 --stat` and review every changed file against the doc.
Flag anything in the diff that the doc does not describe, and any logic bug
(especially in lib/core/bluetooth/bt_poller.dart and
lib/core/providers/bt_provider.dart: one request outstanding at a time,
timeout handling, reconnect loop, stream/timer cleanup on disconnect).
2. Check the byte-offset table in lib/core/protocol/s300_parser.dart against
hvbt_protocol.pdf using plain column counting (0..127), as the doc explains.
3. Run `flutter pub get`, `flutter analyze`, `flutter test`. Fix compile/analyzer
errors and failing tests with minimal changes; do not change protocol logic
or offsets without telling me why.
4. Build: `flutter build apk --debug` and `flutter build apk --release`.
5. Report: what you fixed, analyzer/test results, APK paths, and any concerns.
Do not commit until I say so.
```