A fork of https://github.com/crosspoint-reader/crosspoint-reader
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

feat: aiagent context definition (#922)

## Summary

* As many of us have experienced, the use of AI coding agents (Claude,
Cursor, Copilot) in our daily workflows is no longer a hypothetical—it
is a reality. While these tools can significantly accelerate
development, they also pose a unique risk to a project like CrossPoint
Reader, where our hardware constraints (ESP32-C3 with ~380KB RAM) are
extremely tight.

* AI models often "hallucinate" APIs or suggest high-level C++ patterns
(like std::string or heavy heap usage) that are detrimental to our
memory-constrained environment.

* To address this, I am proposing the introduction of an AI Agent
Guidance File (.skills/SKILL.md recognized by many AI systems). This
file acts as a "Constitutional Document" for AI agents, forcing them to
adhere to our specific engineering rigors before they generate a single
line of code.

## Additional Context

---

### AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing,
please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? _**PARTIALLY**_ Asked
several Ai systems about their needs, added my own 5 cents of nuisances
I faced

authored by

jpirnay and committed by
GitHub
6b64a0a2 1abe307f

+874
+1
.github/skills/crosspoint-reader.md
··· 1 + ../../.skills/SKILL.md
+872
.skills/SKILL.md
··· 1 + # CrossPoint Reader Development Guide 2 + 3 + Project: Open-source e-reader firmware for Xteink X4 (ESP32-C3) 4 + Mission: Provide a lightweight, high-performance reading experience focused on EPUB rendering on constrained hardware. 5 + 6 + ## AI Agent Identity and Cognitive Rules 7 + * Role: Senior Embedded Systems Engineer (ESP-IDF/Arduino-ESP32 specialized). 8 + * Primary Constraint: 380KB RAM is the hard ceiling. Stability is non-negotiable. 9 + * Evidence-Based Reasoning: Before proposing a change, you MUST cite the specific file path and line numbers that justify the modification. 10 + * Anti-Hallucination: Do not assume the existence of libraries or ESP-IDF functions. If you are unsure of an API's availability for the ESP32-C3 RISC-V target, check the open-x4-sdk or official docs first. 11 + * No Unfounded Claims: Do not claim performance gains or memory savings without explaining the technical mechanism (e.g., DRAM vs IRAM usage). 12 + * Resource Justification: You must justify any new heap allocation (new, malloc, std::vector) or explain why a stack/static alternative was rejected. 13 + * Verification: After suggesting a fix, instruct the user on how to verify it (e.g., monitoring heap via Serial or checking a specific cache file). 14 + --- 15 + 16 + ## Development Environment Awareness 17 + 18 + **CRITICAL**: Detect the host platform at session start to choose appropriate tools and commands. 19 + 20 + ### Platform Detection 21 + ```bash 22 + # Detect platform (run once per session) 23 + uname -s 24 + # Returns: MINGW64_NT-* (Windows Git Bash), Linux, Darwin (macOS) 25 + ``` 26 + 27 + **Detection Required**: Run `uname -s` at session start to determine platform 28 + 29 + ### Platform-Specific Behaviors 30 + - **Windows (Git Bash)**: Unix commands, `C:\` paths in Windows but `/` in bash, limited glob (use `find`+`xargs`) 31 + - **Linux/WSL**: Full bash, Unix paths, native glob support 32 + 33 + **Cross-Platform Code Formatting**: 34 + ```bash 35 + find src -name "*.cpp" -o -name "*.h" | xargs clang-format -i 36 + ``` 37 + 38 + --- 39 + 40 + ## Platform and Hardware Constraints 41 + 42 + ### Hardware Specs 43 + * MCU: ESP32-C3 (Single-core RISC-V @ 160MHz) 44 + * RAM: ~380KB usable (VERY LIMITED - primary project constraint) 45 + * **NO PSRAM**: ESP32-C3 has no PSRAM capability (unlike ESP32-S3) 46 + * **Single Buffer Mode**: Only ONE 48KB framebuffer (not double-buffered) 47 + * Flash: 16MB (Instruction storage and static data) 48 + * Display: 800x480 E-Ink (Slow refresh, monochrome, 1-2s full update) 49 + * Framebuffer: 48,000 bytes (800 × 480 ÷ 8) 50 + * Storage: SD Card (Used for books and aggressive caching) 51 + 52 + ### The Resource Protocol 53 + 1. Stack Safety: Limit local function variables to < 256 bytes. The ESP32-C3 default stack is small; use std::unique_ptr or static pools for larger buffers. 54 + 2. Heap Fragmentation: Avoid repeated new/delete in loops. Allocate buffers once during onEnter() and reuse them. 55 + 3. Flash Persistence: Large constant data (UI strings, lookup tables) MUST be marked static const to stay in Flash (Instruction Bus), freeing DRAM. 56 + 4. String Policy: Prohibit std::string and Arduino String in hot paths. Use std::string_view for read-only access and snprintf with fixed char[] buffers for construction. 57 + 5. UI Strings: All user-facing text must use the `tr()` macro (e.g., `tr(STR_LOADING)`) for i18n support. Never hardcode UI strings directly. For the avoidance of doubt, logging messages (LOG_DBG/LOG_ERR) can be hardcoded, but user-facing text must use `tr()`. 58 + 6. `constexpr` First: Compile-time constants and lookup tables must be `constexpr`, not just `static const`. This moves computation to compile time, enables dead-branch elimination, and guarantees flash placement. Use `static constexpr` for class-level constants. 59 + 7. `std::vector` Pre-allocation: Always call `.reserve(N)` before any `push_back()` loop. Each growth event allocates a new block (2×), copies all elements, then frees the old one — three heap operations that fragment DRAM. When the final size is unknown, estimate conservatively. 60 + 8. SPIFFS Write Throttling: Never write a settings file on every user interaction. Guard all writes with a value-change check (`if (newVal == _current) return;`). Progress saves during reading must be debounced — write on activity exit or every N page turns, not on every turn. SPIFFS sectors have a finite erase cycle limit. 61 + 62 + --- 63 + 64 + ## Project Architecture 65 + 66 + ### Build System: PlatformIO 67 + 68 + **PlatformIO is BOTH a VS Code extension AND a CLI tool**: 69 + 70 + 1. **VS Code Extension** (Recommended): 71 + * Extension ID: `platformio.platformio-ide` (see `.vscode/extensions.json`) 72 + * Provides: Toolbar buttons, IntelliSense, integrated build/upload/monitor 73 + * Configuration: `.vscode/c_cpp_properties.json`, `.vscode/tasks.json` 74 + * Usage: Click Build (✓), Upload (→), or Monitor (🔌) buttons 75 + 76 + 2. **CLI Tool** (`pio` command): 77 + * **Installation**: Python package (typically `pip install platformio`) 78 + * **Windows Location**: `C:\Users\<user>\AppData\Local\Programs\Python\Python3xx\Scripts\pio.exe` 79 + * **Verify**: `which pio` (Git Bash) or `where.exe pio` (cmd) 80 + * **Usage**: `pio run`, `pio run -t upload`, etc. 81 + 82 + **Configuration Files**: 83 + * `platformio.ini`: Main build configuration (committed to git) 84 + * `platformio.local.ini`: Local overrides (gitignored, create if needed) 85 + * `partitions.csv`: ESP32 flash partition layout 86 + 87 + ### Build Environment 88 + * **Standard**: C++20 (`-std=c++2a`). No Exceptions, No RTTI. 89 + * **Logging**: ALWAYS use `LOG_INF`, `LOG_DBG`, or `LOG_ERR` from `Logging.h`. Raw Serial output is deprecated. 90 + * **Environments** (in `platformio.ini`): 91 + * `default`: Development (LOG_LEVEL=2, serial enabled) 92 + * `gh_release`: Production (LOG_LEVEL=0) 93 + * `gh_release_rc`: Release candidate (LOG_LEVEL=1) 94 + * `slim`: Minimal build (no serial logging) 95 + 96 + ### Critical Build Flags 97 + These flags in `platformio.ini` fundamentally affect firmware behavior: 98 + 99 + ```cpp 100 + -DEINK_DISPLAY_SINGLE_BUFFER_MODE=1 // Single framebuffer (saves 48KB RAM!) 101 + -DARDUINO_USB_MODE=1 // Enable USB CDC 102 + -DARDUINO_USB_CDC_ON_BOOT=1 // Serial available immediately at boot 103 + -DXML_CONTEXT_BYTES=1024 // XML parser memory limit (EPUB parsing) 104 + -DUSE_UTF8_LONG_NAMES=1 // SD card long filename support 105 + -DMINIZ_NO_ZLIB_COMPATIBLE_NAMES=1 // Avoid zlib name conflicts 106 + -DXML_GE=0 // Disable XML general entities (security) 107 + ``` 108 + 109 + **SINGLE_BUFFER_MODE implications**: 110 + - Only ONE framebuffer exists (not double-buffered) 111 + - Grayscale rendering requires temporary buffer allocation (`renderer.storeBwBuffer()`) 112 + - Must call `renderer.restoreBwBuffer()` to free temporary buffers 113 + - See [lib/GfxRenderer/GfxRenderer.cpp:439-440](lib/GfxRenderer/GfxRenderer.cpp) for malloc usage 114 + 115 + ### Directory Structure 116 + * lib/: Internal libraries (Epub engine, GfxRenderer, UITheme, I18n) 117 + * lib/hal/: Hardware Abstraction Layer (HalDisplay, HalGPIO, HalStorage) 118 + * lib/I18n/: Internationalization (translations in `translations/*.yaml`, generated string tables) 119 + * src/activities/: UI logic using the Activity Lifecycle (onEnter, loop, onExit) 120 + * open-x4-sdk/: Low-level SDK (EInkDisplay, InputManager, BatteryMonitor, SDCardManager) 121 + * .crosspoint/: SD-based binary cache for EPUB metadata and pre-rendered layout sections 122 + 123 + ### Hardware Abstraction Layer (HAL) 124 + 125 + **CRITICAL**: Always use HAL classes, NOT SDK classes directly. 126 + 127 + | HAL Class | Wraps SDK Class | Purpose | Singleton Macro | 128 + |-----------|----------------|---------|-----------------| 129 + | `HalDisplay` | `EInkDisplay` | E-ink display control | *(none)* | 130 + | `HalGPIO` | `InputManager` | Button input handling | *(none)* | 131 + | `HalStorage` | `SDCardManager` | SD card file I/O | `Storage` | 132 + 133 + **Location**: [lib/hal/](lib/hal/) 134 + 135 + **Why HAL?** 136 + - Provides consistent error logging per module 137 + - Abstracts SDK implementation details 138 + - Centralizes resource management 139 + 140 + **Example - HalStorage**: 141 + ```cpp 142 + #include <HalStorage.h> 143 + 144 + // Use Storage singleton (defined via macro) 145 + FsFile file; 146 + if (Storage.openFileForRead("MODULE", "/path/to/file.bin", file)) { 147 + // Read from file 148 + file.close(); // Explicit close required 149 + } 150 + ``` 151 + 152 + **Usage**: See example above. Uses `FsFile` (SdFat), NOT Arduino `File`. 153 + 154 + --- 155 + 156 + ## Coding Standards 157 + 158 + ### Naming Conventions 159 + * Classes: PascalCase (e.g., EpubReaderActivity) 160 + * Methods/Variables: camelCase (e.g., renderPage()) 161 + * Constants: UPPER_SNAKE_CASE (e.g., MAX_BUFFER_SIZE) 162 + * Private Members: memberVariable (no prefix) 163 + * File Names: Match Class names (e.g., EpubReaderActivity.cpp) 164 + 165 + ### Header Guards 166 + * Use #pragma once for all header files. 167 + 168 + ### Memory Safety and RAII 169 + * Smart Pointers: Prefer std::unique_ptr. Avoid std::shared_ptr (unnecessary atomic overhead for a single-core RISC-V). 170 + * RAII: Use destructors for cleanup, but call file.close() or vTaskDelete() explicitly for deterministic resource release. 171 + 172 + ### ESP32-C3 Platform Pitfalls 173 + 174 + #### `std::string_view` and Null Termination 175 + `string_view` is *not* null-terminated. Passing `.data()` to any C-style API (`drawText`, `snprintf`, `strcmp`, SdFat file paths) is undefined behaviour when the view is a substring or a view of a non-null-terminated buffer. 176 + 177 + **Rule**: `string_view` is safe only when passing to C++ APIs that accept `string_view`. For any C API boundary, convert explicitly: 178 + ```cpp 179 + // WRONG - undefined behaviour if view is a substring: 180 + renderer.drawText(font, x, y, myView.data(), true); 181 + 182 + // CORRECT - guaranteed null-terminated: 183 + renderer.drawText(font, x, y, std::string(myView).c_str(), true); 184 + 185 + // CORRECT - for short strings, use a stack buffer: 186 + char buf[64]; 187 + snprintf(buf, sizeof(buf), "%.*s", (int)myView.size(), myView.data()); 188 + ``` 189 + 190 + #### `IRAM_ATTR` and Flash Cache Safety 191 + All code runs from flash via the instruction cache. During SPI flash operations (OTA write, SPIFFS commit, NVS update) the cache is briefly suspended. Any code that can execute during this window — ISRs in particular — must reside in IRAM or it will crash silently. 192 + 193 + ```cpp 194 + // ISR handler: must be in IRAM 195 + void IRAM_ATTR gpioISR() { ... } 196 + 197 + // Data accessed from IRAM_ATTR code: must be in DRAM, never a flash const 198 + static DRAM_ATTR uint32_t isrEventFlags = 0; 199 + ``` 200 + 201 + **Rules**: 202 + - All ISR handlers: `IRAM_ATTR` 203 + - Data read by `IRAM_ATTR` code: `DRAM_ATTR` (a flash-resident `static const` will fault) 204 + - Normal task code does **not** need `IRAM_ATTR` 205 + 206 + #### ISR vs Task Shared State 207 + `xSemaphoreTake()` (mutex) **cannot** be called from ISR context — it will crash. Use the correct primitive for each communication direction: 208 + 209 + | Direction | Correct primitive | 210 + |---|---| 211 + | ISR → task (data) | `xQueueSendFromISR()` + `portYIELD_FROM_ISR()` | 212 + | ISR → task (signal) | `xSemaphoreGiveFromISR()` + `portYIELD_FROM_ISR()` | 213 + | Task → task | `xSemaphoreTake()` / mutex | 214 + | Simple flag (single writer ISR) | `volatile bool` + `portENTER_CRITICAL_ISR()` | 215 + 216 + #### RISC-V Alignment 217 + ESP32-C3 faults on unaligned multi-byte loads. Never cast a `uint8_t*` buffer to a wider pointer type and dereference it directly. Use `memcpy` for any unaligned read: 218 + 219 + ```cpp 220 + // WRONG — faults if buf is not 4-byte aligned: 221 + uint32_t val = *reinterpret_cast<const uint32_t*>(buf); 222 + 223 + // CORRECT: 224 + uint32_t val; 225 + memcpy(&val, buf, sizeof(val)); 226 + ``` 227 + 228 + This applies to all cache deserialization code and any raw buffer-to-struct casting. `__attribute__((packed))` structs have the same hazard when accessed via member reference. 229 + 230 + #### Template and `std::function` Bloat 231 + Each template instantiation generates a separate binary copy. `std::function<void()>` adds ~2–4 KB per unique signature and heap-allocates its closure. Avoid both in library code and any path called from the render loop: 232 + 233 + ```cpp 234 + // Avoid — heap-allocating, large binary footprint: 235 + std::function<void()> callback; 236 + 237 + // Prefer — zero overhead: 238 + void (*callback)() = nullptr; 239 + 240 + // For member function + context (common activity callback pattern): 241 + struct Callback { void* ctx; void (*fn)(void*); }; 242 + ``` 243 + 244 + When a template is necessary, limit instantiations: use explicit template instantiation in a `.cpp` file to prevent the compiler from generating duplicates across translation units. 245 + 246 + --- 247 + 248 + ### Error Handling Philosophy 249 + 250 + **Source**: [src/main.cpp:132-143](src/main.cpp), [lib/GfxRenderer/GfxRenderer.cpp:10](lib/GfxRenderer/GfxRenderer.cpp) 251 + 252 + **Pattern Hierarchy**: 253 + 1. **LOG_ERR + return false** (90%): `LOG_ERR("MOD", "Failed: %s", reason); return false;` 254 + 2. **LOG_ERR + fallback**: `LOG_ERR("MOD", "Unavailable"); useDefault();` 255 + 3. **assert(false)**: Only for fatal "impossible" states (framebuffer missing) 256 + 4. **ESP.restart()**: Only for recovery (OTA complete) 257 + 258 + **Rules**: NO exceptions, NO abort(), ALWAYS log before error return 259 + 260 + ### Acceptable malloc/free Patterns 261 + 262 + **Source**: [src/activities/home/HomeActivity.cpp:166](src/activities/home/HomeActivity.cpp), [lib/GfxRenderer/GfxRenderer.cpp:439-440](lib/GfxRenderer/GfxRenderer.cpp) 263 + 264 + Despite "prefer stack allocation," malloc is acceptable for: 265 + 1. **Large temporary buffers** (> 256 bytes, won't fit on stack) 266 + 2. **One-time allocations** during activity initialization 267 + 3. **Bitmap rendering buffers** (variable size, used briefly) 268 + 269 + **Pattern**: 270 + ```cpp 271 + // Allocate 272 + auto* buffer = static_cast<uint8_t*>(malloc(bufferSize)); 273 + if (!buffer) { 274 + LOG_ERR("MODULE", "malloc failed: %d bytes", bufferSize); 275 + return false; // Handle allocation failure 276 + } 277 + 278 + // Use buffer 279 + processData(buffer, bufferSize); 280 + 281 + // Free immediately after use 282 + free(buffer); 283 + buffer = nullptr; 284 + ``` 285 + 286 + **Rules**: 287 + - **ALWAYS check for nullptr** after malloc 288 + - **Free immediately** after use (don't hold across multiple operations) 289 + - **Set to nullptr** after free (avoid use-after-free) 290 + - **Document size**: Comment why stack allocation was rejected 291 + 292 + **Examples in codebase**: 293 + - Cover image buffers: [HomeActivity.cpp:166](src/activities/home/HomeActivity.cpp#L166) 294 + - Text chunk buffers: [TxtReaderActivity.cpp:259](src/activities/reader/TxtReaderActivity.cpp#L259) 295 + - Bitmap rendering: [GfxRenderer.cpp:439-440](lib/GfxRenderer/GfxRenderer.cpp#L439-L440) 296 + - OTA update buffer: [OtaUpdater.cpp:40](src/network/OtaUpdater.cpp#L40) 297 + 298 + --- 299 + 300 + ## UI and Orientation Guidelines 301 + 302 + ### Orientation-Aware Logic 303 + * No Hardcoding: Never assume 800 or 480. Use renderer.getScreenWidth() and renderer.getScreenHeight(). 304 + * Viewable Area: Use renderer.getOrientedViewableTRBL() to stay within physical bezel margins. 305 + 306 + ### Logical Button Mapping 307 + 308 + **Source**: [src/MappedInputManager.cpp:20-55](src/MappedInputManager.cpp) 309 + 310 + Constraint: Physical button positions are fixed on hardware, but their logical functions change based on user settings and screen orientation. 311 + 312 + **Button Categories**: 313 + 1. **Physical Fixed** (Up/Down side buttons): 314 + - `Button::Up` → Always `HalGPIO::BTN_UP` 315 + - `Button::Down` → Always `HalGPIO::BTN_DOWN` 316 + 317 + 2. **User Remappable** (Front buttons): 318 + - `Button::Back` → Maps to `SETTINGS.frontButtonBack` (hardware index) 319 + - `Button::Confirm` → Maps to `SETTINGS.frontButtonConfirm` 320 + - `Button::Left` → Maps to `SETTINGS.frontButtonLeft` 321 + - `Button::Right` → Maps to `SETTINGS.frontButtonRight` 322 + 323 + 3. **Reader-Specific** (Page navigation with optional swap): 324 + - `Button::PageBack` → Uses side button (swappable via `SETTINGS.sideButtonLayout`) 325 + - `Button::PageForward` → Uses side button (swappable) 326 + 327 + **Implementation**: 328 + - Activities use **logical buttons** (e.g., `Button::Confirm`) 329 + - `MappedInputManager` translates to **physical hardware buttons** 330 + - User can remap front buttons in settings 331 + - Orientation changes handled separately by renderer coordinate transforms 332 + 333 + **Rule**: Always use `MappedInputManager::Button::*` enums, never raw `HalGPIO::BTN_*` indices (except in ButtonRemapActivity). 334 + 335 + ### UITheme (The GUI Macro) 336 + * Rule: All UI rendering must go through the GUI macro (UITheme). 337 + * Do not hardcode fonts, colors, or positioning. This ensures orientation-aware layout consistency. 338 + 339 + --- 340 + 341 + ## Common Patterns 342 + 343 + ### Singleton Access 344 + **Available Singletons**: 345 + ```cpp 346 + #define SETTINGS CrossPointSettings::getInstance() // User settings 347 + #define APP_STATE CrossPointState::getInstance() // Runtime state 348 + #define GUI UITheme::getInstance() // Current theme 349 + #define Storage HalStorage::getInstance() // SD card I/O 350 + #define I18N I18n::getInstance() // Internationalization 351 + ``` 352 + 353 + ### Activity Lifecycle and Memory Management 354 + 355 + **Source**: [src/main.cpp:132-143](src/main.cpp) 356 + 357 + **CRITICAL**: Activities are **heap-allocated** and **deleted on exit**. 358 + 359 + ```cpp 360 + // main.cpp navigation pattern 361 + void exitActivity() { 362 + if (currentActivity) { 363 + currentActivity->onExit(); 364 + delete currentActivity; // Activity deleted here! 365 + currentActivity = nullptr; 366 + } 367 + } 368 + 369 + void enterNewActivity(Activity* activity) { 370 + currentActivity = activity; // Heap-allocated activity 371 + currentActivity->onEnter(); 372 + } 373 + ``` 374 + 375 + **Memory Implications**: 376 + - Activity navigation = `delete` old activity + `new` create next activity 377 + - Any memory allocated in `onEnter()` MUST be freed in `onExit()` 378 + - FreeRTOS tasks MUST be deleted in `onExit()` before activity destruction 379 + - File handles MUST be closed in `onExit()` 380 + 381 + **Activity Pattern**: 382 + ```cpp 383 + void onEnter() { Activity::onEnter(); /* alloc: buffer, tasks */ render(); } 384 + void loop() { mappedInput.update(); /* handle input */ } 385 + void onExit() { /* free: vTaskDelete, free buffer, close files */ Activity::onExit(); } 386 + ``` 387 + 388 + **Critical**: Free resources in reverse order. Delete tasks BEFORE activity destruction. 389 + 390 + ### FreeRTOS Task Guidelines 391 + 392 + **Source**: [src/activities/util/KeyboardEntryActivity.cpp:45-50](src/activities/util/KeyboardEntryActivity.cpp) 393 + 394 + **Pattern**: See Activity Lifecycle above. `xTaskCreate(&taskTrampoline, "Name", stackSize, this, 1, &handle)` 395 + 396 + **Stack Sizing** (in BYTES, not words): 397 + - **2048**: Simple rendering (most activities) 398 + - **4096**: Network, EPUB parsing 399 + - Monitor: `uxTaskGetStackHighWaterMark()` if crashes 400 + 401 + **Rules**: Always `vTaskDelete()` in `onExit()` before destruction. Use mutex if shared state. 402 + 403 + ### Global Font Loading 404 + 405 + **Source**: [src/main.cpp:40-115](src/main.cpp) 406 + 407 + **All fonts are loaded as global static objects** at firmware startup: 408 + - Bookerly: 12, 14, 16, 18pt (4 styles each: regular, bold, italic, bold-italic) 409 + - Noto Sans: 12, 14, 16, 18pt (4 styles each) 410 + - OpenDyslexic: 8, 10, 12, 14pt (4 styles each) 411 + - Ubuntu UI fonts: 10, 12pt (2 styles) 412 + 413 + **Total**: ~80+ global `EpdFont` and `EpdFontFamily` objects 414 + 415 + **Compilation Flag**: 416 + ```cpp 417 + #ifndef OMIT_FONTS 418 + // Most fonts loaded here 419 + #endif 420 + ``` 421 + 422 + **Implications**: 423 + - Fonts stored in **Flash** (marked as `static const` in `lib/EpdFont/builtinFonts/`) 424 + - Font rendering data cached in **DRAM** when first used 425 + - `OMIT_FONTS` can reduce binary size for minimal builds 426 + - Font IDs defined in [src/fontIds.h](src/fontIds.h) 427 + 428 + **Usage**: 429 + ```cpp 430 + #include "fontIds.h" 431 + 432 + renderer.insertFont(FONT_UI_MEDIUM, ui12FontFamily); 433 + renderer.drawText(FONT_UI_MEDIUM, x, y, "Hello", true); 434 + ``` 435 + 436 + --- 437 + 438 + ## Testing and Debugging 439 + 440 + ### Build Commands 441 + 442 + **Via CLI**: 443 + ```bash 444 + # Build firmware (default environment) 445 + pio run 446 + 447 + # Build and upload to device 448 + pio run -t upload 449 + 450 + # Build specific environment 451 + pio run -e gh_release 452 + 453 + # Clean build artifacts 454 + pio run -t clean 455 + 456 + # Upload filesystem data (if using SPIFFS/LittleFS) 457 + pio run -t uploadfs 458 + ``` 459 + 460 + **Via VS Code**: 461 + * Use PlatformIO toolbar: Build (✓), Upload (→), Clean (🗑️) 462 + * Or Command Palette: `PlatformIO: Build`, `PlatformIO: Upload`, etc. 463 + 464 + ### Monitoring and Debugging 465 + 466 + ```bash 467 + # Enhanced monitor with color/logging (recommended) 468 + python3 scripts/debugging_monitor.py 469 + 470 + # Standard PlatformIO monitor 471 + pio device monitor 472 + 473 + # Combined upload + monitor 474 + pio run -t upload && pio device monitor 475 + ``` 476 + 477 + **Via VS Code**: Click Monitor (🔌) button in PlatformIO toolbar 478 + 479 + ### Code Quality 480 + 481 + ```bash 482 + # Static analysis (cppcheck) 483 + pio check 484 + 485 + # Format code (clang-format) - Windows Git Bash 486 + find src -name "*.cpp" -o -name "*.h" | xargs clang-format -i 487 + 488 + # Format code (clang-format) - Linux 489 + clang-format -i src/**/*.cpp src/**/*.h 490 + ``` 491 + 492 + ### Debugging Crashes 493 + 494 + **Common Crash Causes**: 495 + 496 + 1. **Out of Memory** (Most common): 497 + ```cpp 498 + LOG_DBG("MEM", "Free heap: %d bytes", ESP.getFreeHeap()); 499 + ``` 500 + - Monitor heap usage throughout activity lifecycle 501 + - Check if large allocations (>10KB) occur before crash 502 + - Verify buffers are freed in `onExit()` 503 + 504 + 2. **Stack Overflow**: 505 + ```cpp 506 + LOG_DBG("TASK", "Stack high water: %d", uxTaskGetStackHighWaterMark(taskHandle)); 507 + ``` 508 + - Occurs during deep recursion or large local variables 509 + - Increase task stack size in `xTaskCreate()` (2048 → 4096) 510 + - Move large buffers to heap with malloc 511 + 512 + 3. **Use-After-Free**: 513 + - Activity deleted but task still running 514 + - Always `vTaskDelete()` in `onExit()` BEFORE activity destruction 515 + - Set pointers to `nullptr` after `free()` 516 + 517 + 4. **Corrupt Cache Files**: 518 + - Delete `.crosspoint/` directory on SD card 519 + - Forces clean re-parse of all EPUBs 520 + - Check file format versions in [docs/file-formats.md](docs/file-formats.md) 521 + 522 + 5. **Watchdog Timeout**: 523 + - Loop/task blocked for >5 seconds 524 + - Add `vTaskDelay(1)` in tight loops 525 + - Check for blocking I/O operations 526 + 527 + **Verification Steps**: 528 + 1. Check serial output for stack traces 529 + 2. Monitor heap with `ESP.getFreeHeap()` before/after operations 530 + 3. Verify task deletion with task list (`vTaskList()`) 531 + 4. Test with `LOG_LEVEL=2` (debug logging enabled) 532 + 533 + --- 534 + 535 + ## Git Workflow and Repository Awareness 536 + 537 + ### Repository Detection Protocol 538 + 539 + **CRITICAL**: ALWAYS verify repository context before git operations. This could be: 540 + - A **fork** with `origin` pointing to personal repo, `upstream` to main repo 541 + - A **direct clone** with `origin` pointing to main repo 542 + - Multiple collaborator remotes 543 + 544 + **Verification Commands** (run at session start): 545 + ```bash 546 + # Check current branch 547 + git branch --show-current 548 + 549 + # Check all remotes 550 + git remote -v 551 + 552 + # Identify main branch name (could be 'main' or 'master') 553 + git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@' 554 + 555 + # Check working tree status 556 + git status --short 557 + ``` 558 + 559 + **Example Output** (forked repository): 560 + ```text 561 + origin https://github.com/<your-username>/crosspoint-reader.git (fetch/push) 562 + upstream https://github.com/crosspoint-reader/crosspoint-reader.git (fetch/push) 563 + ``` 564 + 565 + ### Git Operation Rules 566 + 567 + 1. **Never assume branch names**: 568 + ```bash 569 + # Bad: git push origin main 570 + # Good: git push origin $(git branch --show-current) 571 + ``` 572 + 573 + 2. **Never assume remote names or write permissions**: 574 + - **Forked repos**: Push to `origin` (your fork), submit PR to `upstream` 575 + - **Direct contributors**: May push feature branches to `upstream` 576 + - **Always ask**: "Should I push to origin or create a PR?" 577 + 578 + 3. **Check for upstream changes before starting work**: 579 + ```bash 580 + # Sync fork with upstream (if applicable) 581 + git fetch upstream 582 + git merge upstream/main # or upstream/master 583 + ``` 584 + 585 + 4. **Use explicit remote and branch names**: 586 + ```bash 587 + # Check remotes first 588 + git remote -v 589 + 590 + # Use explicit syntax 591 + git push <remote> <branch> 592 + ``` 593 + 594 + ### Branch Naming Convention 595 + 596 + **For feature/fix branches**: 597 + ```text 598 + feature/<short-description> # New features 599 + fix/<issue-number>-<description> # Bug fixes 600 + refactor/<component-name> # Code refactoring 601 + docs/<topic> # Documentation updates 602 + ``` 603 + 604 + **Examples**: 605 + - `feature/sd-download-progress` 606 + - `fix/123-orientation-crash` 607 + - `refactor/hal-storage` 608 + 609 + ### Commit Message Format 610 + 611 + **Pattern**: 612 + ```text 613 + <type>: <short summary (50 chars max)> 614 + 615 + <optional detailed description> 616 + 617 + ``` 618 + 619 + **Types**: `feat`, `fix`, `refactor`, `docs`, `test`, `chore`, `perf` 620 + 621 + **Example**: 622 + ```text 623 + feat: add real-time SD download progress bar 624 + 625 + Implements progress tracking for book downloads using 626 + UITheme progress bar component with heap-safe updates. 627 + 628 + Tested in all 4 orientations with 5MB+ files. 629 + ``` 630 + 631 + ### When to Commit 632 + 633 + **DO commit when**: 634 + - User explicitly requests: "commit these changes" 635 + - Feature is complete and tested on device 636 + - Bug fix is verified working 637 + - Refactoring preserves all functionality 638 + - All tests pass (`pio run` succeeds) 639 + 640 + **DO NOT commit when**: 641 + - Changes are untested on actual hardware 642 + - Build fails or has warnings 643 + - Experimenting or debugging in progress 644 + - User hasn't explicitly requested commit 645 + - Files excluded by `.gitignore` would be included — always run `git status` and cross-check against `.gitignore` before staging (e.g., `*.generated.h`, `.pio/`, `compile_commands.json`, `platformio.local.ini`) 646 + 647 + **Rule**: **If uncertain, ASK before committing.** 648 + 649 + --- 650 + 651 + ## Generated Files and Build Artifacts 652 + 653 + ### Files Generated by Build Scripts 654 + 655 + **NEVER manually edit these files** - they are regenerated automatically: 656 + 657 + 1. **HTML Headers** (generated by `scripts/build_html.py`): 658 + - `src/network/html/*.generated.h` 659 + - **Source**: HTML templates in `data/html/` directory 660 + - **Triggered**: During PlatformIO `pre:` build step 661 + - **To modify**: Edit source HTML in `data/html/`, not generated headers 662 + 663 + 2. **I18n Headers** (generated by `scripts/gen_i18n.py`): 664 + - `lib/I18n/I18nKeys.h`, `lib/I18n/I18nStrings.h`, `lib/I18n/I18nStrings.cpp` 665 + - **Source**: YAML translation files in `lib/I18n/translations/` (one per language) 666 + - **To modify**: Edit source YAML files, then run `python scripts/gen_i18n.py lib/I18n/translations lib/I18n/` 667 + - **Commit**: Source YAML files + `I18nKeys.h` and `I18nStrings.h` (needed for IDE symbol resolution), but NOT `I18nStrings.cpp` 668 + 669 + 3. **Build Artifacts** (in `.gitignore`): 670 + - `.pio/` - PlatformIO build output 671 + - `build/` - Compiled binaries 672 + - `*.generated.h` - Any auto-generated headers 673 + - `compile_commands.json` - LSP/IDE metadata 674 + 675 + ### Modifying Generated Content Workflow 676 + 677 + **To change HTML pages**: 678 + 1. Edit source: `data/html/<pagename>.html` 679 + 2. Build: `pio run` (auto-triggers `scripts/build_html.py`) 680 + 3. Generated headers update: `src/network/html/<pagename>Html.generated.h` 681 + 4. **Commit ONLY** source HTML, NOT generated `.generated.h` files 682 + 683 + **To add/modify translations (i18n)**: 684 + 1. Edit or add YAML file: `lib/I18n/translations/<language>.yaml` 685 + - Each file must contain: `_language_name`, `_language_code`, `_order`, and `STR_*` keys 686 + - English (`english.yaml`) is the reference; missing keys in other languages fall back to English 687 + 2. Run generator: `python scripts/gen_i18n.py lib/I18n/translations lib/I18n/` 688 + 3. Generated files update: `I18nKeys.h`, `I18nStrings.h`, `I18nStrings.cpp` 689 + 4. **Commit** source YAML files + `I18nKeys.h` and `I18nStrings.h` (IDE needs these for symbol resolution), but NOT `I18nStrings.cpp` 690 + 691 + **To use translated strings in code**: 692 + ```cpp 693 + #include <I18n.h> 694 + // Use tr() macro with StrId enum (defined in generated I18nKeys.h) 695 + renderer.drawText(FONT_UI, x, y, tr(STR_LOADING), true); 696 + ``` 697 + 698 + **To add custom fonts**: 699 + 1. Place source fonts in `lib/EpdFont/fontsrc/` (gitignored) 700 + 2. Run conversion script (see `lib/EpdFont/README`) 701 + 3. Update global font objects in `src/main.cpp:40-115` 702 + 4. Add font ID constant to `src/fontIds.h` 703 + 704 + --- 705 + 706 + ## Local Development Configuration 707 + 708 + ### platformio.local.ini (Personal Overrides) 709 + 710 + **Purpose**: Personal development settings that should NEVER be committed. 711 + 712 + **Use Cases**: 713 + - Serial port configuration (varies by machine) 714 + - Debug flags for specific testing 715 + - Local build optimizations 716 + - Developer-specific paths 717 + 718 + **Example** `platformio.local.ini`: 719 + ```ini 720 + # platformio.local.ini (gitignored) 721 + [env:default] 722 + upload_port = COM7 # Windows: COMx, Linux: /dev/ttyUSBx 723 + monitor_port = COM7 724 + 725 + build_flags = 726 + ${base.build_flags} 727 + -DMY_DEBUG_FLAG=1 # Personal debug flags 728 + -DTEST_FEATURE_ENABLED=1 729 + ``` 730 + 731 + **Configuration Hierarchy**: 732 + 1. `platformio.ini` - **Committed**, shared project settings 733 + 2. `platformio.local.ini` - **Gitignored**, personal overrides 734 + 3. Local file extends/overrides base config 735 + 736 + **Rules**: 737 + - **NEVER commit** `platformio.local.ini` 738 + - **NEVER put** personal info (serial ports, credentials) in main `platformio.ini` 739 + - Use `${base.build_flags}` to extend (not replace) base flags 740 + 741 + --- 742 + 743 + ## Testing and Verification Workflow 744 + 745 + ### Testing Checklist 746 + 747 + **AI agent scope** (what you CAN verify): 748 + 1. ✅ **Build**: `pio run -t clean && pio run` (0 errors/warnings) 749 + 2. ✅ **Quality**: `pio check` + `find src -name "*.cpp" -o -name "*.h" | xargs clang-format -i` 750 + 3. ✅ **Format**: Commit messages (`feat:`/`fix:`), no `.gitignore`-excluded files staged (e.g., `*.generated.h`, `.pio/`, `platformio.local.ini`) 751 + 4. ✅ **CI**: Fix GitHub Actions failures before review 752 + 5. ✅ **Code review**: Ensure orientation-aware logic is correct in all 4 modes by inspecting switch/case coverage 753 + 754 + **Human tester scope** (flag these for the user): 755 + 6. 🔲 **Device**: Test on hardware 756 + 7. 🔲 **Orientations**: Verify all 4 modes (Portrait/Inverted/Landscape CW/CCW) 757 + 8. 🔲 **Heap**: `ESP.getFreeHeap()` > 50KB, no leaks 758 + 9. 🔲 **Cache**: If EPUB modified, delete `.crosspoint/` and verify re-parse 759 + 760 + ### CI/CD Pipeline Awareness 761 + 762 + **GitHub Actions** run automatically on pull requests: 763 + 764 + | Workflow | File | Purpose | 765 + |----------|------|---------| 766 + | Build Check | `.github/workflows/ci.yml` | Verifies code compiles | 767 + | Format Check | `.github/workflows/pr-formatting-check.yml` | Validates clang-format | 768 + | Release Build | `.github/workflows/release.yml` | Production releases | 769 + | RC Build | `.github/workflows/release_candidate.yml` | Release candidates | 770 + 771 + **Rules**: 772 + - **Fix CI failures BEFORE** requesting review 773 + - CI runs on: Push to PR, PR updates 774 + - Format check fails → Run clang-format locally 775 + - Build check fails → Fix compile errors 776 + 777 + --- 778 + 779 + ## Serial Monitoring and Live Debugging 780 + 781 + ### Serial Monitor Options 782 + 783 + 1. **Enhanced**: `python3 scripts/debugging_monitor.py` (color-coded, recommended) 784 + 2. **Standard**: `pio device monitor` (basic, no colors) 785 + 3. **VS Code**: Monitor (🔌) button (IDE-integrated) 786 + 787 + ### Live Debugging Patterns 788 + 789 + **Heap**: `LOG_DBG("MEM", "Free: %d", ESP.getFreeHeap());` (every 5s in loop) 790 + **Stack**: `uxTaskGetStackHighWaterMark(nullptr)` (< 512 bytes → increase stack) 791 + **Flush**: `logSerial.flush();` (force output before crash) 792 + 793 + **Port Detection**: Windows: `mode` | Linux: `ls /dev/ttyUSB* /dev/ttyACM*` or `dmesg | grep tty` 794 + 795 + --- 796 + 797 + ## Cache Management and Invalidation 798 + 799 + ### Cache Structure on SD Card 800 + 801 + **Location**: `.crosspoint/` directory on SD card root 802 + 803 + **Structure**: `.crosspoint/epub_<hash>/{book.bin, progress.bin, cover.bmp, sections/*.bin}` 804 + 805 + **Hash**: `std::hash<std::string>{}(filepath)` → Moving/renaming file = new hash = lost progress 806 + 807 + ### Cache Invalidation Rules 808 + 809 + **Cache is automatically invalidated when**: 810 + 1. **File format version changes** (see `docs/file-formats.md`) 811 + - `book.bin` version number incremented 812 + - `section.bin` version number incremented 813 + 2. **Render settings change**: 814 + - Font family or size (`SETTINGS.fontFamily`, `SETTINGS.fontSize`) 815 + - Line spacing (`SETTINGS.lineSpacing`) 816 + - Paragraph spacing (`SETTINGS.extraParagraphSpacing`) 817 + - Screen margins (`SETTINGS.screenMargin`) 818 + 3. **Viewport dimensions change**: 819 + - Screen orientation change 820 + - Display resolution change 821 + 4. **Book file modified**: 822 + - Moved, renamed, or content changed (new hash) 823 + 824 + **Manual Cache Clear** (safe operations): 825 + ```bash 826 + # Delete ALL caches (forces full regeneration) 827 + rm -rf /path/to/sd/.crosspoint/ 828 + 829 + # Delete specific book cache 830 + rm -rf /path/to/sd/.crosspoint/epub_<hash>/ 831 + 832 + # Keep progress, delete only rendered sections 833 + rm -rf /path/to/sd/.crosspoint/epub_<hash>/sections/ 834 + ``` 835 + 836 + **When to Clear Cache**: 837 + - EPUB parsing errors after code changes to `lib/Epub/` 838 + - Corrupt rendering (missing text, wrong layout) 839 + - Testing cache generation logic 840 + - After modifying: 841 + - `lib/Epub/Epub/Section.cpp` 842 + - `lib/Epub/Epub/BookMetadataCache.cpp` 843 + - Render settings in `CrossPointSettings` 844 + 845 + ### Cache File Format Versioning 846 + 847 + **Source**: `lib/Epub/Epub/Section.cpp`, `lib/Epub/Epub/BookMetadataCache.cpp` 848 + 849 + **Current Versions** (as of docs/file-formats.md): 850 + - `book.bin`: **Version 5** (metadata structure) 851 + - `section.bin`: **Version 12** (layout structure) 852 + 853 + **Version Increment Rules**: 854 + 1. **ALWAYS increment version** BEFORE changing binary structure 855 + 2. Version mismatch → Cache auto-invalidated and regenerated 856 + 3. Document format changes in `docs/file-formats.md` 857 + 858 + **Example** (incrementing section format version): 859 + ```cpp 860 + // lib/Epub/Epub/Section.cpp 861 + static constexpr uint8_t SECTION_FILE_VERSION = 13; // Was 12, now 13 862 + 863 + // Add new field to structure 864 + struct PageLine { 865 + // ... existing fields ... 866 + uint16_t newField; // New field added 867 + }; 868 + ``` 869 + 870 + --- 871 + 872 + Philosophy: We are building a dedicated e-reader, not a Swiss Army knife. If a feature adds RAM pressure without significantly improving the reading experience, it is Out of Scope.
+1
CLAUDE.md
··· 1 + .skills/SKILL.md