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.

docs: ActivityManager migration guide (#1222)

## Summary

**What is the goal of this PR?**

Added overview and migration guide for ActivityManager changes in #1016.
Thanks for the suggestion, @drbourbon!

---

### 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? _**YES, fully written by
Claude 4.6**_

authored by

Zach Nelson and committed by
GitHub
42b122b8 0e168aa2

+472
+472
docs/activity-manager.md
··· 1 + # Activity & ActivityManager Migration Guide 2 + 3 + This document explains the refactoring from the original per-activity render task model to the centralized `ActivityManager` introduced in [PR #1016](https://github.com/crosspoint-reader/crosspoint-reader/pull/1016). It covers the architectural differences, what changed for activity authors, and the FreeRTOS task and locking model that underpins the system. 4 + 5 + ## Overview of Changes 6 + 7 + | Aspect | Old Model | New Model | 8 + |--------|-----------|-----------| 9 + | Render task | One per activity (8KB stack each) | Single shared task in `ActivityManager` | 10 + | Render mutex | Per-activity `renderingMutex` | Single global mutex in `ActivityManager` | 11 + | `RenderLock` | Inner class of `Activity` | Standalone class, acquires global mutex | 12 + | Subactivities | `ActivityWithSubactivity` base class | Activity stack managed by `ActivityManager` | 13 + | Navigation | Free functions in `main.cpp` | `activityManager.goHome()`, `goToReader()`, etc. | 14 + | Subactivity results | Callback lambdas stored in parent | `startActivityForResult()` / `setResult()` / `finish()` | 15 + | `requestUpdate()` | Notifies activity's own render task | Delegates to `ActivityManager` (immediate or deferred) | 16 + 17 + ## Architecture 18 + 19 + ### Old Model: Per-Activity Render Tasks 20 + 21 + Each activity created its own FreeRTOS render task on entry and destroyed it on exit: 22 + 23 + ```text 24 + ┌─────────────────────────────────────────────────────────┐ 25 + │ Main Task (Arduino loop) │ 26 + │ ┌───────────────────────────────────────────────────┐ │ 27 + │ │ currentActivity->loop() │ │ 28 + │ │ ├── handle input │ │ 29 + │ │ ├── update state (under RenderLock) │ │ 30 + │ │ └── requestUpdate() ──notify──► Render Task │ │ 31 + │ │ (per-activity)│ │ 32 + │ │ 8KB stack │ │ 33 + │ │ owns mutex │ │ 34 + │ └───────────────────────────────────────────────────┘ │ 35 + │ │ 36 + │ ActivityWithSubactivity: │ 37 + │ ┌──────────────┐ ┌──────────────┐ │ 38 + │ │ Parent │────►│ SubActivity │ │ 39 + │ │ (has render │ │ (has own │ │ 40 + │ │ task) │ │ render task) │ │ 41 + │ └──────────────┘ └──────────────┘ │ 42 + └─────────────────────────────────────────────────────────┘ 43 + ``` 44 + 45 + Problems with this approach: 46 + 47 + - **8KB per render task**: Each activity allocated an 8KB FreeRTOS stack for its render task, even though only one renders at a time 48 + - **Dangerous deletion patterns**: `exitActivity()` + `enterNewActivity()` in callbacks led to `delete this` situations where the caller was destroyed while its code was still on the stack 49 + - **Subactivity coupling**: Parents stored callbacks to child results, creating tight coupling and lifetime hazards 50 + 51 + ### New Model: Centralized ActivityManager 52 + 53 + A single `ActivityManager` owns the render task and manages an activity stack: 54 + 55 + ```text 56 + ┌──────────────────────────────────────────────────────────┐ 57 + │ Main Task (Arduino loop) │ 58 + │ │ 59 + │ activityManager.loop() │ 60 + │ │ │ 61 + │ ├── currentActivity->loop() │ 62 + │ │ ├── handle input │ 63 + │ │ ├── update state (under RenderLock) │ 64 + │ │ └── requestUpdate() │ 65 + │ │ │ 66 + │ ├── process pending actions (Push / Pop / Replace) │ 67 + │ │ │ 68 + │ └── if requestedUpdate: ──notify──► Render Task │ 69 + │ (single, shared) │ 70 + │ 8KB stack │ 71 + │ global mutex │ 72 + │ │ 73 + │ Activity Stack: │ 74 + │ ┌──────────┬──────────┬──────────┐ ┌──────────┐ │ 75 + │ │ Home │ Settings │ Wifi │ │ Keyboard │ │ 76 + │ │ (stack) │ (stack) │ (stack) │ │ (current)│ │ 77 + │ └──────────┴──────────┴──────────┘ └──────────┘ │ 78 + │ stackActivities[] currentActivity │ 79 + └──────────────────────────────────────────────────────────┘ 80 + ``` 81 + 82 + ## Migration Checklist 83 + 84 + ### 1. Change Base Class 85 + 86 + If your activity extended `ActivityWithSubactivity`, change it to extend `Activity`: 87 + 88 + ```cpp 89 + // BEFORE 90 + class MyActivity final : public ActivityWithSubactivity { 91 + MyActivity(GfxRenderer& r, MappedInputManager& m, std::function<void()> goBack) 92 + : ActivityWithSubactivity("MyActivity", r, m), goBack(goBack) {} 93 + }; 94 + 95 + // AFTER 96 + class MyActivity final : public Activity { 97 + MyActivity(GfxRenderer& r, MappedInputManager& m) 98 + : Activity("MyActivity", r, m) {} 99 + }; 100 + ``` 101 + 102 + Note that navigation callbacks like `goBack` are no longer stored — use `finish()` or `activityManager.goHome()` instead. 103 + 104 + ### 2. Replace Navigation Functions 105 + 106 + The free functions `exitActivity()` / `enterNewActivity()` in `main.cpp` are gone. Use `ActivityManager` methods: 107 + 108 + ```cpp 109 + // BEFORE (in main.cpp or via stored callbacks) 110 + exitActivity(); 111 + enterNewActivity(new SettingsActivity(renderer, mappedInput, onGoHome)); 112 + 113 + // AFTER (from any Activity method) 114 + activityManager.goToSettings(); 115 + // or for arbitrary navigation: 116 + activityManager.replaceActivity(std::make_unique<MyActivity>(renderer, mappedInput)); 117 + ``` 118 + 119 + `replaceActivity()` destroys the current activity and clears the stack. Use it for top-level navigation (home, reader, settings, etc.). 120 + 121 + ### 3. Replace Subactivity Pattern 122 + 123 + The `enterNewActivity()` / `exitActivity()` subactivity pattern is replaced by a stack with typed results: 124 + 125 + ```cpp 126 + // BEFORE 127 + void MyActivity::launchWifi() { 128 + enterNewActivity(new WifiSelectionActivity(renderer, mappedInput, 129 + [this](bool connected) { onWifiDone(connected); })); 130 + } 131 + // Child calls: onComplete(true); // triggers callback, which may call exitActivity() 132 + 133 + // AFTER 134 + void MyActivity::launchWifi() { 135 + startActivityForResult( 136 + std::make_unique<WifiSelectionActivity>(renderer, mappedInput), 137 + [this](const ActivityResult& result) { 138 + if (result.isCancelled) return; 139 + auto& wifi = std::get<WifiResult>(result.data); 140 + onWifiDone(wifi.connected); 141 + }); 142 + } 143 + // Child calls: 144 + // setResult(WifiResult{.connected = true, .ssid = ssid}); 145 + // finish(); 146 + ``` 147 + 148 + Key differences: 149 + 150 + - **`startActivityForResult()`** pushes the current activity onto the stack and launches the child 151 + - **`setResult()`** stores a typed result on the child activity 152 + - **`finish()`** signals the manager to pop the child, call the result handler, and resume the parent 153 + - The parent is never deleted during this process — it's safely stored on the stack 154 + 155 + ### 4. Update `render()` Signature 156 + 157 + The `RenderLock` type changed from `Activity::RenderLock` (inner class) to standalone `RenderLock`: 158 + 159 + ```cpp 160 + // BEFORE 161 + void render(Activity::RenderLock&&) override; 162 + 163 + // AFTER 164 + void render(RenderLock&&) override; 165 + ``` 166 + 167 + Include `RenderLock.h` if not transitively included via `Activity.h`. 168 + 169 + ### 5. Update `onEnter()` / `onExit()` 170 + 171 + Activities no longer create or destroy render tasks: 172 + 173 + ```cpp 174 + // BEFORE 175 + void MyActivity::onEnter() { 176 + Activity::onEnter(); // created render task + logged 177 + // ... allocate resources 178 + requestUpdate(); 179 + } 180 + void MyActivity::onExit() { 181 + // ... free resources 182 + Activity::onExit(); // acquired RenderLock, deleted render task 183 + } 184 + 185 + // AFTER 186 + void MyActivity::onEnter() { 187 + Activity::onEnter(); // just logs 188 + // ... allocate resources 189 + requestUpdate(); 190 + } 191 + void MyActivity::onExit() { 192 + // ... free resources 193 + Activity::onExit(); // just logs 194 + } 195 + ``` 196 + 197 + The render task lifecycle is handled entirely by `ActivityManager::begin()`. 198 + 199 + ### 6. Update `requestUpdate()` Calls 200 + 201 + The signature changed to accept an `immediate` flag: 202 + 203 + ```cpp 204 + // BEFORE 205 + void requestUpdate(); // always immediate notification to per-activity render task 206 + 207 + // AFTER 208 + void requestUpdate(bool immediate = false); 209 + // immediate=false (default): deferred until end of current loop iteration 210 + // immediate=true: sends notification to render task right away 211 + ``` 212 + 213 + **When to use `immediate`**: Almost never. Deferred updates are batched — if `loop()` triggers multiple state changes that each call `requestUpdate()`, only one render happens. Use `immediate` only when you need the render to start before the current function returns (e.g., before a blocking network call). 214 + 215 + **`requestUpdateAndWait()`**: Blocks the calling task until the render completes. Use sparingly — it's designed for cases where you need the screen to reflect new state before proceeding (e.g., showing "Checking for update..." before calling a network API). 216 + 217 + ### 7. Remove Stored Navigation Callbacks 218 + 219 + Old activities often stored `std::function` callbacks for navigation: 220 + 221 + ```cpp 222 + // BEFORE 223 + class SettingsActivity : public ActivityWithSubactivity { 224 + const std::function<void()> goBack; // stored callback 225 + const std::function<void()> goHome; // stored callback 226 + public: 227 + SettingsActivity(GfxRenderer& r, MappedInputManager& m, 228 + std::function<void()> goBack, std::function<void()> goHome) 229 + : ActivityWithSubactivity("Settings", r, m), goBack(goBack), goHome(goHome) {} 230 + }; 231 + 232 + // AFTER 233 + class SettingsActivity : public Activity { 234 + public: 235 + SettingsActivity(GfxRenderer& r, MappedInputManager& m) 236 + : Activity("Settings", r, m) {} 237 + // Use finish() to go back, activityManager.goHome() to go home 238 + }; 239 + ``` 240 + 241 + This removes `std::function` overhead (~2-4KB per unique signature) and eliminates lifetime risks from captured `this` pointers. 242 + 243 + ## Technical Details 244 + 245 + ### FreeRTOS Task Model 246 + 247 + The firmware runs on an ESP32-C3, a single-core RISC-V microcontroller. FreeRTOS provides cooperative and preemptive multitasking on this single core — only one task executes at any moment, and the scheduler switches between tasks at yield points (blocking calls, `vTaskDelay`, `taskYIELD`) or when a tick interrupt promotes a higher-priority task. 248 + 249 + There are two tasks relevant to the activity system: 250 + 251 + ```text 252 + ┌──────────────────────┐ ┌──────────────────────────┐ 253 + │ Main Task │ │ Render Task │ 254 + │ (Arduino loop) │ │ (ActivityManager-owned) │ 255 + │ Priority: 1 │ │ Priority: 1 │ 256 + │ │ │ │ 257 + │ Runs: │ │ Runs: │ 258 + │ - gpio.update() │ │ - ulTaskNotifyTake() │ 259 + │ - activity->loop() │ │ (blocks until notified)│ 260 + │ - pending actions │ │ - RenderLock (mutex) │ 261 + │ - sleep/power mgmt │ │ - activity->render() │ 262 + │ - requestUpdate →────┼─────┼─► xTaskNotify() │ 263 + │ (end of loop) │ │ │ 264 + └──────────────────────┘ └──────────────────────────┘ 265 + ``` 266 + 267 + Both tasks run at priority 1. Since the ESP32-C3 is single-core, they alternate execution: the main task runs `loop()`, then at the end of the loop iteration, notifies the render task if an update was requested. The render task wakes, acquires the mutex, calls `render()`, releases the mutex, and blocks again. 268 + 269 + Do not use `xTaskCreate` inside activities. If you have a use case that seems to require a background task, open a discussion to propose a lifecycle-aware `Worker` abstraction first. 270 + 271 + ### The Render Mutex and RenderLock 272 + 273 + A single FreeRTOS mutex (`renderingMutex`) protects shared state between `loop()` and `render()`. Since these run on different tasks, any state read by `render()` and written by `loop()` must be guarded. 274 + 275 + `RenderLock` is an RAII wrapper: 276 + 277 + ```cpp 278 + // Standalone class (not tied to any specific activity) 279 + class RenderLock { 280 + bool isLocked = false; 281 + public: 282 + explicit RenderLock(); // acquires activityManager.renderingMutex 283 + explicit RenderLock(Activity&); // same — Activity& param kept for compatibility 284 + ~RenderLock(); // releases mutex if still held 285 + void unlock(); // early release 286 + }; 287 + ``` 288 + 289 + **Usage patterns:** 290 + 291 + ```cpp 292 + // In loop(): protect state mutations that render() reads 293 + void MyActivity::loop() { 294 + if (somethingChanged) { 295 + RenderLock lock; 296 + state = newState; // safe — render() can't run while lock is held 297 + } 298 + requestUpdate(); // trigger render after lock is released 299 + } 300 + 301 + // In render(): lock is passed in, held for duration of render 302 + void MyActivity::render(RenderLock&&) { 303 + // Lock is held — safe to read shared state 304 + renderer.clearScreen(); 305 + renderer.drawText(..., stateString, ...); 306 + renderer.displayBuffer(); 307 + // Lock released when RenderLock destructor runs 308 + } 309 + ``` 310 + 311 + **Critical rule**: Never call `requestUpdateAndWait()` while holding a `RenderLock`. The render task needs the mutex to call `render()`, so holding it while waiting for the render to complete is a deadlock: 312 + 313 + ```text 314 + Main Task Render Task 315 + ────────── ─────────── 316 + RenderLock lock; (blocked on mutex) 317 + requestUpdateAndWait(); 318 + → notify render task 319 + → block waiting for 320 + render to complete → wakes up 321 + → tries to acquire mutex 322 + → DEADLOCK: main holds mutex, 323 + waits for render; render 324 + waits for mutex 325 + ``` 326 + 327 + ### requestUpdate() vs requestUpdateAndWait() 328 + 329 + ```text 330 + requestUpdate(false) requestUpdate(true) 331 + ───────────────── ───────────────── 332 + Sets flag only. Notifies render task 333 + Render happens after immediately. 334 + loop() returns and Render may start 335 + ActivityManager checks before the calling 336 + the flag. function returns. 337 + (Does NOT wait for 338 + render to complete.) 339 + 340 + requestUpdateAndWait() 341 + ────────────────────── 342 + Notifies render task AND 343 + blocks calling task until 344 + render is done. Uses 345 + FreeRTOS direct-to-task 346 + notification on the 347 + caller's task handle. 348 + ``` 349 + 350 + `requestUpdateAndWait()` flow in detail: 351 + 352 + ```text 353 + Calling Task Render Task 354 + ──────────── ─────────── 355 + requestUpdateAndWait() 356 + ├─ assert: not render task 357 + ├─ assert: not holding RenderLock 358 + ├─ store waitingTaskHandle 359 + ├─ xTaskNotify(renderTask) → wakes render task 360 + └─ ulTaskNotifyTake() ─┐ 361 + (blocked) │ RenderLock lock; 362 + │ activity->render(); 363 + │ // render complete 364 + │ taskENTER_CRITICAL 365 + │ waiter = waitingTaskHandle 366 + │ waitingTaskHandle = nullptr 367 + │ taskEXIT_CRITICAL 368 + │ xTaskNotify(waiter) ───┐ 369 + │ │ 370 + ┌──────────────────────┘ │ 371 + │ (woken by notification) ◄────────────────────────┘ 372 + └─ return 373 + ``` 374 + 375 + ### Activity Lifecycle Under ActivityManager 376 + 377 + ```text 378 + activityManager.replaceActivity(make_unique<MyActivity>(...)) 379 + 380 + 381 + ╔═══════════════════════════════════════════════════╗ 382 + ║ pendingAction = Replace ║ 383 + ║ pendingActivity = MyActivity ║ 384 + ╚═══════════════════════════════════════════════════╝ 385 + 386 + ▼ (next loop iteration) 387 + ActivityManager::loop() 388 + 389 + ├── currentActivity->loop() // old activity's last loop 390 + 391 + ├── process pending action: 392 + │ ├── RenderLock lock; 393 + │ ├── oldActivity->onExit() // cleanup under lock 394 + │ ├── delete oldActivity 395 + │ ├── clear stack 396 + │ ├── currentActivity = MyActivity 397 + │ ├── lock.unlock() 398 + │ └── MyActivity->onEnter() // init new activity 399 + 400 + └── if requestedUpdate: 401 + └── notify render task 402 + ``` 403 + 404 + For push/pop (subactivity) navigation: 405 + 406 + ```text 407 + Parent calls: startActivityForResult(make_unique<Child>(...), handler) 408 + 409 + 410 + ╔══════════════════════════════════════╗ 411 + ║ pendingAction = Push ║ 412 + ║ pendingActivity = Child ║ 413 + ║ parent->resultHandler = handler ║ 414 + ╚══════════════════════════════════════╝ 415 + 416 + ▼ (next loop iteration) 417 + ├── Parent moved to stackActivities[] 418 + ├── currentActivity = Child 419 + └── Child->onEnter() 420 + 421 + ... child runs ... 422 + 423 + Child calls: setResult(MyResult{...}); finish(); 424 + 425 + 426 + ╔══════════════════════════════════════╗ 427 + ║ pendingAction = Pop ║ 428 + ║ child->result = MyResult{...} ║ 429 + ╚══════════════════════════════════════╝ 430 + 431 + ▼ (next loop iteration) 432 + ├── result = child->result 433 + ├── Child->onExit(); delete Child 434 + ├── currentActivity = Parent (popped from stack) 435 + ├── Parent->resultHandler(result) 436 + └── requestUpdate() // automatic re-render for parent 437 + ``` 438 + 439 + ### Common Pitfalls 440 + 441 + **Calling `finish()` and continuing to access `this`**: `finish()` sets `pendingAction = Pop` but does not immediately destroy the activity. The activity is destroyed on the next `ActivityManager::loop()` iteration. It's safe to access member variables after `finish()` within the same function, but don't rely on the activity surviving past the current `loop()` call. 442 + 443 + **Modifying shared state without `RenderLock`**: If `render()` reads a variable and `loop()` writes it, the write must be under a `RenderLock`. Without it, `render()` could see a half-written value (e.g., a partially updated string or struct). 444 + 445 + **Creating background tasks that outlive the activity**: Any FreeRTOS task created in `onEnter()` must be deleted in `onExit()` before the activity is destroyed. The `ActivityManager` does not track or clean up background tasks. 446 + 447 + **Holding `RenderLock` across blocking calls**: The render task is blocked on the mutex while you hold the lock. Keep critical sections short — acquire, mutate state, release, then do blocking work. 448 + 449 + ```cpp 450 + // WRONG — blocks render for the entire network call 451 + void MyActivity::doNetworkStuff() { 452 + RenderLock lock; 453 + state = LOADING; 454 + auto result = http.get(url); // blocks for seconds with lock held 455 + state = DONE; 456 + } 457 + 458 + // CORRECT — release lock before blocking 459 + void MyActivity::doNetworkStuff() { 460 + { 461 + RenderLock lock; 462 + state = LOADING; 463 + } 464 + requestUpdate(true); // render "Loading..." immediately, before we block 465 + auto result = http.get(url); // lock is not held 466 + { 467 + RenderLock lock; 468 + state = DONE; 469 + } 470 + requestUpdate(); 471 + } 472 + ```