Repository navigation
Absturzberichte, 16-MB-Layout fürs Display, Loop-Watchdog (2.1.0-beta.3) - #28
Conversation
- partitions/sender_4mb.csv: app slots and NVS unchanged, 64 KB coredump, 256 KB log. partitions/display_16mb.csv: 2 x 4 MB app, 128 KB coredump, 1 MB log, rest reserved. NVS stays at 0x9000 in both -> credentials survive. - lib/diagnostics/SystemHealth: reset reason + core dump summary at boot, GET /api/coredump, POST /api/coredump/clear, shown in both web UIs. - Loop task watchdog (30 s, panic -> core dump -> reboot; with boot validation a hanging new firmware is rolled back). Fed during downloads and web uploads. - flash.sh layout: new partition table + firmware without erasing NVS. - Version V2.1.0-beta.3. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0380eadcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| std::size_t received = 0; | ||
| uint32_t lastDataMs = millis(); | ||
| while (http.connected() && (contentLength < 0 || received < static_cast<std::size_t>(contentLength))) { | ||
| SystemHealth::feed(); // a download takes longer than the loop watchdog allows |
There was a problem hiding this comment.
On a slow or stalled HTTPS connection, the device can now panic and reboot before reaching this feed: downloadInto() first calls the synchronous openGet() with a 60-second HTTP timeout, while the new loop watchdog expires after 30 seconds. Automatic manifest checks also call openGet() without feeding the watchdog; the pinned Arduino NetworkClientSecure implementation defaults to a 120-second TLS handshake timeout. Consequently, an ordinary network failure can interrupt sender telemetry or display operation instead of returning an update error. Bound the blocking connection/handshake phases below the watchdog deadline, or arrange watchdog servicing throughout those phases as well as the download loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Two unresolved moderate performance defects remain, including an O(n²) core-dump download and a full image re-verification on every 2-second status poll, which still need correcting.
3 open issues
What changed in this PR
This PR adds crash diagnostics and resiliency to the CANOBD2Reader firmware (sender + display) and reworks the flash partitioning. A new lib/diagnostics/SystemHealth module reports the reset reason and any stored ESP32 core dump at boot, exposes the dump for download/clearing via the web UIs and /api/coredump[/clear], and installs a 30 s loop task watchdog that panics (→ core dump → reboot, with BootValidation rollback) if loop() hangs. Long blocking operations (OTA upload, GitHub download) now feed the watchdog. Partition tables are moved to per-device CSVs (sender_4mb.csv, display_16mb.csv) with dedicated coredump regions, and the provisioning tooling gains a flash.sh layout command to re-flash the table + firmware without erasing NVS credentials. Version is bumped to V2.1.0-beta.3.
Changes:
- New
SystemHealthdiagnostics module: reset reason, core dump download/erase, loop watchdog; wired into sender/display boot and the shared management web server. - Per-device partition tables with coredump partitions;
make_provisioning_package.pynow derives offsets per device and generates a non-erasingflash.sh layoutcommand. - Web UIs, docs (ARCHITECTURE/HARDWARE/SECURITY/PROVISIONING/DEVELOPMENT), version strings, and
platformio.iniupdated accordingly.
| File | Description |
|---|---|
lib/diagnostics/SystemHealth.h/.cpp |
New crash-diagnostics + watchdog module (reset reason, core dump access, TWDT). |
lib/web/ManagementServer.cpp/.h |
Adds /api/coredump[/clear] routes, status JSON fields, feeds watchdog during upload. |
lib/update/GitHubUpdateClient.cpp |
Feeds the watchdog during long firmware downloads. |
src/sender/SenderApp.cpp, src/display/DisplayApp.cpp |
Log boot summary and start the watchdog. |
src/sender/WebConsoleHandler.cpp, src/display/DisplayWeb.cpp |
Show reset reason + crash-report download/clear in the UIs. |
partitions/sender_4mb.csv, partitions/display_16mb.csv |
New device-specific layouts with coredump regions; ota_4mb.csv removed. |
platformio.ini |
Per-env partition tables, diagnostics include/src-filter wiring. |
scripts/make_provisioning_package.py |
Per-device offset parsing + flash.sh layout (non-erasing) command. |
VERSION.txt, README.md |
Version bump to V2.1.0-beta.3. |
docs/*.md |
Document crash analysis, layout change workflow, partition sizes, watchdog. |
I reviewed the partition math (both CSVs fit 4 MB/16 MB exactly), the generated flash.sh bash (associative arrays, write_images, sed help range), the per-env PlatformIO wiring (dev envs inherit via extends, native env filter updated), and docs consistency — all correct. The main findings are a performance issue in the core-dump download path and a couple of minor maintainability/performance notes, recorded as comments.
Review effort: Balanced
| const CoreDumpInfo info = coreDump(); | ||
| if (!info.present || offset + length > info.size) return false; | ||
| return esp_flash_read(nullptr, buffer, info.address + offset, length) == ESP_OK; |
| field("ip", WiFi.softAPIP().toString()); | ||
| number("apClients", WiFi.softAPgetStationNum()); | ||
| number("apRestarts", apRestarts); | ||
| const SystemHealth::CoreDumpInfo dump = SystemHealth::coreDump(); |
| bool crashReset() { | ||
| #if defined(ARDUINO) | ||
| const esp_reset_reason_t reason = esp_reset_reason(); | ||
| return reason == ESP_RST_PANIC || reason == ESP_RST_INT_WDT || reason == ESP_RST_TASK_WDT || | ||
| reason == ESP_RST_WDT || reason == ESP_RST_BROWNOUT; | ||
| #else | ||
| return false; | ||
| #endif | ||
| } |
…log fixes Findings from the first vehicle test (VW Tiguan): - Functional OBD requests are answered by several ECUs; the first answer won. Supported-PID masks therefore flipped every refresh (oil, intake, throttle shown, then 'n.v.') and values could come from the wrong ECU. Mode 01/09 now only accept the engine ECU (0x7E8). - Low cards: 8 px header 'Label unit' so it no longer overlaps the value. - 'Warte auf Sender' hint cleared on the first packet. - Voltage limits for VW energy management (warn 15.5 V, critical 16 V). - scripts/read_device_logs.py: read log file system and crash report over USB (local or via SSH), unpack with mkspiffs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Polling was ~5 requests/s (fast values every ~1.2 s, slow ~9 s): every ISO-TP/OBD trace line was written to the flash log. Trace now goes to the flash log only with PersistTraceToDiagnosticLog; serial debug off. - Display never slept: the Sleep command existed for one 10 ms update and was practically never sent; and the last RPM before ignition-off kept the car 'running'. Command is sent immediately, the display follows the DisplaySleep state, RPM/speed/load only count while OBD answers are fresh (regression test). - No CAN requests while the engine ECU is silent unless the board voltage shows a running alternator (>= 13 V): requests wake the VW gateway. - Fuel rate from MAF (PID 0x5E reported ~30 % of the real value); instant consumption cleared below 5 km/h; MAF polled fast, coolant/ECU voltage slow. - Thresholds for VW: coolant warn 110 / critical 118 C, oil 130 / 140 C. - Voltage divider calibrated against PID 0x42 (4.81 -> 4.62). - Boot log MAC read from eFuse; read_device_logs.py: 115200 baud for CH340. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Zusammenfassung
0x9000, Zugangsdaten bleiben erhalten):sender_4mb.csv: App-Slots unverändert, neu 64 KB Coredump, Log 320 → 256 KBdisplay_16mb.csv: 2 × 4 MB App (vorher 1,81 MB), 128 KB Coredump, 1 MB Log, Rest Reserve/api/coredump, Löschen über/api/coredump/clear, Anzeige in beiden Weboberflächen. Auswertung siehedocs/DEVELOPMENT.md.flash.sh layout: neue Partitionstabelle plus Firmware schreiben, ohne den Flash zu löschen.V2.1.0-beta.3.Tests
senderunddisplay; Display belegt jetzt 35 % eines 4-MB-Slotsflash.sh layoutfolgt (USB-Termin am kalipi)🤖 Generated with Claude Code