refactor: dev lut

[pablohc]

Summary

• What is the goal of this PR? (e.g., Implements the new feature for file uploading.)
• What changes are included?

Additional Context

• Add any other information that might be helpful for the reviewer (e.g., performance implications, potential risks,
  specific areas to focus on).

---

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 | PARTIALLY | NO >

Summary by CodeRabbit

• New Features

  • .pxc grayscale viewer with sleep-cover support; power-button menu for quick Sleep/Refresh/Screenshot.
  • New "XTC Render Quality" setting.

• Improvements

  • Enhanced grayscale rendering and factory-quality display paths; improved 2-bit grayscale screenshots.
  • UI/layout and wallpaper/file support improvements (including .pxc).
  • EPUB/web conversion now supports device-targeted optimization.

• Settings

  • Added 3-minute sleep option and "Move finished books to Read" setting.

• Localization

  • Added translation strings for new UI entries across many languages.

[coderabbitai]

Actionable comments posted: 14

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/activities/reader/KOReaderSyncActivity.cpp (1)

296-303: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Only tear down Wi‑Fi when this activity actually brought it up.

wifiActivated is set before the already-connected fast path, so entering this screen while Wi‑Fi is already up will still disconnect that existing session and force a silent restart on exit. That makes KOReaderSyncActivity steal ownership from the caller that originally enabled Wi‑Fi.

Suggested fix

-  // Past this point every path uses WiFi.
-  wifiActivated = true;
-
   // Check if already connected (e.g. from settings page auth)
   if (WiFi.status() == WL_CONNECTED) {
     LOG_DBG("KOSync", "Already connected to WiFi");
     onWifiSelectionComplete(true);
     return;
   }
+
+  // We only own WiFi teardown when this activity had to bring it up.
+  wifiActivated = true;

Also applies to: 315-319

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/activities/reader/KOReaderSyncActivity.cpp` around lines 296 - 303, The
code sets wifiActivated before taking the "already connected" fast path, causing
KOReaderSyncActivity to later tear down Wi‑Fi it did not start; move the
assignment of wifiActivated = true so it only runs after you know this activity
actually enabled Wi‑Fi (i.e., after the branch that checks WiFi.status() ==
WL_CONNECTED and returns), and ensure any teardown logic (the code paths
referenced around onWifiSelectionComplete and the block at the other mentioned
spot) checks wifiActivated before disabling or restarting Wi‑Fi so only sessions
started by KOReaderSyncActivity are stopped.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/Epub/Epub.cpp`:
- Around line 641-642: Check the result of
readItemContentsToStream(coverImageHref, coverJpg, 1024, deadline) (and the
similar call at the 678-679 spot) and abort the subsequent cover-conversion path
if the stream read failed or the deadline was exceeded: if the read returns a
failure/false or an exception occurs, ensure the temp file is closed/removed and
return or throw early instead of calling the cover conversion routine; update
the code around readItemContentsToStream, coverImageHref and coverJpg to gate
the conversion step on a successful read and propagate/handle the error
appropriately.

In `@lib/Epub/Epub/hyphenation/HyphenationCommon.cpp`:
- Around line 23-25: The parity-based Latin Extended-A lowering incorrectly maps
U+0130 to U+0131; update the parity-based lowering block to special-case cp ==
0x0130 and return 0x0069 before applying the existing parity rules: check if (cp
== 0x0130) return 0x0069; then keep the existing if ((cp >= 0x0100 ... ) ...) {
return cp + 1; } logic so U+0130 maps to LATIN SMALL LETTER I (0x0069) instead
of U+0131.

In `@lib/I18n/translations/russian.yaml`:
- Line 321: Fix the typo in the Russian translation for the font installation
failure message by updating the value of STR_FONT_INSTALL_FAILED to "Не удалось
установить шрифт" (correcting "удалоь" → "удалось") so the user-facing string is
spelled correctly.
- Line 347: The translation string STR_KB_HINT_CLEAR_TEXT contains a typo:
replace the incorrect word "очитисть" with the correct "очистить" so the value
becomes "Удерживайте <— чтобы очистить весь текст"; update the value for the
STR_KB_HINT_CLEAR_TEXT key in the translations file accordingly.

