Small code cleanup (#83)

## Summary

* Fix cppcheck low violations
* Remove teardown method on parsers, use destructor
* Code cleanup

.github/workflows/ci.yml

@@ -34,7 +34,7 @@ jobs:
           sudo apt-get install -y clang-format-21
 
       - name: Run cppcheck
-        run: pio check --fail-on-defect medium --fail-on-defect high
+        run: pio check --fail-on-defect low --fail-on-defect medium --fail-on-defect high
 
       - name: Run clang-format
         run: PATH="/usr/lib/llvm-21/bin:$PATH" ./bin/clang-format-fix && git diff --exit-code || (echo "Please run 'bin/clang-format-fix' to fix formatting issues" && exit 1)

lib/Epub/Epub.cpp

@@ -30,24 +30,22 @@ bool Epub::findContentOpfFile(std::string* contentOpfFile) const {
   // Stream read (reusing your existing stream logic)
   if (!readItemContentsToStream(containerPath, containerParser, 512)) {
     Serial.printf("[%lu] [EBP] Could not read META-INF/container.xml\n", millis());
-    containerParser.teardown();
     return false;
   }
 
   // Extract the result
   if (containerParser.fullPath.empty()) {
     Serial.printf("[%lu] [EBP] Could not find valid rootfile in container.xml\n", millis());
-    containerParser.teardown();
     return false;
   }
 
   *contentOpfFile = std::move(containerParser.fullPath);
-
-  containerParser.teardown();
   return true;
 }
 
 bool Epub::parseContentOpf(const std::string& contentOpfFilePath) {
+  Serial.printf("[%lu] [EBP] Parsing content.opf: %s\n", millis(), contentOpfFilePath.c_str());
+
   size_t contentOpfSize;
   if (!getItemSize(contentOpfFilePath, &contentOpfSize)) {
     Serial.printf("[%lu] [EBP] Could not get size of content.opf\n", millis());
@@ -63,7 +61,6 @@ bool Epub::parseContentOpf(const std::string& contentOpfFilePath) {
 
   if (!readItemContentsToStream(contentOpfFilePath, opfParser, 1024)) {
     Serial.printf("[%lu] [EBP] Could not read content.opf\n", millis());
-    opfParser.teardown();
     return false;
   }
 
@@ -84,8 +81,6 @@ bool Epub::parseContentOpf(const std::string& contentOpfFilePath) {
   }
 
   Serial.printf("[%lu] [EBP] Successfully parsed content.opf\n", millis());
-
-  opfParser.teardown();
   return true;
 }
 
@@ -96,6 +91,8 @@ bool Epub::parseTocNcxFile() {
     return false;
   }
 
+  Serial.printf("[%lu] [EBP] Parsing toc ncx file: %s\n", millis(), tocNcxItem.c_str());
+
   size_t tocSize;
   if (!getItemSize(tocNcxItem, &tocSize)) {
     Serial.printf("[%lu] [EBP] Could not get size of toc ncx\n", millis());
@@ -111,15 +108,12 @@ bool Epub::parseTocNcxFile() {
 
   if (!readItemContentsToStream(tocNcxItem, ncxParser, 1024)) {
     Serial.printf("[%lu] [EBP] Could not read toc ncx stream\n", millis());
-    ncxParser.teardown();
     return false;
   }
 
   this->toc = std::move(ncxParser.toc);
 
   Serial.printf("[%lu] [EBP] Parsed %d TOC items\n", millis(), this->toc.size());
-
-  ncxParser.teardown();
   return true;
 }
 

lib/Epub/Epub/Section.h

@@ -21,9 +21,10 @@ class Section {
   int currentPage = 0;
 
   explicit Section(const std::shared_ptr<Epub>& epub, const int spineIndex, GfxRenderer& renderer)
-      : epub(epub), spineIndex(spineIndex), renderer(renderer) {
-    cachePath = epub->getCachePath() + "/" + std::to_string(spineIndex);
-  }
+      : epub(epub),
+        spineIndex(spineIndex),
+        renderer(renderer),
+        cachePath(epub->getCachePath() + "/" + std::to_string(spineIndex)) {}
   ~Section() = default;
   bool loadCacheMetadata(int fontId, float lineCompression, int marginTop, int marginRight, int marginBottom,
                          int marginLeft, bool extraParagraphSpacing);

lib/Epub/Epub/parsers/ContainerParser.cpp

@@ -14,12 +14,11 @@ bool ContainerParser::setup() {
   return true;
 }
 
-bool ContainerParser::teardown() {
+ContainerParser::~ContainerParser() {
   if (parser) {
     XML_ParserFree(parser);
     parser = nullptr;
   }
-  return true;
 }
 
 size_t ContainerParser::write(const uint8_t data) { return write(&data, 1); }

lib/Epub/Epub/parsers/ContainerParser.h

@@ -23,9 +23,9 @@ class ContainerParser final : public Print {
   std::string fullPath;
 
   explicit ContainerParser(const size_t xmlSize) : remainingSize(xmlSize) {}
+  ~ContainerParser() override;
 
   bool setup();
-  bool teardown();
 
   size_t write(uint8_t) override;
   size_t write(const uint8_t* buffer, size_t size) override;

lib/Epub/Epub/parsers/ContentOpfParser.cpp

@@ -4,7 +4,7 @@
 #include <ZipFile.h>
 
 namespace {
-constexpr const char MEDIA_TYPE_NCX[] = "application/x-dtbncx+xml";
+constexpr char MEDIA_TYPE_NCX[] = "application/x-dtbncx+xml";
 }
 
 bool ContentOpfParser::setup() {
@@ -20,12 +20,11 @@ bool ContentOpfParser::setup() {
   return true;
 }
 
-bool ContentOpfParser::teardown() {
+ContentOpfParser::~ContentOpfParser() {
   if (parser) {
     XML_ParserFree(parser);
     parser = nullptr;
   }
-  return true;
 }
 
 size_t ContentOpfParser::write(const uint8_t data) { return write(&data, 1); }

lib/Epub/Epub/parsers/ContentOpfParser.h

@@ -34,9 +34,9 @@ class ContentOpfParser final : public Print {
 
   explicit ContentOpfParser(const std::string& baseContentPath, const size_t xmlSize)
       : baseContentPath(baseContentPath), remainingSize(xmlSize) {}
+  ~ContentOpfParser() override;
 
   bool setup();
-  bool teardown();
 
   size_t write(uint8_t) override;
   size_t write(const uint8_t* buffer, size_t size) override;

lib/Epub/Epub/parsers/TocNcxParser.cpp

@@ -1,5 +1,6 @@
 #include "TocNcxParser.h"
 
+#include <Esp.h>
 #include <HardwareSerial.h>
 
 bool TocNcxParser::setup() {
@@ -15,12 +16,11 @@ bool TocNcxParser::setup() {
   return true;
 }
 
-bool TocNcxParser::teardown() {
+TocNcxParser::~TocNcxParser() {
   if (parser) {
     XML_ParserFree(parser);
     parser = nullptr;
   }
-  return true;
 }
 
 size_t TocNcxParser::write(const uint8_t data) { return write(&data, 1); }

lib/Epub/Epub/parsers/TocNcxParser.h

@@ -28,9 +28,9 @@ class TocNcxParser final : public Print {
 
   explicit TocNcxParser(const std::string& baseContentPath, const size_t xmlSize)
       : baseContentPath(baseContentPath), remainingSize(xmlSize) {}
+  ~TocNcxParser() override;
 
   bool setup();
-  bool teardown();
 
   size_t write(uint8_t) override;
   size_t write(const uint8_t* buffer, size_t size) override;

platformio.ini

@@ -9,8 +9,8 @@ framework = arduino
 monitor_speed = 115200
 upload_speed = 921600
 check_tool = cppcheck
+check_flags = --enable=all --suppress=missingIncludeSystem --suppress=unusedFunction --suppress=unmatchedSuppression --inline-suppr
 check_skip_packages = yes
-check_severity = medium, high
 
 board_upload.flash_size = 16MB
 board_upload.maximum_size = 16777216

src/WifiCredentialStore.cpp

@@ -111,13 +111,13 @@ bool WifiCredentialStore::loadFromFile() {
 
 bool WifiCredentialStore::addCredential(const std::string& ssid, const std::string& password) {
   // Check if this SSID already exists and update it
-  for (auto& cred : credentials) {
-    if (cred.ssid == ssid) {
-      cred.password = password;
+  const auto cred = find_if(credentials.begin(), credentials.end(),
+                            [&ssid](const WifiCredential& cred) { return cred.ssid == ssid; });
+  if (cred != credentials.end()) {
+    cred->password = password;
     Serial.printf("[%lu] [WCS] Updated credentials for: %s\n", millis(), ssid.c_str());
     return saveToFile();
   }
-  }
 
   // Check if we've reached the limit
   if (credentials.size() >= MAX_NETWORKS) {
@@ -132,22 +132,24 @@ bool WifiCredentialStore::addCredential(const std::string& ssid, const std::stri
 }
 
 bool WifiCredentialStore::removeCredential(const std::string& ssid) {
-  for (auto it = credentials.begin(); it != credentials.end(); ++it) {
-    if (it->ssid == ssid) {
-      credentials.erase(it);
+  const auto cred = find_if(credentials.begin(), credentials.end(),
+                            [&ssid](const WifiCredential& cred) { return cred.ssid == ssid; });
+  if (cred != credentials.end()) {
+    credentials.erase(cred);
     Serial.printf("[%lu] [WCS] Removed credentials for: %s\n", millis(), ssid.c_str());
     return saveToFile();
   }
-  }
   return false;  // Not found
 }
 
 const WifiCredential* WifiCredentialStore::findCredential(const std::string& ssid) const {
-  for (const auto& cred : credentials) {
-    if (cred.ssid == ssid) {
-      return &cred;
-    }
+  const auto cred = find_if(credentials.begin(), credentials.end(),
+                            [&ssid](const WifiCredential& cred) { return cred.ssid == ssid; });
+
+  if (cred != credentials.end()) {
+    return &*cred;
   }
+
   return nullptr;
 }
 

src/activities/boot_sleep/SleepActivity.cpp

@@ -1,11 +1,11 @@
 #include "SleepActivity.h"
 
 #include <GfxRenderer.h>
+#include <SD.h>
 
 #include <vector>
 
 #include "CrossPointSettings.h"
-#include "SD.h"
 #include "config.h"
 #include "images/CrossLarge.h"
 

src/activities/network/CrossPointWebServerActivity.cpp

@@ -217,8 +217,7 @@ void CrossPointWebServerActivity::render() const {
 }
 
 void CrossPointWebServerActivity::renderServerRunning() const {
-  const auto pageWidth = GfxRenderer::getScreenWidth();
-  const auto pageHeight = GfxRenderer::getScreenHeight();
+  const auto pageHeight = renderer.getScreenHeight();
   const auto height = renderer.getLineHeight(UI_FONT_ID);
   const auto top = (pageHeight - height * 5) / 2;
 
@@ -226,7 +225,7 @@ void CrossPointWebServerActivity::renderServerRunning() const {
 
   std::string ssidInfo = "Network: " + connectedSSID;
   if (ssidInfo.length() > 28) {
-    ssidInfo = ssidInfo.substr(0, 25) + "...";
+    ssidInfo.replace(25, ssidInfo.length() - 25, "...");
   }
   renderer.drawCenteredText(UI_FONT_ID, top + 10, ssidInfo.c_str(), true, REGULAR);
 

src/activities/network/WifiSelectionActivity.cpp

@@ -138,6 +138,7 @@ void WifiSelectionActivity::processWifiScanResults() {
   // Convert map to vector
   networks.clear();
   for (const auto& pair : uniqueNetworks) {
+    // cppcheck-suppress useStlAlgorithm
     networks.push_back(pair.second);
   }
 
@@ -334,11 +335,10 @@ void WifiSelectionActivity::loop() {
         // User chose "Yes" - forget the network
         WIFI_STORE.removeCredential(selectedSSID);
         // Update the network list to reflect the change
-        for (auto& network : networks) {
-          if (network.ssid == selectedSSID) {
-            network.hasSavedPassword = false;
-            break;
-          }
+        const auto network = find_if(networks.begin(), networks.end(),
+                                     [this](const WifiNetworkInfo& net) { return net.ssid == selectedSSID; });
+        if (network != networks.end()) {
+          network->hasSavedPassword = false;
         }
       }
       // Go back to network list
@@ -468,8 +468,8 @@ void WifiSelectionActivity::render() const {
 }
 
 void WifiSelectionActivity::renderNetworkList() const {
-  const auto pageWidth = GfxRenderer::getScreenWidth();
-  const auto pageHeight = GfxRenderer::getScreenHeight();
+  const auto pageWidth = renderer.getScreenWidth();
+  const auto pageHeight = renderer.getScreenHeight();
 
   // Draw header
   renderer.drawCenteredText(READER_FONT_ID, 10, "WiFi Networks", true, BOLD);
@@ -506,7 +506,7 @@ void WifiSelectionActivity::renderNetworkList() const {
       // Draw network name (truncate if too long)
       std::string displayName = network.ssid;
       if (displayName.length() > 16) {
-        displayName = displayName.substr(0, 13) + "...";
+        displayName.replace(13, displayName.length() - 13, "...");
       }
       renderer.drawText(UI_FONT_ID, 20, networkY, displayName.c_str());
 
@@ -544,15 +544,13 @@ void WifiSelectionActivity::renderNetworkList() const {
 }
 
 void WifiSelectionActivity::renderPasswordEntry() const {
-  const auto pageHeight = GfxRenderer::getScreenHeight();
-
   // Draw header
   renderer.drawCenteredText(READER_FONT_ID, 5, "WiFi Password", true, BOLD);
 
   // Draw network name with good spacing from header
   std::string networkInfo = "Network: " + selectedSSID;
   if (networkInfo.length() > 30) {
-    networkInfo = networkInfo.substr(0, 27) + "...";
+    networkInfo.replace(27, networkInfo.length() - 27, "...");
   }
   renderer.drawCenteredText(UI_FONT_ID, 38, networkInfo.c_str(), true, REGULAR);
 
@@ -563,7 +561,7 @@ void WifiSelectionActivity::renderPasswordEntry() const {
 }
 
 void WifiSelectionActivity::renderConnecting() const {
-  const auto pageHeight = GfxRenderer::getScreenHeight();
+  const auto pageHeight = renderer.getScreenHeight();
   const auto height = renderer.getLineHeight(UI_FONT_ID);
   const auto top = (pageHeight - height) / 2;
 
@@ -574,15 +572,14 @@ void WifiSelectionActivity::renderConnecting() const {
 
     std::string ssidInfo = "to " + selectedSSID;
     if (ssidInfo.length() > 25) {
-      ssidInfo = ssidInfo.substr(0, 22) + "...";
+      ssidInfo.replace(22, ssidInfo.length() - 22, "...");
     }
     renderer.drawCenteredText(UI_FONT_ID, top, ssidInfo.c_str(), true, REGULAR);
   }
 }
 
 void WifiSelectionActivity::renderConnected() const {
-  const auto pageWidth = GfxRenderer::getScreenWidth();
-  const auto pageHeight = GfxRenderer::getScreenHeight();
+  const auto pageHeight = renderer.getScreenHeight();
   const auto height = renderer.getLineHeight(UI_FONT_ID);
   const auto top = (pageHeight - height * 4) / 2;
 
@@ -590,7 +587,7 @@ void WifiSelectionActivity::renderConnected() const {
 
   std::string ssidInfo = "Network: " + selectedSSID;
   if (ssidInfo.length() > 28) {
-    ssidInfo = ssidInfo.substr(0, 25) + "...";
+    ssidInfo.replace(25, ssidInfo.length() - 25, "...");
   }
   renderer.drawCenteredText(UI_FONT_ID, top + 10, ssidInfo.c_str(), true, REGULAR);
 
@@ -601,8 +598,8 @@ void WifiSelectionActivity::renderConnected() const {
 }
 
 void WifiSelectionActivity::renderSavePrompt() const {
-  const auto pageWidth = GfxRenderer::getScreenWidth();
-  const auto pageHeight = GfxRenderer::getScreenHeight();
+  const auto pageWidth = renderer.getScreenWidth();
+  const auto pageHeight = renderer.getScreenHeight();
   const auto height = renderer.getLineHeight(UI_FONT_ID);
   const auto top = (pageHeight - height * 3) / 2;
 
@@ -610,7 +607,7 @@ void WifiSelectionActivity::renderSavePrompt() const {
 
   std::string ssidInfo = "Network: " + selectedSSID;
   if (ssidInfo.length() > 28) {
-    ssidInfo = ssidInfo.substr(0, 25) + "...";
+    ssidInfo.replace(25, ssidInfo.length() - 25, "...");
   }
   renderer.drawCenteredText(UI_FONT_ID, top, ssidInfo.c_str(), true, REGULAR);
 
@@ -641,7 +638,7 @@ void WifiSelectionActivity::renderSavePrompt() const {
 }
 
 void WifiSelectionActivity::renderConnectionFailed() const {
-  const auto pageHeight = GfxRenderer::getScreenHeight();
+  const auto pageHeight = renderer.getScreenHeight();
   const auto height = renderer.getLineHeight(UI_FONT_ID);
   const auto top = (pageHeight - height * 2) / 2;
 
@@ -651,8 +648,8 @@ void WifiSelectionActivity::renderConnectionFailed() const {
 }
 
 void WifiSelectionActivity::renderForgetPrompt() const {
-  const auto pageWidth = GfxRenderer::getScreenWidth();
-  const auto pageHeight = GfxRenderer::getScreenHeight();
+  const auto pageWidth = renderer.getScreenWidth();
+  const auto pageHeight = renderer.getScreenHeight();
   const auto height = renderer.getLineHeight(UI_FONT_ID);
   const auto top = (pageHeight - height * 3) / 2;
 
@@ -660,7 +657,7 @@ void WifiSelectionActivity::renderForgetPrompt() const {
 
   std::string ssidInfo = "Network: " + selectedSSID;
   if (ssidInfo.length() > 28) {
-    ssidInfo = ssidInfo.substr(0, 25) + "...";
+    ssidInfo.replace(25, ssidInfo.length() - 25, "...");
   }
   renderer.drawCenteredText(UI_FONT_ID, top, ssidInfo.c_str(), true, REGULAR);
 

src/activities/network/server/CrossPointWebServer.cpp

@@ -383,9 +383,7 @@ void CrossPointWebServer::handleFileList() {
       // Folders come first
       if (a.isDirectory != b.isDirectory) return a.isDirectory > b.isDirectory;
       // Then sort by epub status (epubs first among files)
-      if (!a.isDirectory && !b.isDirectory) {
       if (a.isEpub != b.isEpub) return a.isEpub > b.isEpub;
-      }
       // Then alphabetically
       return a.name < b.name;
     });

src/activities/reader/EpubReaderActivity.cpp

@@ -383,7 +383,7 @@ void EpubReaderActivity::renderStatusBar() const {
     title = tocItem.title;
     titleWidth = renderer.getTextWidth(SMALL_FONT_ID, title.c_str());
     while (titleWidth > availableTextWidth && title.length() > 11) {
-      title = title.substr(0, title.length() - 8) + "...";
+      title.replace(title.length() - 8, 8, "...");
       titleWidth = renderer.getTextWidth(SMALL_FONT_ID, title.c_str());
     }
   }

src/activities/reader/FileSelectionActivity.cpp

@@ -93,7 +93,7 @@ void FileSelectionActivity::loop() {
     }
   } else if (inputManager.wasPressed(InputManager::BTN_BACK)) {
     if (basepath != "/") {
-      basepath = basepath.substr(0, basepath.rfind('/'));
+      basepath.replace(basepath.find_last_of('/'), std::string::npos, "");
       if (basepath.empty()) basepath = "/";
       loadFiles();
       updateRequired = true;

src/activities/util/KeyboardEntryActivity.cpp

@@ -20,7 +20,7 @@ KeyboardEntryActivity::KeyboardEntryActivity(GfxRenderer& renderer, InputManager
 void KeyboardEntryActivity::setText(const std::string& newText) {
   text = newText;
   if (maxLength > 0 && text.length() > maxLength) {
-    text = text.substr(0, maxLength);
+    text.resize(maxLength);
   }
 }
 

src/main.cpp

@@ -5,7 +5,6 @@
 #include <InputManager.h>
 #include <SD.h>
 #include <SPI.h>
-#include <WiFi.h>
 #include <builtinFonts/bookerly_2b.h>
 #include <builtinFonts/bookerly_bold_2b.h>
 #include <builtinFonts/bookerly_bold_italic_2b.h>
@@ -203,20 +202,18 @@ void setup() {
 }
 
 void loop() {
-  static unsigned long lastLoopTime = 0;
   static unsigned long maxLoopDuration = 0;
-
-  unsigned long loopStartTime = millis();
-
+  const unsigned long loopStartTime = millis();
   static unsigned long lastMemPrint = 0;
+
+  inputManager.update();
+
   if (Serial && millis() - lastMemPrint >= 10000) {
     Serial.printf("[%lu] [MEM] Free: %d bytes, Total: %d bytes, Min Free: %d bytes\n", millis(), ESP.getFreeHeap(),
                   ESP.getHeapSize(), ESP.getMinFreeHeap());
     lastMemPrint = millis();
   }
 
-  inputManager.update();
-
   // Check for any user activity (button press or release)
   static unsigned long lastActivityTime = millis();
   if (inputManager.wasAnyPressed() || inputManager.wasAnyReleased()) {
@@ -237,13 +234,13 @@ void loop() {
     return;
   }
 
-  unsigned long activityStartTime = millis();
+  const unsigned long activityStartTime = millis();
   if (currentActivity) {
     currentActivity->loop();
   }
-  unsigned long activityDuration = millis() - activityStartTime;
+  const unsigned long activityDuration = millis() - activityStartTime;
 
-  unsigned long loopDuration = millis() - loopStartTime;
+  const unsigned long loopDuration = millis() - loopStartTime;
   if (loopDuration > maxLoopDuration) {
     maxLoopDuration = loopDuration;
     if (maxLoopDuration > 50) {
@@ -252,8 +249,6 @@ void loop() {
     }
   }
 
-  lastLoopTime = loopStartTime;
-
   // Add delay at the end of the loop to prevent tight spinning
   // When an activity requests skip loop delay (e.g., webserver running), use yield() for faster response
   // Otherwise, use longer delay to save power