addresses comments from maintainer

src/CrossPointSettings.cpp

@@ -14,7 +14,7 @@ CrossPointSettings CrossPointSettings::instance;
 namespace {
 constexpr uint8_t SETTINGS_FILE_VERSION = 1;
 // Increment this when adding new persisted settings fields
-constexpr uint8_t SETTINGS_COUNT = 14;
+constexpr uint8_t SETTINGS_COUNT = 12;
 constexpr char SETTINGS_FILE[] = "/.crosspoint/settings.bin";
 }  // namespace
 
@@ -40,9 +40,7 @@ bool CrossPointSettings::saveToFile() const {
   serialization::writePod(outputFile, fontSize);
   serialization::writePod(outputFile, lineSpacing);
   serialization::writePod(outputFile, paragraphAlignment);
-  serialization::writePod(outputFile, sideMargin);
   serialization::writeString(outputFile, std::string(opdsServerUrl));
-  serialization::writePod(outputFile, calibreWirelessEnabled);
   outputFile.close();
 
   Serial.printf("[%lu] [CPS] Settings saved to file\n", millis());
@@ -91,8 +89,6 @@ bool CrossPointSettings::loadFromFile() {
     if (++settingsRead >= fileSettingsCount) break;
     serialization::readPod(inputFile, paragraphAlignment);
     if (++settingsRead >= fileSettingsCount) break;
-    serialization::readPod(inputFile, sideMargin);
-    if (++settingsRead >= fileSettingsCount) break;
     {
       std::string urlStr;
       serialization::readString(inputFile, urlStr);
@@ -100,8 +96,6 @@ bool CrossPointSettings::loadFromFile() {
       opdsServerUrl[sizeof(opdsServerUrl) - 1] = '\0';
     }
     if (++settingsRead >= fileSettingsCount) break;
-    serialization::readPod(inputFile, calibreWirelessEnabled);
-    if (++settingsRead >= fileSettingsCount) break;
   } while (false);
 
   inputFile.close();
@@ -186,17 +180,3 @@ int CrossPointSettings::getReaderFontId() const {
       }
   }
 }
-
-int CrossPointSettings::getReaderSideMargin() const {
-  switch (sideMargin) {
-    case MARGIN_NONE:
-      return 0;
-    case MARGIN_SMALL:
-    default:
-      return 5;
-    case MARGIN_MEDIUM:
-      return 20;
-    case MARGIN_LARGE:
-      return 30;
-  }
-}

src/CrossPointSettings.h