In `@src/activities/network/CalibreConnectActivity.cpp`:
- Around line 55-59: The current exit path in CalibreConnectActivity.cpp calls
silentRestart() whenever WiFi is active, causing a full device reboot on normal
exit; change the flow so WiFi.getMode() != WIFI_MODE_NULL only triggers
WiFi.disconnect(false) and delay(30) and then returns/continues without calling
silentRestart(). Remove the unconditional silentRestart() from this normal-exit
branch (or move it behind an explicit fatal-error condition), and ensure any
remaining reboot behavior is only invoked from an explicit error handler, not
from the normal exit code in this activity.

In `@src/activities/network/CrossPointWebServerActivity.cpp`:
- Around line 79-87: The reboot path currently jumps to silentRestart() without
shutting down the web stack; call the CrossPointWebServer stop routine before
reboot so active WebSocket uploads are aborted and partial files cleaned up.
Locate the block around WiFi.getMode() and, before calling silentRestart(),
invoke the web server cleanup (e.g. CrossPointWebServer::stop() or the instance
method like crossPointWebServer.stop()), guarding for a null/uninitialized
instance or already-stopped state so the call is safe; ensure this happens
regardless of isApMode/softAPdisconnect vs WiFi.disconnect().

In `@src/activities/settings/FontDownloadActivity.cpp`:
- Around line 458-477: When retrying a failed single-family download the
progress counters aren't reset, so ensure you set currentFileIndex_ = 0 and
currentFileTotal_ = family.files.size() before calling downloadFamily(family) on
retry; locate the retry branch that currently calls downloadFamily() directly
(the same branch that handles non-installed or hasUpdate when using
familyIndexFromList(selectedIndex_)) and add the same counter initialization
used elsewhere (the blocks that call downloadAll() and updateAll()) so progress
text reflects the restarted download.
- Around line 191-197: updateAll() is calling downloadFamily() for families_[i]
with hasUpdate set, but downloadFamily() currently uses the destructive install
path that deletes an existing family on abort/failure; change the update flow so
that updates are non-destructive: modify downloadFamily() (or add a new
downloadFamilyForUpdate() overload/flag) to download into a temporary location
and only replace the installed family on successful completion, and on
abort/failure/cancel leave the existing installed family untouched; update
callers (updateAll(), and other places that invoked downloadFamily() for
updates) to pass the update-mode flag or call the new non-destructive helper,
and ensure error/cancel branches do not call the deletion/cleanup that removes
the existing installed font.

In `@src/components/themes/BaseTheme.cpp`:
- Around line 459-474: When drawBitmap(bitmap, ...) returns false, clear the
cached-cover state so stale buffers aren't reused: in the else branch where
hasCoverImage is set to false and book dimensions recomputed, also set
coverRendered = false and coverBufferStored = false (and if there's a cover
buffer pointer/handle, reset/clear it too); update the else branch near the
drawBitmap call and ensure the symbols coverRendered, coverBufferStored, and any
cover buffer handle are reset there.

In `@src/components/themes/lyra/Lyra3CoversTheme.cpp`:
- Around line 49-68: The code currently leaves hasCover true when
Storage.openFileForRead("HOME", coverBmpPath, file) returns false; update the
open-failure branch in Lyra3CoversTheme.cpp (the block using
Storage.openFileForRead, Bitmap, renderer.drawBitmap and the hasCover flag) to
explicitly set hasCover = false when openFileForRead fails, and only call
file.close() or proceed with Bitmap parsing/drawing when the file was
successfully opened (ensure the early-fail path does not attempt to use file or
skip the empty-cover fallback).

In `@src/components/UITheme.cpp`:
- Around line 74-104: UITheme::getScreenSafeArea currently ignores the
hasSideButtonHints parameter, so side button hint areas are not subtracted from
the returned safeArea; update the switch handling in UITheme::getScreenSafeArea
to also subtract currentMetrics->buttonHintsHeight from the appropriate edges
when hasSideButtonHints is true (for LandscapeClockwise and
LandscapeCounterClockwise adjust x/width appropriately; for
Portrait/PortraitInverted adjust y/height if side hints apply there), preserving
the existing hasFrontButtonHints logic and using the same
currentMetrics->buttonHintsHeight value.

In `@src/main.cpp`:
- Around line 510-515: Menu-triggered screenshots use the normal framebuffer
path unconditionally, causing blank/incorrect images in the FactoryLut display
state; update the PowerButtonMenuActivity::MenuAction::SCREENSHOT case so after
acquiring RenderLock it checks renderer->getDisplayState() for
GfxRenderer::DisplayState::FactoryLut and, if so, invoke the same
FactoryLut-specific screenshot logic used by the combo handler (mirror that code
path), otherwise call ScreenshotUtil::takeScreenshot(renderer); keep RenderLock
scope the same and reuse the exact FactoryLut-handling function or code block
the combo handler uses to ensure identical behavior.

