fix: make transaction reset best effort#240
Conversation
|
Potential pitfalls / review concerns for this reset-policy change: This PR intentionally changes the meaning of Specific concerns to evaluate:
My recommendation is to merge this only if we are comfortable with the global policy shift for normal requests and commands. If the desired behavior is specifically "ESP32 I2C init should not get stuck behind a failing reset," a narrower alternative would be to keep normal transactions reset-or-fail and apply best-effort semantics only in the initialization/probe path. |
Summary
Split out the transaction reset behavior change from PR #239 so it can be reviewed independently from the new
NotePing()API.This changes
_noteTransactionShouldLock()so that whenresetRequiredis set, the transaction path makes a best-effort call to_Reset(), clearsresetRequired, and proceeds with the transaction even if reset did not report success.Rationale
ESP32 I2C initialization can repeatedly fail in a way that prevents progress when reset is treated as a hard gate. This keeps the reset attempt, but avoids blocking the following transaction solely because the reset hook failed.
Tests
docker run --rm -v "$PWD":/note-c -w /note-c ghcr.io/blues/note_c_ci:latest bash -lc 'cmake --build build --target NoteTransaction_test NoteTransactionHooks_test NoteReset_test _noteHardReset_test -j2 && ctest --test-dir build -R "(NoteTransaction|NoteReset|_noteHardReset)" --output-on-failure'Result: 8/8 related transaction/reset tests passed.
Note: a full
run_unit_tests.shpass was attempted, but this workspace's reused Docker build directory produced CMake*_NOT_BUILTplaceholders unless individual test targets were built. The directly relevant reset/transaction targets were rebuilt and passed.