@@ -44,7 +44,6 @@ class CrossPointSettings {
   enum FONT_SIZE { SMALL = 0, MEDIUM = 1, LARGE = 2, EXTRA_LARGE = 3 };
   enum LINE_COMPRESSION { TIGHT = 0, NORMAL = 1, WIDE = 2 };
   enum PARAGRAPH_ALIGNMENT { JUSTIFIED = 0, LEFT_ALIGN = 1, CENTER_ALIGN = 2, RIGHT_ALIGN = 3 };
-  enum SIDE_MARGIN { MARGIN_NONE = 0, MARGIN_SMALL = 1, MARGIN_MEDIUM = 2, MARGIN_LARGE = 3 };
 
   // Sleep screen settings
   uint8_t sleepScreen = DARK;
@@ -65,11 +64,8 @@ class CrossPointSettings {
   uint8_t fontSize = MEDIUM;
   uint8_t lineSpacing = NORMAL;
   uint8_t paragraphAlignment = JUSTIFIED;
-  uint8_t sideMargin = MARGIN_SMALL;
   // OPDS browser settings
   char opdsServerUrl[128] = "";
-  // Calibre wireless device settings
-  uint8_t calibreWirelessEnabled = 0;
 
   ~CrossPointSettings() = default;
 
@@ -83,7 +79,6 @@ class CrossPointSettings {
   bool loadFromFile();
 
   float getReaderLineCompression() const;
-  int getReaderSideMargin() const;
 };
 
 // Helper macro to access settings

src/ScreenComponents.cpp

@@ -2,6 +2,7 @@
 
 #include <GfxRenderer.h>
 
+#include <cstdint>
 #include <string>
 
 #include "Battery.h"
@@ -39,3 +40,26 @@ void ScreenComponents::drawBattery(const GfxRenderer& renderer, const int left,
 
   renderer.fillRect(x + 1, y + 1, filledWidth, batteryHeight - 2);
 }
+
+void ScreenComponents::drawProgressBar(const GfxRenderer& renderer, const int x, const int y, const int width,
+                                        const int height, const size_t current, const size_t total) {
+  if (total == 0) {
+    return;
+  }
+
+  // Use 64-bit arithmetic to avoid overflow for large files
+  const int percent = static_cast<int>((static_cast<uint64_t>(current) * 100) / total);
+
+  // Draw outline
+  renderer.drawRect(x, y, width, height);
+
+  // Draw filled portion
+  const int fillWidth = (width - 4) * percent / 100;
+  if (fillWidth > 0) {
+    renderer.fillRect(x + 2, y + 2, fillWidth, height - 4);
+  }
+
+  // Draw percentage text centered below bar
+  const std::string percentText = std::to_string(percent) + "%";
+  renderer.drawCenteredText(UI_10_FONT_ID, y + height + 15, percentText.c_str());
+}

src/ScreenComponents.h

@@ -1,8 +1,24 @@
 #pragma once
 
+#include <cstddef>
+#include <cstdint>
+
 class GfxRenderer;
 
 class ScreenComponents {
  public:
   static void drawBattery(const GfxRenderer& renderer, int left, int top);
+
+  /**
+   * Draw a progress bar with percentage text.
+   * @param renderer The graphics renderer
+   * @param x Left position of the bar
+   * @param y Top position of the bar
+   * @param width Width of the bar
+   * @param height Height of the bar
+   * @param current Current progress value
+   * @param total Total value for 100% progress
+   */
+  static void drawProgressBar(const GfxRenderer& renderer, int x, int y, int width, int height, size_t current,
+                              size_t total);
 };

src/activities/browser/OpdsBookBrowserActivity.cpp

@@ -2,57 +2,21 @@
 
 #include <GfxRenderer.h>
 #include <HardwareSerial.h>
+#include <WiFi.h>
 
 #include "CrossPointSettings.h"
 #include "MappedInputManager.h"
+#include "ScreenComponents.h"
+#include "WifiCredentialStore.h"
 #include "fontIds.h"
 #include "network/HttpDownloader.h"
+#include "util/StringUtils.h"
+#include "util/UrlUtils.h"
 
 namespace {
 constexpr int PAGE_ITEMS = 23;
 constexpr int SKIP_PAGE_MS = 700;
 constexpr char OPDS_ROOT_PATH[] = "opds";  // No leading slash - relative to server URL
-
-// Prepend http:// if no protocol specified (server will redirect to https if needed)
-std::string ensureProtocol(const std::string& url) {
-  if (url.find("://") == std::string::npos) {
-    return "http://" + url;
-  }
-  return url;
-}
-
-// Extract host with protocol from URL (e.g., "http://example.com" from "http://example.com/path")
-std::string extractHost(const std::string& url) {
-  const size_t protocolEnd = url.find("://");
-  if (protocolEnd == std::string::npos) {
-    // No protocol, find first slash
-    const size_t firstSlash = url.find('/');
-    return firstSlash == std::string::npos ? url : url.substr(0, firstSlash);
-  }
-  // Find the first slash after the protocol
-  const size_t hostStart = protocolEnd + 3;
-  const size_t pathStart = url.find('/', hostStart);
-  return pathStart == std::string::npos ? url : url.substr(0, pathStart);
-}
-
-// Build full URL from server URL and path
-// If path starts with /, it's an absolute path from the host root
-// Otherwise, it's relative to the server URL
-std::string buildUrl(const std::string& serverUrl, const std::string& path) {
-  const std::string urlWithProtocol = ensureProtocol(serverUrl);
-  if (path.empty()) {
-    return urlWithProtocol;
-  }
-  if (path[0] == '/') {
-    // Absolute path - use just the host
-    return extractHost(urlWithProtocol) + path;
-  }
-  // Relative path - append to server URL
-  if (urlWithProtocol.back() == '/') {
-    return urlWithProtocol + path;
-  }
-  return urlWithProtocol + "/" + path;
-}
 }  // namespace
 
 void OpdsBookBrowserActivity::taskTrampoline(void* param) {
@@ -64,13 +28,13 @@ void OpdsBookBrowserActivity::onEnter() {
   Activity::onEnter();
 
   renderingMutex = xSemaphoreCreateMutex();
-  state = BrowserState::LOADING;
+  state = BrowserState::CHECK_WIFI;
   entries.clear();
   navigationHistory.clear();
   currentPath = OPDS_ROOT_PATH;
   selectorIndex = 0;
   errorMessage.clear();
-  statusMessage = "Loading...";
+  statusMessage = "Checking WiFi...";
   updateRequired = true;
 
   xTaskCreate(&OpdsBookBrowserActivity::taskTrampoline, "OpdsBookBrowserTask",
@@ -80,8 +44,8 @@ void OpdsBookBrowserActivity::onEnter() {
               &displayTaskHandle  // Task handle
   );
 
-  // Fetch feed after setting up the display task
+  // Check WiFi and connect if needed, then fetch feed
-  fetchFeed(currentPath);
+  checkAndConnectWifi();
 }
 
 void OpdsBookBrowserActivity::onExit() {
@@ -112,6 +76,14 @@ void OpdsBookBrowserActivity::loop() {
     return;
   }
 
+  // Handle WiFi check state - only Back works
+  if (state == BrowserState::CHECK_WIFI) {
+    if (mappedInput.wasReleased(MappedInputManager::Button::Back)) {
+      onGoHome();
+    }
+    return;
+  }
+
   // Handle loading state - only Back works
   if (state == BrowserState::LOADING) {
     if (mappedInput.wasReleased(MappedInputManager::Button::Back)) {
@@ -182,6 +154,14 @@ void OpdsBookBrowserActivity::render() const {
 
   renderer.drawCenteredText(UI_12_FONT_ID, 15, "Calibre Library", true, EpdFontFamily::BOLD);
 
+  if (state == BrowserState::CHECK_WIFI) {
+    renderer.drawCenteredText(UI_10_FONT_ID, pageHeight / 2, statusMessage.c_str());
+    const auto labels = mappedInput.mapLabels("« Back", "", "", "");
+    renderer.drawButtonHints(UI_10_FONT_ID, labels.btn1, labels.btn2, labels.btn3, labels.btn4);
+    renderer.displayBuffer();
+    return;
+  }
+
   if (state == BrowserState::LOADING) {
     renderer.drawCenteredText(UI_10_FONT_ID, pageHeight / 2, statusMessage.c_str());
     const auto labels = mappedInput.mapLabels("« Back", "", "", "");
@@ -200,13 +180,14 @@ void OpdsBookBrowserActivity::render() const {
   }
 
   if (state == BrowserState::DOWNLOADING) {
-    renderer.drawCenteredText(UI_10_FONT_ID, pageHeight / 2 - 20, "Downloading...");
+    renderer.drawCenteredText(UI_10_FONT_ID, pageHeight / 2 - 40, "Downloading...");
-    renderer.drawCenteredText(UI_10_FONT_ID, pageHeight / 2 + 10, statusMessage.c_str());
+    renderer.drawCenteredText(UI_10_FONT_ID, pageHeight / 2 - 10, statusMessage.c_str());
     if (downloadTotal > 0) {
-      const int percent = (downloadProgress * 100) / downloadTotal;
+      const int barWidth = pageWidth - 100;
-      char progressText[32];
+      constexpr int barHeight = 20;
-      snprintf(progressText, sizeof(progressText), "%d%%", percent);
+      constexpr int barX = 50;
-      renderer.drawCenteredText(UI_10_FONT_ID, pageHeight / 2 + 40, progressText);
+      const int barY = pageHeight / 2 + 20;
+      ScreenComponents::drawProgressBar(renderer, barX, barY, barWidth, barHeight, downloadProgress, downloadTotal);
     }
     renderer.displayBuffer();
     return;
@@ -262,7 +243,7 @@ void OpdsBookBrowserActivity::fetchFeed(const std::string& path) {
     return;
   }
 
-  std::string url = buildUrl(serverUrl, path);
+  std::string url = UrlUtils::buildUrl(serverUrl, path);
   Serial.printf("[%lu] [OPDS] Fetching: %s\n", millis(), url.c_str());
 
   std::string content;
@@ -336,10 +317,14 @@ void OpdsBookBrowserActivity::downloadBook(const OpdsEntry& book) {
   updateRequired = true;
 
   // Build full download URL
-  std::string downloadUrl = buildUrl(SETTINGS.opdsServerUrl, book.href);
+  std::string downloadUrl = UrlUtils::buildUrl(SETTINGS.opdsServerUrl, book.href);
 
-  // Create sanitized filename
+  // Create sanitized filename: "Title - Author.epub" or just "Title.epub" if no author
-  std::string filename = "/" + sanitizeFilename(book.title) + ".epub";
+  std::string baseName = book.title;
+  if (!book.author.empty()) {
+    baseName += " - " + book.author;
+  }
+  std::string filename = "/" + StringUtils::sanitizeFilename(baseName) + ".epub";
 
   Serial.printf("[%lu] [OPDS] Downloading: %s -> %s\n", millis(), downloadUrl.c_str(), filename.c_str());
 
@@ -361,33 +346,51 @@ void OpdsBookBrowserActivity::downloadBook(const OpdsEntry& book) {
   }
 }
 
-std::string OpdsBookBrowserActivity::sanitizeFilename(const std::string& title) const {
+void OpdsBookBrowserActivity::checkAndConnectWifi() {
-  std::string result;
+  // Already connected?
-  result.reserve(title.size());
+  if (WiFi.status() == WL_CONNECTED) {
-
+    state = BrowserState::LOADING;
-  for (char c : title) {
+    statusMessage = "Loading...";
-    // Replace invalid filename characters with underscore
+    updateRequired = true;
-    if (c == '/' || c == '\\' || c == ':' || c == '*' || c == '?' || c == '"' || c == '<' || c == '>' || c == '|') {
+    fetchFeed(currentPath);
-      result += '_';
+    return;
-    } else if (c >= 32 && c < 127) {
-      // Keep printable ASCII characters
-      result += c;
-    }
-    // Skip non-printable characters
   }
 
-  // Trim leading/trailing spaces and dots
+  // Try to connect using saved credentials
-  size_t start = result.find_first_not_of(" .");
+  statusMessage = "Connecting to WiFi...";
-  if (start == std::string::npos) {
+  updateRequired = true;
-    return "book";  // Fallback if title is all invalid characters
-  }
-  size_t end = result.find_last_not_of(" .");
-  result = result.substr(start, end - start + 1);
 
-  // Limit filename length (SD card FAT32 has 255 char limit, but let's be safe)
+  WIFI_STORE.loadFromFile();
-  if (result.length() > 100) {
+  const auto& credentials = WIFI_STORE.getCredentials();
-    result.resize(100);
+  if (credentials.empty()) {
+    state = BrowserState::ERROR;
+    errorMessage = "No WiFi credentials saved";
+    updateRequired = true;
+    return;
   }
 
-  return result.empty() ? "book" : result;
+  // Use the first saved credential
+  const auto& cred = credentials[0];
+  WiFi.mode(WIFI_STA);
+  WiFi.begin(cred.ssid.c_str(), cred.password.c_str());
+
+  // Wait for connection with timeout
+  constexpr int WIFI_TIMEOUT_MS = 10000;
+  const unsigned long startTime = millis();
+  while (WiFi.status() != WL_CONNECTED && millis() - startTime < WIFI_TIMEOUT_MS) {
+    vTaskDelay(100 / portTICK_PERIOD_MS);
   }
+
+  if (WiFi.status() == WL_CONNECTED) {
+    Serial.printf("[%lu] [OPDS] WiFi connected: %s\n", millis(), WiFi.localIP().toString().c_str());
+    state = BrowserState::LOADING;
+    statusMessage = "Loading...";
+    updateRequired = true;
+    fetchFeed(currentPath);
+  } else {
+    state = BrowserState::ERROR;
+    errorMessage = "WiFi connection failed";
+    updateRequired = true;
+  }
+}
+

src/activities/browser/OpdsBookBrowserActivity.h

@@ -17,6 +17,7 @@
 class OpdsBookBrowserActivity final : public Activity {
  public:
   enum class BrowserState {
+    CHECK_WIFI,   // Checking WiFi connection
     LOADING,      // Fetching OPDS feed
     BROWSING,     // Displaying entries (navigation or books)
     DOWNLOADING,  // Downloading selected EPUB
@@ -52,9 +53,9 @@ class OpdsBookBrowserActivity final : public Activity {
   [[noreturn]] void displayTaskLoop();
   void render() const;
 
+  void checkAndConnectWifi();
   void fetchFeed(const std::string& path);
   void navigateToEntry(const OpdsEntry& entry);
   void navigateBack();
   void downloadBook(const OpdsEntry& book);
-  std::string sanitizeFilename(const std::string& title) const;
 };

src/activities/home/HomeActivity.cpp

@@ -21,7 +21,7 @@ void HomeActivity::taskTrampoline(void* param) {
 int HomeActivity::getMenuItemCount() const {
   int count = 3;  // Browse files, File transfer, Settings
   if (hasContinueReading) count++;
-  if (hasBrowserUrl) count++;
+  if (hasOpdsUrl) count++;
   return count;
 }
 
@@ -33,8 +33,8 @@ void HomeActivity::onEnter() {
   // Check if we have a book to continue reading
   hasContinueReading = !APP_STATE.openEpubPath.empty() && SdMan.exists(APP_STATE.openEpubPath.c_str());
 
-  // Check if browser URL is configured
+  // Check if OPDS browser URL is configured
-  hasBrowserUrl = strlen(SETTINGS.opdsServerUrl) > 0;
+  hasOpdsUrl = strlen(SETTINGS.opdsServerUrl) > 0;
 
   if (hasContinueReading) {
     // Extract filename from path for display
@@ -102,7 +102,7 @@ void HomeActivity::loop() {
     int idx = 0;
     const int continueIdx = hasContinueReading ? idx++ : -1;
     const int browseFilesIdx = idx++;
-    const int browseBookIdx = hasBrowserUrl ? idx++ : -1;
+    const int opdsLibraryIdx = hasOpdsUrl ? idx++ : -1;
     const int fileTransferIdx = idx++;
     const int settingsIdx = idx;
 
@@ -110,8 +110,8 @@ void HomeActivity::loop() {
       onContinueReading();
     } else if (selectorIndex == browseFilesIdx) {
       onReaderOpen();
-    } else if (selectorIndex == browseBookIdx) {
+    } else if (selectorIndex == opdsLibraryIdx) {
-      onBrowserOpen();
+      onOpdsBrowserOpen();
     } else if (selectorIndex == fileTransferIdx) {
       onFileTransferOpen();
     } else if (selectorIndex == settingsIdx) {
@@ -289,13 +289,11 @@ void HomeActivity::render() const {
 
   // --- Bottom menu tiles ---
   // Build menu items dynamically
-  std::vector<const char*> menuItems;
+  std::vector<const char*> menuItems = {"Browse Files", "File Transfer", "Settings"};
-  menuItems.push_back("Browse Files");
+  if (hasOpdsUrl) {
-  if (hasBrowserUrl) {
+    // Insert Calibre Library after Browse Files
-    menuItems.push_back("Calibre Library");
+    menuItems.insert(menuItems.begin() + 1, "Calibre Library");
   }
-  menuItems.push_back("File Transfer");
-  menuItems.push_back("Settings");
 
   const int menuTileWidth = pageWidth - 2 * margin;
   constexpr int menuTileHeight = 45;

src/activities/home/HomeActivity.h

@@ -13,14 +13,14 @@ class HomeActivity final : public Activity {
   int selectorIndex = 0;
   bool updateRequired = false;
   bool hasContinueReading = false;
-  bool hasBrowserUrl = false;
+  bool hasOpdsUrl = false;
   std::string lastBookTitle;
   std::string lastBookAuthor;
   const std::function<void()> onContinueReading;
   const std::function<void()> onReaderOpen;
   const std::function<void()> onSettingsOpen;
   const std::function<void()> onFileTransferOpen;
-  const std::function<void()> onBrowserOpen;
+  const std::function<void()> onOpdsBrowserOpen;
 
   static void taskTrampoline(void* param);
   [[noreturn]] void displayTaskLoop();
@@ -31,13 +31,13 @@ class HomeActivity final : public Activity {
   explicit HomeActivity(GfxRenderer& renderer, MappedInputManager& mappedInput,
                         const std::function<void()>& onContinueReading, const std::function<void()>& onReaderOpen,
                         const std::function<void()>& onSettingsOpen, const std::function<void()>& onFileTransferOpen,
-                        const std::function<void()>& onBrowserOpen)
+                        const std::function<void()>& onOpdsBrowserOpen)
       : Activity("Home", renderer, mappedInput),
         onContinueReading(onContinueReading),
         onReaderOpen(onReaderOpen),
         onSettingsOpen(onSettingsOpen),
         onFileTransferOpen(onFileTransferOpen),
-        onBrowserOpen(onBrowserOpen) {}
+        onOpdsBrowserOpen(onOpdsBrowserOpen) {}
   void onEnter() override;
   void onExit() override;
   void loop() override;

src/activities/network/CalibreWirelessActivity.cpp

@@ -1,17 +1,21 @@
 #include "CalibreWirelessActivity.h"
 
 #include <GfxRenderer.h>
+#include <HardwareSerial.h>
 #include <SDCardManager.h>
 #include <WiFi.h>
 
 #include <cstring>
 
-#include "CrossPointSettings.h"
 #include "MappedInputManager.h"
+#include "ScreenComponents.h"
 #include "fontIds.h"
+#include "util/StringUtils.h"
 
-// Define static constexpr members
+namespace {
-constexpr uint16_t CalibreWirelessActivity::UDP_PORTS[];
+constexpr uint16_t UDP_PORTS[] = {54982, 48123, 39001, 44044, 59678};
+constexpr uint16_t LOCAL_UDP_PORT = 8134;  // Port to receive responses
+}  // namespace
 
 void CalibreWirelessActivity::displayTaskTrampoline(void* param) {
   auto* self = static_cast<CalibreWirelessActivity*>(param);
@@ -57,10 +61,6 @@ void CalibreWirelessActivity::onEnter() {
 void CalibreWirelessActivity::onExit() {
   Activity::onExit();
 
-  // Always turn off the setting when exiting so it shows OFF in settings
-  SETTINGS.calibreWirelessEnabled = 0;
-  SETTINGS.saveToFile();
-
   // Stop UDP listening
   udp.stop();
 
@@ -74,13 +74,15 @@ void CalibreWirelessActivity::onExit() {
     currentFile.close();
   }
 
-  // Delete network task first (it may be blocked on network operations)
+  // Acquire stateMutex before deleting network task to avoid race condition
+  xSemaphoreTake(stateMutex, portMAX_DELAY);
   if (networkTaskHandle) {
     vTaskDelete(networkTaskHandle);
     networkTaskHandle = nullptr;
   }
+  xSemaphoreGive(stateMutex);
 
-  // Acquire mutex before deleting display task
+  // Acquire renderingMutex before deleting display task
   xSemaphoreTake(renderingMutex, portMAX_DELAY);
   if (displayTaskHandle) {
     vTaskDelete(displayTaskHandle);
@@ -143,8 +145,8 @@ void CalibreWirelessActivity::networkTaskLoop() {
 
 void CalibreWirelessActivity::listenForDiscovery() {
   // Broadcast "hello" on all UDP discovery ports to find Calibre
-  for (size_t i = 0; i < UDP_PORT_COUNT; i++) {
+  for (const uint16_t port : UDP_PORTS) {
-    udp.beginPacket("255.255.255.255", UDP_PORTS[i]);
+    udp.beginPacket("255.255.255.255", port);
     udp.write(reinterpret_cast<const uint8_t*>("hello"), 5);
     udp.endPacket();
   }
@@ -384,9 +386,10 @@ bool CalibreWirelessActivity::readJsonMessage(std::string& message) {
 void CalibreWirelessActivity::sendJsonResponse(int opcode, const std::string& data) {
   // Format: length + [opcode, {data}]
   std::string json = "[" + std::to_string(opcode) + "," + data + "]";
-  std::string packet = std::to_string(json.length()) + json;
+  const std::string lengthPrefix = std::to_string(json.length());
+  json.insert(0, lengthPrefix);
 
-  tcpClient.write(reinterpret_cast<const uint8_t*>(packet.c_str()), packet.length());
+  tcpClient.write(reinterpret_cast<const uint8_t*>(json.c_str()), json.length());
   tcpClient.flush();
 }
 
@@ -418,17 +421,24 @@ void CalibreWirelessActivity::handleCommand(int opcode, const std::string& data)
       break;
     case OP_SET_CALIBRE_DEVICE_INFO:
     case OP_SET_CALIBRE_DEVICE_NAME:
-      // Just acknowledge
+      // These set metadata about the connected Calibre instance.
+      // We don't need this info, just acknowledge receipt.
       sendJsonResponse(OP_OK, "{}");
       break;
     case OP_SET_LIBRARY_INFO:
+      // Library metadata (name, UUID) - not needed for receiving books
+      sendJsonResponse(OP_OK, "{}");
+      break;
     case OP_SEND_BOOKLISTS:
+      // Calibre asking us to send our book list. We report 0 books in
+      // handleGetBookCount, so this is effectively a no-op.
       sendJsonResponse(OP_OK, "{}");
       break;
     case OP_TOTAL_SPACE:
       handleFreeSpace();
       break;
     default:
+      Serial.printf("[%lu] [CAL] Unknown opcode: %d\n", millis(), opcode);
       sendJsonResponse(OP_OK, "{}");
       break;
   }
@@ -453,8 +463,11 @@ void CalibreWirelessActivity::handleGetInitializationInfo(const std::string& dat
   response += "\"canStreamBooks\":true,";
   response += "\"canStreamMetadata\":true,";
   response += "\"canUseCachedMetadata\":true,";
-  response += "\"ccVersionNumber\":212,";  // Match a known CC version
+  // ccVersionNumber: Calibre Companion protocol version. 212 matches CC 5.4.20+.
-  response += "\"coverHeight\":240,";
+  // Using a known version ensures compatibility with Calibre's feature detection.
+  response += "\"ccVersionNumber\":212,";
+  // coverHeight: Max cover image height. We don't process covers, so this is informational only.
+  response += "\"coverHeight\":800,";
   response += "\"deviceKind\":\"CrossPoint\",";
   response += "\"deviceName\":\"CrossPoint\",";
   response += "\"extensionPathLengths\":{\"epub\":37},";
@@ -472,17 +485,18 @@ void CalibreWirelessActivity::handleGetDeviceInformation() {
   response += "\"device_info\":{";
   response += "\"device_store_uuid\":\"" + getDeviceUuid() + "\",";
   response += "\"device_name\":\"CrossPoint Reader\",";
-  response += "\"device_version\":\"1.0\"";
+  response += "\"device_version\":\"" CROSSPOINT_VERSION "\"";
   response += "},";
   response += "\"version\":1,";
-  response += "\"device_version\":\"1.0\"";
+  response += "\"device_version\":\"" CROSSPOINT_VERSION "\"";
   response += "}";
 
   sendJsonResponse(OP_OK, response);
 }
 
 void CalibreWirelessActivity::handleFreeSpace() {
-  // Report 10GB free space
+  // TODO: Report actual SD card free space instead of hardcoded value
+  // Report 10GB free space for now
   sendJsonResponse(OP_OK, "{\"free_space_on_device\":10737418240}");
 }
 
@@ -558,7 +572,7 @@ void CalibreWirelessActivity::handleSendBook(const std::string& data) {
   }
 
   // Sanitize and create full path
-  currentFilename = "/" + sanitizeFilename(filename);
+  currentFilename = "/" + StringUtils::sanitizeFilename(filename);
   if (currentFilename.find(".epub") == std::string::npos) {
     currentFilename += ".epub";
   }
@@ -684,20 +698,11 @@ void CalibreWirelessActivity::render() const {
 
   // Draw progress if receiving
   if (state == CalibreWirelessState::RECEIVING && currentFileSize > 0) {
-    const int percent = static_cast<int>((bytesReceived * 100) / currentFileSize);
-
-    // Progress bar
     const int barWidth = pageWidth - 100;
-    const int barHeight = 20;
+    constexpr int barHeight = 20;
-    const int barX = 50;
+    constexpr int barX = 50;
     const int barY = statusY + 20;
-
+    ScreenComponents::drawProgressBar(renderer, barX, barY, barWidth, barHeight, bytesReceived, currentFileSize);
-    renderer.drawRect(barX, barY, barWidth, barHeight);
-    renderer.fillRect(barX + 2, barY + 2, (barWidth - 4) * percent / 100, barHeight - 4);
-
-    // Percentage text
-    const std::string percentText = std::to_string(percent) + "%";
-    renderer.drawCenteredText(UI_10_FONT_ID, barY + barHeight + 15, percentText.c_str());
   }
 
   // Draw error if present
@@ -712,31 +717,6 @@ void CalibreWirelessActivity::render() const {
   renderer.displayBuffer();
 }
 
-std::string CalibreWirelessActivity::sanitizeFilename(const std::string& name) const {
-  std::string result;
-  result.reserve(name.size());
-
-  for (char c : name) {
-    if (c == '/' || c == '\\' || c == ':' || c == '*' || c == '?' || c == '"' || c == '<' || c == '>' || c == '|') {
-      result += '_';
-    } else if (c >= 32 && c < 127) {
-      result += c;
-    }
-  }
-
-  // Trim leading/trailing spaces and dots
-  size_t start = 0;
-  while (start < result.size() && (result[start] == ' ' || result[start] == '.')) {
-    start++;
-  }
-  size_t end = result.size();
-  while (end > start && (result[end - 1] == ' ' || result[end - 1] == '.')) {
-    end--;
-  }
-
-  return result.substr(start, end - start);
-}
-
 std::string CalibreWirelessActivity::getDeviceUuid() const {
   // Generate a consistent UUID based on MAC address
   uint8_t mac[6];

src/activities/network/CalibreWirelessActivity.h

@@ -28,11 +28,14 @@ enum class CalibreWirelessState {
  * CalibreWirelessActivity implements Calibre's "wireless device" protocol.
  * This allows Calibre desktop to send books directly to the device over WiFi.
  *
- * Protocol:
+ * Protocol specification sourced from Calibre's smart device driver:
- * 1. Device listens on UDP ports 54982, 48123, 39001, 44044, 59678
+ * https://github.com/kovidgoyal/calibre/blob/master/src/calibre/devices/smart_device_app/driver.py
- * 2. Calibre broadcasts discovery messages
+ *
- * 3. Device responds with its TCP server address
+ * Protocol overview:
- * 4. Calibre connects via TCP and sends JSON commands
+ * 1. Device broadcasts "hello" on UDP ports 54982, 48123, 39001, 44044, 59678
+ * 2. Calibre responds with its TCP server address
+ * 3. Device connects to Calibre's TCP server
+ * 4. Calibre sends JSON commands with length-prefixed messages
  * 5. Books are transferred as binary data after SEND_BOOK command
  */
 class CalibreWirelessActivity final : public Activity {
@@ -47,9 +50,6 @@ class CalibreWirelessActivity final : public Activity {
 
   // UDP discovery
   WiFiUDP udp;
-  static constexpr uint16_t UDP_PORTS[] = {54982, 48123, 39001, 44044, 59678};
-  static constexpr size_t UDP_PORT_COUNT = 5;
-  static constexpr uint16_t LOCAL_UDP_PORT = 8134;  // Port to receive responses
 
   // TCP connection (we connect to Calibre)
   WiFiClient tcpClient;
@@ -118,7 +118,6 @@ class CalibreWirelessActivity final : public Activity {
   void handleNoop(const std::string& data);
 
   // Utility
-  std::string sanitizeFilename(const std::string& title) const;
   std::string getDeviceUuid() const;
   void setState(CalibreWirelessState newState);
   void setStatus(const std::string& message);

src/activities/reader/EpubReaderActivity.cpp

@@ -17,6 +17,7 @@ constexpr int pagesPerRefresh = 15;
 constexpr unsigned long skipChapterMs = 700;
 constexpr unsigned long goHomeMs = 1000;
 constexpr int topPadding = 5;
+constexpr int horizontalPadding = 5;
 constexpr int statusBarMargin = 19;
 }  // namespace
 
@@ -253,8 +254,8 @@ void EpubReaderActivity::renderScreen() {
   renderer.getOrientedViewableTRBL(&orientedMarginTop, &orientedMarginRight, &orientedMarginBottom,
                                    &orientedMarginLeft);
   orientedMarginTop += topPadding;
-  orientedMarginLeft += SETTINGS.getReaderSideMargin();
+  orientedMarginLeft += horizontalPadding;
-  orientedMarginRight += SETTINGS.getReaderSideMargin();
+  orientedMarginRight += horizontalPadding;
   orientedMarginBottom += statusBarMargin;
 
   if (!section) {

src/activities/settings/CalibreSettingsActivity.cpp

@@ -98,13 +98,7 @@ void CalibreSettingsActivity::handleSelection() {
           updateRequired = true;
         }));
   } else if (selectedIndex == 1) {
-    // Wireless Device - toggle and launch activity if enabling
+    // Wireless Device - launch the activity (handles WiFi connection internally)
-    const bool wasEnabled = SETTINGS.calibreWirelessEnabled;
-    SETTINGS.calibreWirelessEnabled = !wasEnabled;
-    SETTINGS.saveToFile();
-
-    if (!wasEnabled) {
-      // Just enabled - launch the wireless activity
     exitActivity();
     if (WiFi.status() != WL_CONNECTED) {
       enterNewActivity(new WifiSelectionActivity(renderer, mappedInput, [this](bool connected) {
@@ -115,9 +109,6 @@ void CalibreSettingsActivity::handleSelection() {
             updateRequired = true;
           }));
         } else {
-            // WiFi connection failed/cancelled, turn off the setting
-            SETTINGS.calibreWirelessEnabled = 0;
-            SETTINGS.saveToFile();
           updateRequired = true;
         }
       }));
@@ -127,10 +118,6 @@ void CalibreSettingsActivity::handleSelection() {
         updateRequired = true;
       }));
     }
-    } else {
-      // Just disabled - just update the display
-      updateRequired = true;
-    }
   }
 
   xSemaphoreGive(renderingMutex);
@@ -166,19 +153,13 @@ void CalibreSettingsActivity::render() {
 
     renderer.drawText(UI_10_FONT_ID, 20, settingY, menuNames[i], !isSelected);
 
-    // Draw status
+    // Draw status for URL setting
-    const char* status = "";
     if (i == 0) {
-      // Calibre Web URL
+      const char* status = (strlen(SETTINGS.opdsServerUrl) > 0) ? "[Set]" : "[Not Set]";
-      status = (strlen(SETTINGS.opdsServerUrl) > 0) ? "[Set]" : "[Not Set]";
-    } else if (i == 1) {
-      // Wireless Device
-      status = SETTINGS.calibreWirelessEnabled ? "ON" : "OFF";
-    }
-
       const auto width = renderer.getTextWidth(UI_10_FONT_ID, status);
       renderer.drawText(UI_10_FONT_ID, pageWidth - 20 - width, settingY, status, !isSelected);
     }
+  }
 
   // Draw button hints
   const auto labels = mappedInput.mapLabels("« Back", "Select", "", "");

src/activities/settings/SettingsActivity.cpp

@@ -12,7 +12,7 @@
 
 // Define the static settings list
 namespace {
-constexpr int settingsCount = 14;
+constexpr int settingsCount = 13;
 const SettingInfo settingsList[settingsCount] = {
     // Should match with SLEEP_SCREEN_MODE
     {"Sleep Screen", SettingType::ENUM, &CrossPointSettings::sleepScreen, {"Dark", "Light", "Custom", "Cover"}},
@@ -41,7 +41,6 @@ const SettingInfo settingsList[settingsCount] = {
      SettingType::ENUM,
      &CrossPointSettings::paragraphAlignment,
      {"Justify", "Left", "Center", "Right"}},
-    {"Reader Side Margin", SettingType::ENUM, &CrossPointSettings::sideMargin, {"None", "Small", "Medium", "Large"}},
     {"Calibre Settings", SettingType::ACTION, nullptr, {}},
     {"Check for updates", SettingType::ACTION, nullptr, {}},
 };

src/main.cpp

@@ -228,43 +228,9 @@ void onGoToSettings() {
   enterNewActivity(new SettingsActivity(renderer, mappedInputManager, onGoHome));
 }
 
-// Helper to launch browser after WiFi is connected
-void launchBrowserWithUrlCheck() {
-  // If no server URL configured, prompt for one first
-  if (strlen(SETTINGS.opdsServerUrl) == 0) {
-    enterNewActivity(new KeyboardEntryActivity(
-        renderer, mappedInputManager, "Calibre Web URL", "", 10, 127, false,
-        [](const std::string& url) {
-          strncpy(SETTINGS.opdsServerUrl, url.c_str(), sizeof(SETTINGS.opdsServerUrl) - 1);
-          SETTINGS.opdsServerUrl[sizeof(SETTINGS.opdsServerUrl) - 1] = '\0';
-          SETTINGS.saveToFile();
-          exitActivity();
-          enterNewActivity(new OpdsBookBrowserActivity(renderer, mappedInputManager, onGoHome));
-        },
-        [] {
-          exitActivity();
-          onGoHome();
-        }));
-  } else {
-    enterNewActivity(new OpdsBookBrowserActivity(renderer, mappedInputManager, onGoHome));
-  }
-}
-
 void onGoToBrowser() {
   exitActivity();
-  // Check WiFi connectivity first
+  enterNewActivity(new OpdsBookBrowserActivity(renderer, mappedInputManager, onGoHome));
-  if (WiFi.status() != WL_CONNECTED) {
-    enterNewActivity(new WifiSelectionActivity(renderer, mappedInputManager, [](bool connected) {
-      exitActivity();
-      if (connected) {
-        launchBrowserWithUrlCheck();
-      } else {
-        onGoHome();
-      }
-    }));
-  } else {
-    launchBrowserWithUrlCheck();
-  }
 }
 
 void onGoHome() {

src/util/StringUtils.cpp

@@ -0,0 +1,36 @@
+#include "StringUtils.h"
+
+namespace StringUtils {
+
+std::string sanitizeFilename(const std::string& name, size_t maxLength) {
+  std::string result;
+  result.reserve(name.size());
+
+  for (char c : name) {
+    // Replace invalid filename characters with underscore
+    if (c == '/' || c == '\\' || c == ':' || c == '*' || c == '?' || c == '"' || c == '<' || c == '>' || c == '|') {
+      result += '_';
+    } else if (c >= 32 && c < 127) {
+      // Keep printable ASCII characters
+      result += c;
+    }
+    // Skip non-printable characters
+  }
+
+  // Trim leading/trailing spaces and dots
+  size_t start = result.find_first_not_of(" .");
+  if (start == std::string::npos) {
+    return "book";  // Fallback if name is all invalid characters
+  }
+  size_t end = result.find_last_not_of(" .");
+  result = result.substr(start, end - start + 1);
+
+  // Limit filename length
+  if (result.length() > maxLength) {
+    result.resize(maxLength);
+  }
+
+  return result.empty() ? "book" : result;
+}
+
+}  // namespace StringUtils

src/util/StringUtils.h

@@ -0,0 +1,13 @@
+#pragma once
+#include <string>
+
+namespace StringUtils {
+
+/**
+ * Sanitize a string for use as a filename.
+ * Replaces invalid characters with underscores, trims spaces/dots,
+ * and limits length to maxLength characters.
+ */
+std::string sanitizeFilename(const std::string& name, size_t maxLength = 100);
+
+}  // namespace StringUtils

src/util/UrlUtils.cpp

@@ -0,0 +1,41 @@
+#include "UrlUtils.h"
+
+namespace UrlUtils {
+
+std::string ensureProtocol(const std::string& url) {
+  if (url.find("://") == std::string::npos) {
+    return "http://" + url;
+  }
+  return url;
+}
+
+std::string extractHost(const std::string& url) {
+  const size_t protocolEnd = url.find("://");
+  if (protocolEnd == std::string::npos) {
+    // No protocol, find first slash
+    const size_t firstSlash = url.find('/');
+    return firstSlash == std::string::npos ? url : url.substr(0, firstSlash);
+  }
+  // Find the first slash after the protocol
+  const size_t hostStart = protocolEnd + 3;
+  const size_t pathStart = url.find('/', hostStart);
+  return pathStart == std::string::npos ? url : url.substr(0, pathStart);
+}
+
+std::string buildUrl(const std::string& serverUrl, const std::string& path) {
+  const std::string urlWithProtocol = ensureProtocol(serverUrl);
+  if (path.empty()) {
+    return urlWithProtocol;
+  }
+  if (path[0] == '/') {
+    // Absolute path - use just the host
+    return extractHost(urlWithProtocol) + path;
+  }
+  // Relative path - append to server URL
+  if (urlWithProtocol.back() == '/') {
+    return urlWithProtocol + path;
+  }
+  return urlWithProtocol + "/" + path;
+}
+
+}  // namespace UrlUtils

src/util/UrlUtils.h

@@ -0,0 +1,23 @@
+#pragma once
+#include <string>
+
+namespace UrlUtils {
+
+/**
+ * Prepend http:// if no protocol specified (server will redirect to https if needed)
+ */
+std::string ensureProtocol(const std::string& url);
+
+/**
+ * Extract host with protocol from URL (e.g., "http://example.com" from "http://example.com/path")
+ */
+std::string extractHost(const std::string& url);
+
+/**
+ * Build full URL from server URL and path.
+ * If path starts with /, it's an absolute path from the host root.
+ * Otherwise, it's relative to the server URL.
+ */
+std::string buildUrl(const std::string& serverUrl, const std::string& path);
+
+}  // namespace UrlUtils