Skip to content

Commit d5de64a

Browse files
committed
fix(reader): never lose reading time — deep-sleep commit + incremental save + drop 10s floor
Ports the applicable subset of dawsonfi/aalu's (MIT) reading-stats correctness fixes into our architecture. Three bugs in the stock session-tracking path: 1. Deep-sleep didn't commit the reader's session. enterDeepSleep called activityManager.goToSleep() which deferred the replace (and therefore the reader's onExit) until the next loop tick — but the hardware deep-sleep entry ran immediately after, so the transition never fired. Every minute spent reading since the last onExit was lost on each power-off. 2. A brown-out, hard hang, or sudden battery cut also lost the whole session. stats.bin was only written at onExit, never during reading. 3. Sessions under 10 seconds discarded reading time entirely (floor-gated at 10s for totalReadingSeconds). Users who do many short reads accumulated nothing. Fixes: - New Activity::onBeforeDeepSleep() virtual + ActivityManager:: notifyBeforeDeepSleep() that fans out to current + stacked. Wired into main.cpp::enterDeepSleep BEFORE the hardware sleep call so the reader commits its session before powering off. - EpubReaderActivity::commitReadingSession() — idempotent helper that banks elapsed time from the current segment (segment anchor resets after commit so successive commits don't double-count). Replaces the inline onExit() calculation and is shared with onBeforeDeepSleep + the incremental-save tick. - 60s incremental save in EpubReaderActivity::loop() — every minute of reading, commitReadingSession flushes to stats.bin. Worst-case loss on hard crash drops from "entire session" to "<=1 minute". - Drop the 10s floor on totalReadingSeconds. All elapsed seconds add to lifetime totals; sessionCount still uses the 60s floor (only bumped once per open, even across multiple commits). - Live in-reader stats popup now adds `millis() - sessionSegmentStartMs` on top of saved stats instead of `millis() - sessionStartMs`. With incremental saves, the previously-banked time is already in stats — using sessionStartMs would double-count. aalu's fixes #2 (don't walk progressPercent backwards) and #4 (beginSession doesn't overwrite progress) don't apply: our architecture stores progress in a separate progress.bin file rather than a progressPercent field in stats, and onEnter doesn't write progress. Fix #5 (100%/99% off-by-one) doesn't appear to apply either — our Epub::calculateProgress already does byte-weighted cumulative-size math and reaches 1.0 at last-page/last-spine.
1 parent 929d37e commit d5de64a

6 files changed

Lines changed: 121 additions & 17 deletions

File tree

src/activities/Activity.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ class Activity {
3030
virtual ~Activity() = default;
3131
virtual void onEnter();
3232
virtual void onExit();
33+
// FlexBLE: called by main.cpp::enterDeepSleep BEFORE the hardware
34+
// sleep enters. Activities can use this to commit in-flight state
35+
// (reading sessions, draft inputs, etc.) that would otherwise be
36+
// lost — the normal onExit() path doesn't fire when going to deep
37+
// sleep, because the activity isn't being torn down; the chip is
38+
// just being powered off. Default no-op.
39+
virtual void onBeforeDeepSleep() {}
3340
virtual void loop() {}
3441

3542
virtual void render(RenderLock&&) {}

src/activities/ActivityManager.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,16 @@ ScreenshotInfo ActivityManager::getScreenshotInfo() const {
275275
return {};
276276
}
277277

278+
void ActivityManager::notifyBeforeDeepSleep() {
279+
// Forward to the current activity AND anything stacked underneath.
280+
// The reader might not be the top of the stack (e.g. an action menu
281+
// is open on top of it) but still wants its session committed.
282+
if (currentActivity) currentActivity->onBeforeDeepSleep();
283+
for (auto& stacked : stackActivities) {
284+
if (stacked) stacked->onBeforeDeepSleep();
285+
}
286+
}
287+
278288
void ActivityManager::requestUpdate(bool immediate) {
279289
if (immediate) {
280290
if (renderTaskHandle) {

src/activities/ActivityManager.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ class ActivityManager {
112112
bool canSnapshotForSleepOverlay() const;
113113
bool skipLoopDelay() const;
114114
ScreenshotInfo getScreenshotInfo() const;
115+
// FlexBLE: called from main.cpp::enterDeepSleep so the current
116+
// activity can flush in-flight state (e.g. reader session time)
117+
// before the chip powers off. See Activity::onBeforeDeepSleep().
118+
void notifyBeforeDeepSleep();
115119

116120
// If immediate is true, the update will be triggered immediately.
117121
// Otherwise, it will be deferred until the end of the current loop iteration.

src/activities/reader/EpubReaderActivity.cpp

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,10 @@ void EpubReaderActivity::onEnter() {
242242
// Session count and reading time are committed on exit once thresholds are met.
243243
stats = BookReadingStats::load(epub->getCachePath());
244244
sessionStartMs = millis();
245+
sessionSegmentStartMs = sessionStartMs;
246+
totalSessionMsThisOpen = 0UL;
247+
sessionCountedThisOpen = false;
248+
lastIncrementalSaveMs = sessionStartMs;
245249

246250
globalStats = GlobalReadingStats::load();
247251

@@ -279,21 +283,10 @@ void EpubReaderActivity::onExit() {
279283
APP_STATE.readerActivityLoadCount = 0;
280284
APP_STATE.saveToFile();
281285

282-
// Commit session stats based on how long the session lasted.
283-
// Sessions under 1 minute don't count toward session count or reading time.
284-
// Sessions under 10 seconds don't add to reading time.
285-
const unsigned long elapsedMs = millis() - sessionStartMs;
286-
if (elapsedMs >= 60000UL) {
287-
stats.sessionCount++;
288-
globalStats.totalSessions++;
289-
}
290-
if (elapsedMs >= 10000UL) {
291-
const uint32_t elapsedSecs = static_cast<uint32_t>(elapsedMs / 1000UL);
292-
stats.totalReadingSeconds += elapsedSecs;
293-
globalStats.totalReadingSeconds += elapsedSecs;
294-
}
295-
stats.save(epub->getCachePath());
296-
globalStats.save();
286+
// Commit any remaining session time. Idempotent — if a deep-sleep
287+
// commit or incremental save already banked the current segment,
288+
// commitReadingSession returns without double-counting.
289+
commitReadingSession();
297290

298291
BOOKMARKS.unload();
299292
section.reset();
@@ -336,13 +329,66 @@ void EpubReaderActivity::onExit() {
336329
}
337330
}
338331

332+
void EpubReaderActivity::commitReadingSession() {
333+
if (!epub) return;
334+
// Bank elapsed time from the current segment. sessionSegmentStartMs
335+
// is reset every time we commit so successive commits (incremental
336+
// save, deep-sleep, onExit) don't double-add the same milliseconds.
337+
const unsigned long now = millis();
338+
const unsigned long segmentMs = now - sessionSegmentStartMs;
339+
if (segmentMs == 0UL) return;
340+
sessionSegmentStartMs = now;
341+
totalSessionMsThisOpen += segmentMs;
342+
343+
// Session count: incremented at most once per open (when cumulative
344+
// time crosses the 60s threshold). A book briefly tapped open
345+
// doesn't bump the count; a long read commits exactly one +1 even
346+
// if it spans multiple deep-sleep commits.
347+
if (!sessionCountedThisOpen && totalSessionMsThisOpen >= 60000UL) {
348+
stats.sessionCount++;
349+
globalStats.totalSessions++;
350+
sessionCountedThisOpen = true;
351+
}
352+
353+
// Reading time: no longer floor-gated. Every banked ms adds to the
354+
// lifetime totals (was previously gated at 10 s, which silently
355+
// discarded short reads — particularly bad for users who do
356+
// many <10s sessions, e.g. mid-session deep-sleep cycles).
357+
const uint32_t elapsedSecs = static_cast<uint32_t>(segmentMs / 1000UL);
358+
if (elapsedSecs > 0) {
359+
stats.totalReadingSeconds += elapsedSecs;
360+
globalStats.totalReadingSeconds += elapsedSecs;
361+
}
362+
363+
stats.save(epub->getCachePath());
364+
globalStats.save();
365+
}
366+
367+
void EpubReaderActivity::onBeforeDeepSleep() {
368+
// Same commit path as onExit, but the activity STAYS alive (just
369+
// gets put to sleep alongside the chip). When the device wakes,
370+
// session-resume continues from the saved progress.bin position
371+
// and a fresh session segment begins.
372+
commitReadingSession();
373+
}
374+
339375
void EpubReaderActivity::loop() {
340376
if (!epub) {
341377
// Should never happen
342378
finish();
343379
return;
344380
}
345381

382+
// Incremental session save. Without this, a brown-out / hard crash
383+
// mid-reading loses ALL accumulated time since onEnter (or the last
384+
// commit). With it, worst-case loss is kIncrementalSaveMs. The cost
385+
// is small: one SD write per minute of reading.
386+
constexpr unsigned long kIncrementalSaveMs = 60000UL; // 1 min
387+
if (millis() - lastIncrementalSaveMs >= kIncrementalSaveMs) {
388+
commitReadingSession();
389+
lastIncrementalSaveMs = millis();
390+
}
391+
346392
if (completionPromptQueued) {
347393
completionPromptQueued = false;
348394
completionPromptShown = true;
@@ -842,9 +888,13 @@ void EpubReaderActivity::onReaderMenuConfirm(EpubReaderMenuActivity::MenuAction
842888
break;
843889
}
844890
case EpubReaderMenuActivity::MenuAction::READING_STATS: {
845-
// Include elapsed time from the current session in the display stats.
891+
// Include elapsed time from the CURRENT (uncommitted) session
892+
// segment on top of what's been banked into stats. Previously
893+
// banked segments are already in `stats.totalReadingSeconds`
894+
// because commitReadingSession persists them incrementally —
895+
// adding `millis() - sessionStartMs` would double-count.
846896
BookReadingStats displayStats = stats;
847-
displayStats.totalReadingSeconds += static_cast<uint32_t>((millis() - sessionStartMs) / 1000UL);
897+
displayStats.totalReadingSeconds += static_cast<uint32_t>((millis() - sessionSegmentStartMs) / 1000UL);
848898
startActivityForResult(
849899
std::make_unique<BookStatsActivity>(renderer, mappedInput, epub->getPath(), epub->getTitle(),
850900
epub->getThumbBmpPath(), displayStats, globalStats),

src/activities/reader/EpubReaderActivity.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,20 @@ class EpubReaderActivity final : public Activity {
3232
BookReadingStats stats;
3333
GlobalReadingStats globalStats;
3434
unsigned long sessionStartMs = 0UL;
35+
// Wall-clock anchor for the current "session segment" — reset by
36+
// commitReadingSession every time it banks elapsed time so we don't
37+
// double-count across deep-sleep / shutdown commits.
38+
unsigned long sessionSegmentStartMs = 0UL;
39+
// Cumulative session ms already banked into stats this opening. Used
40+
// only to gate sessionCount (the +1 happens once per session ≥ 60s,
41+
// even if multiple commits add up to >60s).
42+
unsigned long totalSessionMsThisOpen = 0UL;
43+
bool sessionCountedThisOpen = false;
44+
// Wall-clock anchor for the periodic incremental save. Reading
45+
// sessions that crash before onExit (e.g. brown-out, hard hang) used
46+
// to lose ALL elapsed time. Now we flush every kIncrementalSaveMs
47+
// milliseconds during loop() so worst-case loss is bounded.
48+
unsigned long lastIncrementalSaveMs = 0UL;
3549
// Signals that the next render should reposition within the newly loaded section
3650
// based on a cross-book percentage jump.
3751
bool pendingPercentJump = false;
@@ -89,6 +103,12 @@ class EpubReaderActivity final : public Activity {
89103
SavedPosition savedPositions[MAX_FOOTNOTE_DEPTH] = {};
90104
int footnoteDepth = 0;
91105

106+
// Banks elapsed time from `sessionSegmentStartMs` into stats.bin +
107+
// GlobalReadingStats and resets the anchor so subsequent calls don't
108+
// double-count. Idempotent: a 0-ms segment is a no-op. Called from
109+
// onExit, onBeforeDeepSleep, and the incremental save tick.
110+
void commitReadingSession();
111+
92112
void renderContents(std::unique_ptr<Page> page, int orientedMarginTop, int orientedMarginRight,
93113
int orientedMarginBottom, int orientedMarginLeft);
94114
void renderStatusBar() const;
@@ -124,6 +144,11 @@ class EpubReaderActivity final : public Activity {
124144
: Activity("EpubReader", renderer, mappedInput), epub(std::move(epub)) {}
125145
void onEnter() override;
126146
void onExit() override;
147+
// Banks the current reading session into stats before the device
148+
// powers off. Without this, time read since the last commit was
149+
// lost — onExit only fires on explicit activity transitions, and
150+
// hardware deep-sleep skips that path. Idempotent with onExit.
151+
void onBeforeDeepSleep() override;
127152
void loop() override;
128153
void render(RenderLock&& lock) override;
129154
bool preventAutoSleep() override { return automaticPageTurnActive; }

src/main.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,14 @@ void enterDeepSleep() {
526526
APP_STATE.lastSleepFromReader = activityManager.isReaderActivity();
527527
APP_STATE.saveToFile();
528528

529+
// FlexBLE: give the current activity a chance to flush in-flight
530+
// state (most importantly: the reader's accumulated session time)
531+
// before the chip powers off. Without this hook, every minute spent
532+
// reading since the last natural activity-exit was lost on each
533+
// deep-sleep cycle. Ported in spirit from dawsonfi/aalu's
534+
// ReadingStatsManager::endSession deep-sleep wiring.
535+
activityManager.notifyBeforeDeepSleep();
536+
529537
// Disable BLE before deep sleep so the NimBLE host shuts down cleanly and
530538
// the radio is released before the chip powers off. Idempotent if BLE was
531539
// already off (reader exit path) — defensive against the auto-sleep timer

0 commit comments

Comments
 (0)