In `@src/network/html/FilesPage.html`:
- Around line 4906-4908: The code currently sets ok = !conversionFailed &&
conversionSucceeded which treats fallback-to-original as a failure; instead,
mark the file as successful because this block runs after a successful upload.
Replace the ok calculation (in the save-to-log path using saveToFileBatchLog and
the variables conversionFailed, conversionSucceeded, convOriginalSize,
convNewSize) so the file is considered successful (ok = true) and pass sizes
accordingly: use convOriginalSize as the original size and use convNewSize only
when conversionSucceeded, otherwise use convOriginalSize for the delivered size
so fallback uploads are not counted as failed.
- Around line 4430-4440: The code decodes src with decodeHref for matching but
then writes back decodedSrc which strips percent-encodings; instead, keep the
original encoded src for the DOM update: use decodeHref(src) and
resolvePath(xhtmlPath, decodedSrc) to find a matching entry in renamed, but when
updating img.setAttribute('src') replace only the last path segment of the
original src (the one after the final '/') with the new basename
(newPath.split('/').pop()) and URL-encode that basename (e.g. via
encodeURIComponent) before inserting so percent-encoding is preserved; update
modified = true as before.

---

Outside diff comments:
In `@src/activities/reader/KOReaderSyncActivity.cpp`:
- Around line 296-303: The code sets wifiActivated before taking the "already
connected" fast path, causing KOReaderSyncActivity to later tear down Wi‑Fi it
did not start; move the assignment of wifiActivated = true so it only runs after
you know this activity actually enabled Wi‑Fi (i.e., after the branch that
checks WiFi.status() == WL_CONNECTED and returns), and ensure any teardown logic
(the code paths referenced around onWifiSelectionComplete and the block at the
other mentioned spot) checks wifiActivated before disabling or restarting Wi‑Fi
so only sessions started by KOReaderSyncActivity are stopped.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 88c20477-194a-48df-847b-e7af688728b4

📥 Commits

Reviewing files that changed from the base of the PR and between f896238 and ed1f342.

⛔ Files ignored due to path filters (2)

