From c6be185f4c11efb33e58c3488555191a75b410ac Mon Sep 17 00:00:00 2001 From: John Date: Wed, 23 Sep 2026 00:38:30 -0500 Subject: [PATCH] Agent identified fixes --- .github/workflows/ci.yml | 27 +++++ README.md | 21 ++-- TODO.md | 24 ++++ epub_to_chapters.py | 214 ++++++++++++--------------------- platformio.ini | 3 +- requirements.txt | 4 +- src/Library.cpp | 3 +- src/Settings.cpp | 26 +++- src/main.cpp | 57 ++++++--- tests/__init__.py | 1 + tests/test_epub_to_chapters.py | 54 +++++++++ 11 files changed, 259 insertions(+), 175 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 tests/__init__.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..458734e --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,27 @@ +name: CI + +on: + push: + pull_request: + +jobs: + converter: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + - run: python -m pip install -r requirements.txt + - run: python -m unittest discover -v + - run: python epub_to_chapters.py examples/rsvp-timing-sample.epub /tmp/timing-sample + + firmware: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + - run: python -m pip install platformio + - run: pio run diff --git a/README.md b/README.md index d4fb08c..a4893e3 100644 --- a/README.md +++ b/README.md @@ -99,18 +99,19 @@ Chapter 2 Generate this layout from an EPUB with the included converter: ```bash -python3 -m pip install -r requirements.txt -python3 epub_to_chapters.py book.epub "Example Book" +./.venv/bin/python -m pip install -r requirements.txt +./.venv/bin/python epub_to_chapters.py book.epub "Example Book" ``` For a quick end-to-end check, the repository includes a two-chapter sample with EPUB styles, punctuation, and repeated invented names: ```bash -python3 epub_to_chapters.py examples/rsvp-timing-sample.epub "Timing Sample" +./.venv/bin/python epub_to_chapters.py examples/rsvp-timing-sample.epub "Timing Sample" ``` -Copy the resulting `Example Book` directory into `/books` on the card. A +The output directory must be empty unless `--overwrite` is supplied. Copy the +resulting `Example Book` directory into `/books` on the card. A section may be up to 1.5 MiB. The converter writes `CONVERSION_REPORT.txt` in the book directory. It records any unsupported non-ASCII characters left after normalizing common punctuation, ligatures, accents, whitespace, fractions, and @@ -220,12 +221,12 @@ GPIO12 wakes it. ## Resume and power behavior -The reader saves the selected section, WPM, brightness, and font settings in -the ESP32's nonvolatile storage. On the next boot or wake it opens the same -section's title card and remains paused; pressing the touch-edge Home button -begins at that section's first word. It defers full chapter-list validation -until chapter browsing is needed, so normal startup opens only the selected -chapter and its sidecar. +The reader saves the selected section, paragraph-level position, WPM, +brightness, and font settings in the ESP32's nonvolatile storage. On the next +boot or wake it opens the same section's title card and remains paused; +pressing the touch-edge Home button begins at the saved paragraph. It validates +the saved section against its `TOC.txt` before opening it, then falls back to +the first available book if the card has changed. The physical power switch only disconnects the battery. USB power is sufficient for development and flashing. The reader intentionally has no Wi-Fi, Bluetooth, diff --git a/TODO.md b/TODO.md index cff1a97..0bbc590 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,29 @@ # TODO +## Bookmarks and per-book progress + +Paragraph-level resume restores the last reading position. Add named bookmarks +and a compact recent-books/progress list so readers can return to several +places in a book without overwriting their normal resume point. + +## Reading profiles + +Add selectable reading profiles for punctuation and paragraph pause lengths, +long-word delay, unfamiliar-word timing, and WPM presets. Keep a plain/default +profile that preserves the current cadence. + +## Converter preflight and safe section splitting + +Add a `--check` mode that reports section and sidecar sizes, filename issues, +and unsupported characters without writing output. Offer optional splitting at +paragraph boundaries for sections that exceed the device's 1.5 MiB file limit. + +## Memory-limit hardware validation + +Stress-test the advertised maximum section and sidecar sizes on real hardware, +record PSRAM headroom, and lower the supported limit if token indexing or +sidecar metadata leaves insufficient memory for reliable reading. + ## Add anti-aliased reader fonts The current reader uses TFT_eSPI's bitmap FreeSans, FreeSerif, and FreeMono diff --git a/epub_to_chapters.py b/epub_to_chapters.py index 3f6541d..3a4d689 100644 --- a/epub_to_chapters.py +++ b/epub_to_chapters.py @@ -122,6 +122,8 @@ SIDECAR_HEADER = struct.Struct("<4sBBQI") CSS_RULE = re.compile(r"([^{}]+)\{([^{}]*)\}") CSS_CLASS = re.compile(r"\.([A-Za-z_-][A-Za-z0-9_-]*)") WORD_LOOKUP_KEY = re.compile(r"^[^A-Za-z]*([A-Za-z]+(?:['-][A-Za-z]+)*)[^A-Za-z]*$") +MAX_SECTION_BYTES = 1500 * 1024 +MAX_SECTION_NAME_BYTES = 200 TIMING_NONE = 0 TIMING_DOUBLE = 2 @@ -412,8 +414,8 @@ def encode_varint(value): return bytes(encoded) -def write_style_sidecar(output_file, text, style_records): - """Write validated sparse style and timing metadata beside a chapter.""" +def style_sidecar_bytes(text, style_records): + """Return validated sparse style and timing metadata for a chapter.""" payload = bytearray(SIDECAR_HEADER.pack( SIDECAR_MAGIC, @@ -437,7 +439,13 @@ def write_style_sidecar(output_file, text, style_records): payload.extend((style_flags, timing_modifier)) previous_index = word_index - output_file.write_bytes(payload) + return bytes(payload) + + +def write_style_sidecar(output_file, text, style_records): + """Write validated sparse style and timing metadata beside a chapter.""" + + output_file.write_bytes(style_sidecar_bytes(text, style_records)) def output_section_name(title, number, used_names): @@ -453,10 +461,19 @@ def output_section_name(title, number, used_names): if not title: title = f"Section {number}" + encoded_title = title.encode("utf-8") + if len(encoded_title) > MAX_SECTION_NAME_BYTES: + title = encoded_title[:MAX_SECTION_NAME_BYTES].decode( + "utf-8", errors="ignore" + ).rstrip(". ") + candidate = title duplicate = 2 while candidate.casefold() in used_names: - candidate = f"{title} ({duplicate})" + suffix = f" ({duplicate})" + available = MAX_SECTION_NAME_BYTES - len(suffix.encode("utf-8")) + stem = title.encode("utf-8")[:available].decode("utf-8", errors="ignore") + candidate = f"{stem.rstrip('. ')}{suffix}" duplicate += 1 used_names.add(candidate.casefold()) @@ -995,103 +1012,6 @@ def spine_documents(book): return result -# --------------------------------------------------------------------------- -# Chapter extraction using TOC -# --------------------------------------------------------------------------- - -def extract_toc_chapter(entries, index, class_styles=None): - """ - Extract one chapter based on its TOC position. - - This handles: - - chapter.xhtml#chapter1 - chapter.xhtml#chapter2 - - correctly by stopping at the next TOC anchor. - - It also handles chapters spanning multiple XHTML files. - """ - - current = entries[index] - - next_entry = ( - entries[index + 1] - if index + 1 < len(entries) - else None - ) - - current_item = current["item"] - - current_soup = parse_document(current_item, class_styles) - current_root = document_root(current_soup) - - start = find_fragment( - current_soup, - current["fragment"], - ) - - # --------------------------------------------------------------- - # Find next chapter boundary if it is in this same document. - # --------------------------------------------------------------- - - end = None - - if ( - next_entry is not None - and same_document(current_item, next_entry["item"]) - ): - end = find_fragment( - current_soup, - next_entry["fragment"], - ) - - # --------------------------------------------------------------- - # Same XHTML file. - # --------------------------------------------------------------- - - if next_entry is not None and same_document( - current_item, - next_entry["item"], - ): - - return extract_range( - current_root, - start, - end, - ) - - # --------------------------------------------------------------- - # Different XHTML file. - # - # Everything from the current starting point through the end - # of the current document belongs to this chapter. - # --------------------------------------------------------------- - - parts = [] - - parts.append( - extract_range( - current_root, - start, - None, - ) - ) - - # If there is no next chapter, we stop here. - if next_entry is None: - return normalize_text("\n\n".join( - part for part in parts if part - )) - - # Add any spine documents between current and next document. - spine = spine_documents(book=None) if False else [] - - return normalize_text("\n\n".join( - part for part in parts if part - )) - - # --------------------------------------------------------------------------- # Improved multi-document chapter extraction # --------------------------------------------------------------------------- @@ -1170,10 +1090,11 @@ def extract_chapter(book, entries, index, doc_positions, class_styles=None): pieces = [] - # Process documents from current through the document before - # the next chapter. + # Process documents from current through the document before the next + # chapter. A fragment in the current document is a special case: include + # that document once and stop at the next fragment. end_pos = ( - next_pos + next_pos + 1 if next_pos == current_pos and next_entry is not None else next_pos if next_pos is not None else len(docs) ) @@ -1321,7 +1242,7 @@ def remove_duplicate_chapters(chapters): # Main conversion # --------------------------------------------------------------------------- -def convert_epub(epub_path, output_dir): +def convert_epub(epub_path, output_dir, overwrite=False): epub_path = Path(epub_path) output_dir = Path(output_dir) @@ -1333,10 +1254,21 @@ def convert_epub(epub_path, output_dir): ) sys.exit(1) - output_dir.mkdir( - parents=True, - exist_ok=True, - ) + if output_dir.exists() and any(output_dir.iterdir()): + if not overwrite: + print( + f"Error: output directory is not empty: {output_dir}. " + "Choose an empty directory or pass --overwrite." + ) + sys.exit(1) + for output_file in output_dir.iterdir(): + if output_file.is_file() and ( + output_file.suffix in {".txt", ".rsvp"} or + output_file.name == "CONVERSION_REPORT.txt" + ): + output_file.unlink() + + output_dir.mkdir(parents=True, exist_ok=True) print(f"Reading: {epub_path}") @@ -1453,7 +1385,7 @@ def convert_epub(epub_path, output_dir): # --------------------------------------------------------------- toc_entries = [] - written_sections = [] + prepared_sections = [] used_names = set() for number, chapter in enumerate( @@ -1467,29 +1399,30 @@ def convert_epub(epub_path, output_dir): used_names, ) - output_file = ( - output_dir / - f"{title}.txt" - ) - - output_file.write_text( - chapter["text"], - encoding="utf-8", - ) - - sidecar_file = output_dir / f"{title}.rsvp" - write_style_sidecar( - sidecar_file, - chapter["text"], - chapter["style_records"], - ) - toc_entries.append(title) + prepared_sections.append((title, chapter)) + + for title, chapter in prepared_sections: + text_bytes = chapter["text"].encode("utf-8") + sidecar_bytes = style_sidecar_bytes( + chapter["text"], chapter["style_records"] + ) + if len(text_bytes) > MAX_SECTION_BYTES or len(sidecar_bytes) > MAX_SECTION_BYTES: + print( + f"Error: {title!r} exceeds the device's {MAX_SECTION_BYTES} byte " + "per-file limit. Split the section before converting." + ) + sys.exit(1) + + written_sections = [] + for number, (title, chapter) in enumerate(prepared_sections, start=1): + output_file = output_dir / f"{title}.txt" + sidecar_file = output_dir / f"{title}.rsvp" + output_file.write_text(chapter["text"], encoding="utf-8") + write_style_sidecar(sidecar_file, chapter["text"], chapter["style_records"]) written_sections.append((output_file.name, chapter["text"])) print( - f"{number:4}. " - f"{title} " - f"-> {output_file.name}, {sidecar_file.name}" + f"{number:4}. {title} -> {output_file.name}, {sidecar_file.name}" ) toc_file = output_dir / "TOC.txt" @@ -1529,20 +1462,24 @@ def convert_epub(epub_path, output_dir): # --------------------------------------------------------------------------- def main(): + arguments = sys.argv[1:] + overwrite = "--overwrite" in arguments + if overwrite: + arguments.remove("--overwrite") - if len(sys.argv) < 2: + if not arguments or len(arguments) > 2: print( "Usage:\n" " python epub_to_chapters.py book.epub\n" - " python epub_to_chapters.py " - "book.epub output_directory" + " python epub_to_chapters.py book.epub output_directory\n" + " python epub_to_chapters.py book.epub output_directory --overwrite" ) sys.exit(1) epub_path = Path( - sys.argv[1] + arguments[0] ) if not epub_path.exists(): @@ -1563,10 +1500,10 @@ def main(): sys.exit(1) - if len(sys.argv) >= 3: + if len(arguments) >= 2: output_dir = Path( - sys.argv[2] + arguments[1] ) else: @@ -1575,10 +1512,7 @@ def main(): "chapters" ) - convert_epub( - epub_path, - output_dir, - ) + convert_epub(epub_path, output_dir, overwrite) if __name__ == "__main__": diff --git a/platformio.ini b/platformio.ini index bb20777..c16717d 100644 --- a/platformio.ini +++ b/platformio.ini @@ -8,11 +8,10 @@ board = t-display-s3-pro framework = arduino monitor_speed = 115200 board_build.partitions = partitions.csv -lib_extra_dirs = T-Display-S3-Pro/lib build_flags = -DARDUINO_USB_CDC_ON_BOOT=1 -DDISABLE_ALL_LIBRARY_WARNINGS lib_deps = - TFT_eSPI + TFT_eSPI@2.5.43 lewisxhe/SensorLib@0.4.0 lewisxhe/XPowersLib@0.2.9 diff --git a/requirements.txt b/requirements.txt index f1abb10..f332b82 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ -ebooklib -beautifulsoup4 +ebooklib==0.20 +beautifulsoup4==4.15.0 english-words==2.0.2 diff --git a/src/Library.cpp b/src/Library.cpp index eda8f0e..110a5b4 100644 --- a/src/Library.cpp +++ b/src/Library.cpp @@ -22,7 +22,8 @@ bool Library::loadLastOrFirst(String& path, String& contents) const { Settings settings; settings.begin(); const SavedState state = settings.load(); - if (isChapterPath(state.bookPath) && loadFile(state.bookPath, contents)) { + if (isChapterPath(state.bookPath) && isListedChapter(state.bookPath) && + loadFile(state.bookPath, contents)) { path = state.bookPath; return true; } diff --git a/src/Settings.cpp b/src/Settings.cpp index 2a59360..8db2346 100644 --- a/src/Settings.cpp +++ b/src/Settings.cpp @@ -25,11 +25,25 @@ SavedState Settings::load() const { void Settings::save(const SavedState& state) const { Preferences preferences; preferences.begin(kNamespace, false); - preferences.putString("book", state.bookPath); - preferences.putUInt("word", state.wordIndex); - preferences.putUShort("wpm", state.wpm); - preferences.putUChar("brightness", state.brightness); - preferences.putUChar("font_style", state.fontStyle); - preferences.putUChar("font_size", state.fontSize); + if (!preferences.isKey("book") || preferences.getString("book") != state.bookPath) { + preferences.putString("book", state.bookPath); + } + if (!preferences.isKey("word") || preferences.getUInt("word") != state.wordIndex) { + preferences.putUInt("word", state.wordIndex); + } + if (!preferences.isKey("wpm") || preferences.getUShort("wpm") != state.wpm) { + preferences.putUShort("wpm", state.wpm); + } + if (!preferences.isKey("brightness") || + preferences.getUChar("brightness") != state.brightness) { + preferences.putUChar("brightness", state.brightness); + } + if (!preferences.isKey("font_style") || + preferences.getUChar("font_style") != state.fontStyle) { + preferences.putUChar("font_style", state.fontStyle); + } + if (!preferences.isKey("font_size") || preferences.getUChar("font_size") != state.fontSize) { + preferences.putUChar("font_size", state.fontSize); + } preferences.end(); } diff --git a/src/main.cpp b/src/main.cpp index 2a4a971..8cdf000 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -23,6 +23,8 @@ bool displayIsOff = false; bool displayIsDimmed = false; uint32_t lastInteractionMs = 0; uint32_t nextWordMs = 0; +uint32_t lastPersistMs = 0; +bool stateDirty = false; TouchDrvCSTXXX touch; bool touchAvailable = false; bool touchPlayPauseRequested = false; @@ -85,9 +87,21 @@ bool released(Button& button) { return wasPressed && button.stable == HIGH; } -void persist() { - state.wordIndex = reader.currentIndex(); +void markStateDirty() { stateDirty = true; } + +void persist(bool force = false) { + const uint32_t currentWord = reader.currentIndex(); + if (state.wordIndex != currentWord) { + state.wordIndex = currentWord; + stateDirty = true; + } + constexpr uint32_t kProgressCheckpointMs = 30UL * 1000UL; + if (!stateDirty || (!force && millis() - lastPersistMs < kProgressCheckpointMs)) { + return; + } settings.save(state); + stateDirty = false; + lastPersistMs = millis(); } void setReadingContext(uint8_t progressPercent) { @@ -130,6 +144,7 @@ bool loadChapter(const String& path) { state.bookPath = path; state.wordIndex = 0; reader.restoreAtParagraphStart(0); + markStateDirty(); readerView = ReaderView::kChapterTitle; playing = false; return true; @@ -231,7 +246,8 @@ void applyPicker() { } else { loadChapter(pendingChapterPath); } - persist(); + markStateDirty(); + persist(true); returnToReader(); } @@ -240,14 +256,14 @@ void togglePlayback() { if (readerView == ReaderView::kTheEnd) return; if (readerView == ReaderView::kChapterTitle) { - reader.restoreAtParagraphStart(0); + reader.restoreAtParagraphStart(state.wordIndex); readerView = ReaderView::kWord; playing = true; showCurrentWord(); } else { playing = !playing; } - persist(); + persist(true); if (playing) nextWordMs = millis() + reader.delayMs(state.wpm); } @@ -283,7 +299,7 @@ void navigatePreviousPaused() { void navigateNextPaused() { switch (readerView) { case ReaderView::kChapterTitle: - reader.restoreAtParagraphStart(0); + reader.restoreAtParagraphStart(state.wordIndex); readerView = ReaderView::kWord; showReaderScreen(); break; @@ -372,7 +388,7 @@ void wakeDisplay() { } void enterDeepSleep() { - persist(); + persist(true); board.displayOff(); // GPIO12 is RTC-capable on ESP32-S3. A press (active low) wakes the reader. esp_sleep_enable_ext0_wakeup(static_cast(config::kButtonPrevious), 0); @@ -405,11 +421,12 @@ void handleInput() { state.wpm = state.wpm > config::kMinimumWpm + config::kWpmStep ? state.wpm - config::kWpmStep : config::kMinimumWpm; + markStateDirty(); showCurrentWord(); } else { navigatePreviousPaused(); } - persist(); + persist(true); } if (released(next)) { wakeDisplay(); @@ -426,11 +443,12 @@ void handleInput() { state.wpm = state.wpm + config::kWpmStep < config::kMaximumWpm ? state.wpm + config::kWpmStep : config::kMaximumWpm; + markStateDirty(); showCurrentWord(); } else { navigateNextPaused(); } - persist(); + persist(true); } } } // namespace @@ -444,24 +462,27 @@ void setup() { pinMode(config::kButtonNext, INPUT_PULLUP); settings.begin(); state = settings.load(); + bool sanitizedSettings = false; if (state.wpm < config::kMinimumWpm || state.wpm > config::kMaximumWpm) { state.wpm = config::kDefaultWpm; + sanitizedSettings = true; } if (state.brightness < 1 || state.brightness > 16) { state.brightness = config::kNormalBrightness; + sanitizedSettings = true; } if (state.fontStyle > 2) { state.fontStyle = config::kDefaultFontStyle; + sanitizedSettings = true; } if (state.fontSize < config::kMinimumFontSize || state.fontSize > config::kMaximumFontSize || state.fontSize % config::kFontSizeStep != 0) { state.fontSize = config::kDefaultFontSize; + sanitizedSettings = true; } - if (!board.beginSd()) { - while (true) delay(1000); - } + const bool sdAvailable = board.beginSd(); if (!board.begin()) { while (true) delay(1000); } @@ -469,6 +490,11 @@ void setup() { board.setFont(state.fontStyle, state.fontSize); beginTouch(); + if (!sdAvailable) { + board.drawWord("SD CARD ERROR", state.wpm); + while (true) delay(1000); + } + String book; std::vector sidecar; if (!library.loadLastOrFirst(state.bookPath, book)) { @@ -480,8 +506,11 @@ void setup() { board.drawWord("NO BOOKS", state.wpm); while (true) delay(1000); } - state.wordIndex = 0; - reader.restoreAtParagraphStart(0); + const uint32_t savedWord = state.wordIndex; + reader.restoreAtParagraphStart(savedWord); + state.wordIndex = reader.currentIndex(); + stateDirty = sanitizedSettings || state.wordIndex != savedWord; + persist(true); readerView = ReaderView::kChapterTitle; showReaderScreen(); lastInteractionMs = millis(); diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..d0c1f3f --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1 @@ +"""Test package for unittest discovery.""" diff --git a/tests/test_epub_to_chapters.py b/tests/test_epub_to_chapters.py index e2eb1f6..1fff5cb 100644 --- a/tests/test_epub_to_chapters.py +++ b/tests/test_epub_to_chapters.py @@ -15,7 +15,9 @@ from epub_to_chapters import ( add_unfamiliar_word_timing, decode_styled_text, document_root, + extract_chapter, html_to_text, + item_index_map, normalize_display_text, normalize_text, output_section_name, @@ -58,6 +60,11 @@ class DisplayTextNormalizationTests(unittest.TestCase): "Cafe - Part 1-2", ) + def test_section_names_fit_within_the_device_filename_budget(self): + name = output_section_name("A" * 300, 1, set()) + + self.assertLessEqual(len(name.encode("utf-8")), 200) + class ConversionReportTests(unittest.TestCase): def test_reports_remaining_non_ascii_characters_by_generated_file(self): @@ -234,5 +241,52 @@ class UnfamiliarWordTimingTests(unittest.TestCase): self.assertEqual(chapters[0]["style_records"], []) +class SameDocumentTocTests(unittest.TestCase): + def test_extracts_each_fragment_when_two_toc_entries_share_a_document(self): + class FakeItem: + def __init__(self, name, content): + self.name = name + self.content = content + + def get_content(self): + return self.content.encode("utf-8") + + def get_name(self): + return self.name + + def get_id(self): + return self.name + + def get_type(self): + from epub_to_chapters import ITEM_DOCUMENT + return ITEM_DOCUMENT + + class FakeBook: + def __init__(self, item): + self.item = item + self.spine = [(item.get_id(),)] + + def get_items(self): + return [self.item] + + item = FakeItem( + "chapter.xhtml", + "

One

First text.

" + "

Two

Second text.

", + ) + book = FakeBook(item) + entries = [ + {"title": "One", "item": item, "fragment": "one"}, + {"title": "Two", "item": item, "fragment": "two"}, + ] + positions = item_index_map(book) + + first, _ = decode_styled_text(extract_chapter(book, entries, 0, positions)) + second, _ = decode_styled_text(extract_chapter(book, entries, 1, positions)) + + self.assertEqual(first, "One\nFirst text.") + self.assertEqual(second, "Two\nSecond text.") + + if __name__ == "__main__": unittest.main()