• lib/Epub/Epub/hyphenation/generated/hyph-pl.trie.h is excluded by !**/generated/**
• lib/Epub/Epub/hyphenation/generated/hyph-sv.trie.h is excluded by !**/generated/**

📒 Files selected for processing (73)

• .skills/SKILL.md
• README.md
• lib/EpdFont/SdCardFont.cpp
• lib/EpdFont/SdCardFont.h
• lib/EpdFont/scripts/sd-fonts.yaml
• lib/Epub/Epub.cpp
• lib/Epub/Epub.h
• lib/Epub/Epub/ParsedText.cpp
• lib/Epub/Epub/hyphenation/HyphenationCommon.cpp
• lib/Epub/Epub/hyphenation/Hyphenator.cpp
• lib/Epub/Epub/hyphenation/LanguageRegistry.cpp
• lib/GfxRenderer/GfxRenderer.cpp
• lib/GfxRenderer/GfxRenderer.h
• lib/I18n/translations/english.yaml
• lib/I18n/translations/italian.yaml
• lib/I18n/translations/russian.yaml
• lib/I18n/translations/spanish.yaml
• lib/JpegToBmpConverter/JpegToBmpConverter.cpp
• lib/JpegToBmpConverter/JpegToBmpConverter.h
• lib/KOReaderSync/KOReaderSyncClient.cpp
• lib/Memory/Memory.h
• lib/PngToBmpConverter/PngToBmpConverter.cpp
• lib/PngToBmpConverter/PngToBmpConverter.h
• lib/ZipFile/ZipFile.cpp
• lib/ZipFile/ZipFile.h
• lib/hal/HalGPIO.cpp
• lib/hal/HalGPIO.h
• platformio.ini
• scripts/update_hyphenation.sh
• src/CrossPointSettings.h
• src/CrossPointState.h
• src/SdCardFontGlobals.h
• src/SdCardFontSystem.h
• src/SettingsList.h
• src/SilentRestart.h
• src/activities/ActivityManager.cpp
• src/activities/boot_sleep/BootActivity.cpp
• src/activities/boot_sleep/SleepActivity.cpp
• src/activities/browser/OpdsBookBrowserActivity.cpp
• src/activities/network/CalibreConnectActivity.cpp
• src/activities/network/CrossPointWebServerActivity.cpp
• src/activities/network/CrossPointWebServerActivity.h
• src/activities/reader/KOReaderSyncActivity.cpp
• src/activities/reader/KOReaderSyncActivity.h
• src/activities/reader/ReaderActivity.cpp
• src/activities/reader/TxtReaderActivity.cpp
• src/activities/settings/FontDownloadActivity.cpp
• src/activities/settings/FontDownloadActivity.h
• src/activities/settings/FontSelectionActivity.cpp
• src/activities/settings/LanguageSelectActivity.cpp
• src/activities/settings/SettingsActivity.cpp
• src/activities/util/PowerButtonMenuActivity.cpp
• src/activities/util/PowerButtonMenuActivity.h
• src/components/UITheme.cpp
• src/components/UITheme.h
• src/components/icons/cog.h
• src/components/icons/settings.h
• src/components/themes/BaseTheme.cpp
• src/components/themes/BaseTheme.h
• src/components/themes/lyra/Lyra3CoversTheme.cpp
• src/components/themes/lyra/LyraTheme.cpp
• src/components/themes/lyra/LyraTheme.h
• src/components/themes/roundedraff/RoundedRaffTheme.cpp
• src/components/themes/roundedraff/RoundedRaffTheme.h
• src/images/CrossLarge.h
• src/main.cpp
• src/network/CrossPointWebServer.cpp
• src/network/HttpDownloader.cpp
• src/network/HttpDownloader.h
• src/network/html/FilesPage.html
• test/hyphenation_eval/HyphenationEvaluationTest.cpp
• test/hyphenation_eval/resources/polish_hyphenation_tests.txt
• test/hyphenation_eval/resources/swedish_hyphenation_tests.txt

💤 Files with no reviewable changes (5)

• src/SdCardFontGlobals.h
• src/components/icons/cog.h
• src/activities/ActivityManager.cpp
• src/components/icons/settings.h
• src/activities/network/CrossPointWebServerActivity.h

✅ Files skipped from review due to trivial changes (6)

• src/activities/settings/SettingsActivity.cpp
• src/SdCardFontSystem.h
• platformio.ini
• src/images/CrossLarge.h
• scripts/update_hyphenation.sh
• lib/I18n/translations/english.yaml

🚧 Files skipped from review as they are similar to previous changes (6)

• src/activities/reader/ReaderActivity.cpp
• src/SettingsList.h
• src/CrossPointSettings.h
• src/components/UITheme.h
• src/activities/boot_sleep/SleepActivity.cpp
• lib/GfxRenderer/GfxRenderer.cpp

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle failed cover extraction before conversion.

If the deadline is exceeded (or stream read fails), conversion still proceeds on partial/invalid temp data. This defeats early cancellation and wastes work.

💡 Proposed fix

-    readItemContentsToStream(coverImageHref, coverJpg, 1024, deadline);
+    if (!readItemContentsToStream(coverImageHref, coverJpg, 1024, deadline)) {
+      coverJpg.close();
+      Storage.remove(coverJpgTempPath.c_str());
+      return false;
+    }
@@
-    readItemContentsToStream(coverImageHref, coverPng, 1024, deadline);
+    if (!readItemContentsToStream(coverImageHref, coverPng, 1024, deadline)) {
+      coverPng.close();
+      Storage.remove(coverPngTempPath.c_str());
+      return false;
+    }

Also applies to: 678-679

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/Epub/Epub.cpp` around lines 641 - 642, Check the result of
readItemContentsToStream(coverImageHref, coverJpg, 1024, deadline) (and the
similar call at the 678-679 spot) and abort the subsequent cover-conversion path
if the stream read failed or the deadline was exceeded: if the read returns a
failure/false or an exception occurs, ensure the temp file is closed/removed and
return or throw early instead of calling the cover conversion routine; update
the code around readItemContentsToStream, coverImageHref and coverJpg to gate
the conversion step on a successful read and propagate/handle the error
appropriately.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What is the Unicode simple lowercase mapping for U+0130 (LATIN CAPITAL LETTER I WITH DOT ABOVE)?

💡 Result:

The Unicode simple lowercase mapping for U+0130 (LATIN CAPITAL LETTER I WITH DOT ABOVE) is U+0069 (LATIN SMALL LETTER I) [1]. This is the “simple” (locale-neutral) lowercase mapping from the Unicode character’s simple lowercase mapping data: U+0130 → U+0069 [1]. Note that Unicode also defines a special Turkish/Azeri locale-aware behavior where lowercasing U+0130 produces the two-code-point sequence U+0069 U+0307 (i plus combining dot above), but that is not the “simple” mapping asked for [2].

Citations:

• 1: https://codepoints.net/U+0130
• 2: http://www.unicode.org/faq/casemap_charprop

---

🏁 Script executed:

# First, locate the file
find . -name "HyphenationCommon.cpp" -type f

Repository: pablohc/crosspoint-reader

Length of output: 119

---

🏁 Script executed:

# Read the file around lines 23-25 to verify the code snippet
sed -n '1,50p' ./lib/Epub/Epub/hyphenation/HyphenationCommon.cpp | cat -n

Repository: pablohc/crosspoint-reader

Length of output: 1990

---

🏁 Script executed:

# Verify U+0130 falls into the problematic range and parity check
python3 << 'EOF'
cp = 0x0130
print(f"U+0130 = {cp} decimal")
print(f"In range 0x0100-0x0137? {0x0100 <= cp <= 0x0137}")
print(f"Parity (even)? {cp % 2 == 0}")
print(f"Would be mapped to: 0x{cp + 1:04X}")
print()
print("Correct mapping should be: 0x0069 (LATIN SMALL LETTER I)")
EOF

Repository: pablohc/crosspoint-reader

Length of output: 224

---

🏁 Script executed:

# Check if there are any existing special cases for U+0130 elsewhere in the function
grep -n "0x0130\|0x0131" ./lib/Epub/Epub/hyphenation/HyphenationCommon.cpp

Repository: pablohc/crosspoint-reader

Length of output: 51

---

Special-case U+0130 before parity-based Latin Extended-A lowering.

The parity rule maps U+0130 to U+0131, which violates Unicode case-mapping semantics. The correct simple lowercase mapping for U+0130 (LATIN CAPITAL LETTER I WITH DOT ABOVE) is U+0069 (LATIN SMALL LETTER I), not U+0131. This causes incorrect normalization of Turkish and other language text.

Proposed fix

 uint32_t toLowerLatinImpl(const uint32_t cp) {
+  // Unicode special casing: LATIN CAPITAL LETTER I WITH DOT ABOVE
+  // (simple lowercase mapping in single-codepoint form).
+  if (cp == 0x0130) {
+    return 0x0069;  // i
+  }
+
   if (cp >= 'A' && cp <= 'Z') {
     return cp - 'A' + 'a';
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/Epub/Epub/hyphenation/HyphenationCommon.cpp` around lines 23 - 25, The
parity-based Latin Extended-A lowering incorrectly maps U+0130 to U+0131; update
the parity-based lowering block to special-case cp == 0x0130 and return 0x0069
before applying the existing parity rules: check if (cp == 0x0130) return
0x0069; then keep the existing if ((cp >= 0x0100 ... ) ...) { return cp + 1; }
logic so U+0130 maps to LATIN SMALL LETTER I (0x0069) instead of U+0131.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix typo in installation failure message.

"Не удалоь установить шрифт" has a typo and should be "Не удалось установить шрифт".

Proposed fix

-STR_FONT_INSTALL_FAILED: "Не удалоь установить шрифт"
+STR_FONT_INSTALL_FAILED: "Не удалось установить шрифт"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	STR_FONT_INSTALL_FAILED: "Не удалоь установить шрифт"
	STR_FONT_INSTALL_FAILED: "Не удалось установить шрифт"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/I18n/translations/russian.yaml` at line 321, Fix the typo in the Russian
translation for the font installation failure message by updating the value of
STR_FONT_INSTALL_FAILED to "Не удалось установить шрифт" (correcting "удалоь" →
"удалось") so the user-facing string is spelled correctly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix typo in keyboard hint text.

"очитисть" should be "очистить".

Proposed fix

-STR_KB_HINT_CLEAR_TEXT: "Удерживайте <— чтобы очитисть весь текст"
+STR_KB_HINT_CLEAR_TEXT: "Удерживайте <— чтобы очистить весь текст"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	STR_KB_HINT_CLEAR_TEXT: "Удерживайте <— чтобы очитисть весь текст"
	STR_KB_HINT_CLEAR_TEXT: "Удерживайте <— чтобы очистить весь текст"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/I18n/translations/russian.yaml` at line 347, The translation string
STR_KB_HINT_CLEAR_TEXT contains a typo: replace the incorrect word "очитисть"
with the correct "очистить" so the value becomes "Удерживайте <— чтобы очистить
весь текст"; update the value for the STR_KB_HINT_CLEAR_TEXT key in the
translations file accordingly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid rebooting on normal Calibre activity exit.

This path now restarts the whole device whenever Wi‑Fi is active, so a normal “exit” becomes a full reboot. That’s a high-impact UX/regression risk for a routine navigation flow.

💡 Proposed fix

 void CalibreConnectActivity::onExit() {
   Activity::onExit();
 
-  if (WiFi.getMode() != WIFI_MODE_NULL) {
-    WiFi.disconnect(false);
-    delay(30);
-    silentRestart();
-  }
+  stopWebServer();
+  MDNS.end();
+  if (WiFi.getMode() != WIFI_MODE_NULL) {
+    WiFi.disconnect(true);
+    delay(30);
+  }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/activities/network/CalibreConnectActivity.cpp` around lines 55 - 59, The
current exit path in CalibreConnectActivity.cpp calls silentRestart() whenever
WiFi is active, causing a full device reboot on normal exit; change the flow so
WiFi.getMode() != WIFI_MODE_NULL only triggers WiFi.disconnect(false) and
delay(30) and then returns/continues without calling silentRestart(). Remove the
unconditional silentRestart() from this normal-exit branch (or move it behind an
explicit fatal-error condition), and ensure any remaining reboot behavior is
only invoked from an explicit error handler, not from the normal exit code in
this activity.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle failed cover file open explicitly.

When Storage.openFileForRead(...) fails, hasCover stays true, so the empty-cover fallback is skipped. Set hasCover = false on the open-failure path.

Proposed fix

             FsFile file;
             if (Storage.openFileForRead("HOME", coverBmpPath, file)) {
               Bitmap bitmap(file);
               if (bitmap.parseHeaders() == BmpReaderError::Ok) {
                 float coverHeight = static_cast<float>(bitmap.getHeight());
                 float coverWidth = static_cast<float>(bitmap.getWidth());
                 float ratio = coverWidth / coverHeight;
                 const float tileRatio = static_cast<float>(tileWidth - 2 * hPaddingInSelection) /
                                         static_cast<float>(Lyra3CoversMetrics::values.homeCoverHeight);
                 float cropX = 1.0f - (tileRatio / ratio);

                 if (!renderer.drawBitmap(bitmap, tileX + hPaddingInSelection, tileY + hPaddingInSelection,
                                          tileWidth - 2 * hPaddingInSelection,
                                          Lyra3CoversMetrics::values.homeCoverHeight, cropX)) {
                   hasCover = false;
                 }
               } else {
                 hasCover = false;
               }
               file.close();
+            } else {
+              hasCover = false;
             }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (Storage.openFileForRead("HOME", coverBmpPath, file)) {
	Bitmap bitmap(file);
	if (bitmap.parseHeaders() == BmpReaderError::Ok) {
	float coverHeight = static_cast<float>(bitmap.getHeight());
	float coverWidth = static_cast<float>(bitmap.getWidth());
	float ratio = coverWidth / coverHeight;
	const float tileRatio = static_cast<float>(tileWidth - 2 * hPaddingInSelection) /
	static_cast<float>(Lyra3CoversMetrics::values.homeCoverHeight);
	float cropX = 1.0f - (tileRatio / ratio);
	if (!renderer.drawBitmap(bitmap, tileX + hPaddingInSelection, tileY + hPaddingInSelection,
	tileWidth - 2 * hPaddingInSelection,
	Lyra3CoversMetrics::values.homeCoverHeight, cropX)) {
	hasCover = false;
	}
	} else {
	hasCover = false;
	}
	file.close();
	}
	if (Storage.openFileForRead("HOME", coverBmpPath, file)) {
	Bitmap bitmap(file);
	if (bitmap.parseHeaders() == BmpReaderError::Ok) {
	float coverHeight = static_cast<float>(bitmap.getHeight());
	float coverWidth = static_cast<float>(bitmap.getWidth());
	float ratio = coverWidth / coverHeight;
	const float tileRatio = static_cast<float>(tileWidth - 2 * hPaddingInSelection) /
	static_cast<float>(Lyra3CoversMetrics::values.homeCoverHeight);
	float cropX = 1.0f - (tileRatio / ratio);
	if (!renderer.drawBitmap(bitmap, tileX + hPaddingInSelection, tileY + hPaddingInSelection,
	tileWidth - 2 * hPaddingInSelection,
	Lyra3CoversMetrics::values.homeCoverHeight, cropX)) {
	hasCover = false;
	}
	} else {
	hasCover = false;
	}
	file.close();
	} else {
	hasCover = false;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/themes/lyra/Lyra3CoversTheme.cpp` around lines 49 - 68, The
code currently leaves hasCover true when Storage.openFileForRead("HOME",
coverBmpPath, file) returns false; update the open-failure branch in
Lyra3CoversTheme.cpp (the block using Storage.openFileForRead, Bitmap,
renderer.drawBitmap and the hasCover flag) to explicitly set hasCover = false
when openFileForRead fails, and only call file.close() or proceed with Bitmap
parsing/drawing when the file was successfully opened (ensure the early-fail
path does not attempt to use file or skip the empty-cover fallback).

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Honor hasSideButtonHints in the safe-area calculation.

The new helper accepts hasSideButtonHints, but the flag is never used. Callers that pass true will still lay out content into the full rect, so side button hints can overlap the UI.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/UITheme.cpp` around lines 74 - 104, UITheme::getScreenSafeArea
currently ignores the hasSideButtonHints parameter, so side button hint areas
are not subtracted from the returned safeArea; update the switch handling in
UITheme::getScreenSafeArea to also subtract currentMetrics->buttonHintsHeight
from the appropriate edges when hasSideButtonHints is true (for
LandscapeClockwise and LandscapeCounterClockwise adjust x/width appropriately;
for Portrait/PortraitInverted adjust y/height if side hints apply there),
preserving the existing hasFrontButtonHints logic and using the same
currentMetrics->buttonHintsHeight value.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Mirror the FactoryLut screenshot path here too.

The combo handler special-cases GfxRenderer::DisplayState::FactoryLut because the normal framebuffer screenshot is wrong in that state. This menu action still uses the normal path unconditionally, so menu-triggered screenshots can come out blank/incorrect on the same screen.

💡 Suggested fix

       case PowerButtonMenuActivity::MenuAction::SCREENSHOT:
         LOG_DBG("MAIN", "Power button menu: screenshot");
         {
           RenderLock lock;
-          ScreenshotUtil::takeScreenshot(renderer);
+          if (renderer.getDisplayState() == GfxRenderer::DisplayState::FactoryLut) {
+            if (Activity* activity = activityManager.getCurrentActivity()) {
+              ScreenshotUtil::prepareFactoryLutScreenshot(renderer);
+              activity->onScreenshotRequest();
+            }
+          } else {
+            ScreenshotUtil::takeScreenshot(renderer);
+          }
         }
         break;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	case PowerButtonMenuActivity::MenuAction::SCREENSHOT:
	LOG_DBG("MAIN", "Power button menu: screenshot");
	{
	RenderLock lock;
	ScreenshotUtil::takeScreenshot(renderer);
	}
	case PowerButtonMenuActivity::MenuAction::SCREENSHOT:
	LOG_DBG("MAIN", "Power button menu: screenshot");
	{
	RenderLock lock;
	if (renderer.getDisplayState() == GfxRenderer::DisplayState::FactoryLut) {
	if (Activity* activity = activityManager.getCurrentActivity()) {
	ScreenshotUtil::prepareFactoryLutScreenshot(renderer);
	activity->onScreenshotRequest();
	}
	} else {
	ScreenshotUtil::takeScreenshot(renderer);
	}
	}
	break;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main.cpp` around lines 510 - 515, Menu-triggered screenshots use the
normal framebuffer path unconditionally, causing blank/incorrect images in the
FactoryLut display state; update the
PowerButtonMenuActivity::MenuAction::SCREENSHOT case so after acquiring
RenderLock it checks renderer->getDisplayState() for
GfxRenderer::DisplayState::FactoryLut and, if so, invoke the same
FactoryLut-specific screenshot logic used by the combo handler (mirror that code
path), otherwise call ScreenshotUtil::takeScreenshot(renderer); keep RenderLock
scope the same and reuse the exact FactoryLut-handling function or code block
the combo handler uses to ensure identical behavior.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

When rewriting XHTML attributes, does decoding%20withdecodeURIComponentand then writing the decoded string back viasetAttribute('src', ...) risk breaking percent-encoded relative URLs in EPUB/XHTML?

💡 Result:

Yes—decoding a percent-encoded value like %20 to a literal space and then writing it back via setAttribute('src', ...) can risk breaking relative URLs inside EPUB/XHTML, because it changes the URL syntax that the reading system expects to match against the actual resource paths stored in the package. EPUB 3 specifies that “Parsing may replace some characters in the file path by their percent encoded alternative. For example, A/B/C/file name.xhtml becomes A/B/C/file%20name.xhtml” [1][2]. This indicates percent-encoded forms in paths are significant and are the normalized/expected representation. Additionally, real-world tooling bugs show failures when percent-escaped characters are not handled consistently. For example, epubcheck issue reports that resource lookup can fail when URL-escaped file paths are not correctly identified, and that URI-escaped filenames (including %20-style escapes) can pass where other handling fails [3]. Similarly, epub.js has reported image 404s related to encoded URLs, with debugging involving decoding URL strings and replacing encoded forms to correct requests [4]. On the JavaScript side, decodeURIComponent() decodes URI components and converts %20 into a space (and also throws if percent escapes are invalid) [5]. If you apply that directly to an authored src value and then set the DOM attribute with the decoded result, you have changed the URL string that will be serialized/used for fetching. Practical implication: if your goal is to preserve the original resource targeting in EPUB, don’t round-trip through decodeURIComponent() and then re-set the decoded string. Instead, either: - preserve percent-encoding as-is (recommended), or - if you must normalize, do it by percent-encoding correctly after any transformation (i.e., produce a percent-encoded path form rather than a literal space) [6][1]. In short: decoding %20 to spaces is exactly the change EPUB parsing/fetching behavior is designed to avoid; it can lead to mismatches and missing resources in EPUB readers.

Citations:

• 1: https://www.w3.org/TR/epub-33/
• 2: https://w3c.github.io/epub-specs/epub33/core/
• 3: URI escaped filepaths not always correctly identified w3c/epubcheck#1479
• 4: URL encoded href causing 404 on images futurepress/epub.js#838
• 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent
• 6: https://developer.mozilla.org/en-US/docs/Glossary/Percent-encoding

---

Preserve URI encoding when rewriting img src attributes.

decodeHref(src) for matching is correct, but writing decodedSrc.replace(...) back to the DOM converts percent-encoded characters (e.g., %20 → space) and breaks the XHTML reference. EPUB 3 treats percent-encoded filenames as the canonical form; changing My%20Cover.png to My Cover.png will fail to locate the resource. Decode only for comparison; when rewriting, either preserve the original src encoding during the replacement or re-encode the result before setting it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/network/html/FilesPage.html` around lines 4430 - 4440, The code decodes
src with decodeHref for matching but then writes back decodedSrc which strips
percent-encodings; instead, keep the original encoded src for the DOM update:
use decodeHref(src) and resolvePath(xhtmlPath, decodedSrc) to find a matching
entry in renamed, but when updating img.setAttribute('src') replace only the
last path segment of the original src (the one after the final '/') with the new
basename (newPath.split('/').pop()) and URL-encode that basename (e.g. via
encodeURIComponent) before inserting so percent-encoding is preserved; update
modified = true as before.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don't count fallback uploads as failed files.

This path only runs after the file was uploaded successfully. If optimization falls back to the original EPUB, the batch summary currently increments filesFailed, so the exported log says the file failed even though delivery succeeded.

💡 Suggested fix

-        const ok = !conversionFailed && conversionSucceeded;
-        saveToFileBatchLog(file.name, ok, ok ? convOriginalSize : 0, ok ? convNewSize : 0);
+        const optimized = conversionSucceeded && !conversionFailed;
+        saveToFileBatchLog(file.name, true, optimized ? convOriginalSize : 0, optimized ? convNewSize : 0);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	const ok = !conversionFailed && conversionSucceeded;
	saveToFileBatchLog(file.name, ok, ok ? convOriginalSize : 0, ok ? convNewSize : 0);
	}
	const optimized = conversionSucceeded && !conversionFailed;
	saveToFileBatchLog(file.name, true, optimized ? convOriginalSize : 0, optimized ? convNewSize : 0);
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/network/html/FilesPage.html` around lines 4906 - 4908, The code currently
sets ok = !conversionFailed && conversionSucceeded which treats
fallback-to-original as a failure; instead, mark the file as successful because
this block runs after a successful upload. Replace the ok calculation (in the
save-to-log path using saveToFileBatchLog and the variables conversionFailed,
conversionSucceeded, convOriginalSize, convNewSize) so the file is considered
successful (ok = true) and pass sizes accordingly: use convOriginalSize as the
original size and use convNewSize only when conversionSucceeded, otherwise use
convOriginalSize for the delivered size so fallback uploads are not counted as
failed.