Skip to content

Add transactional named saved-list management - #1446

Draft
p-keminer wants to merge 3 commits into
justcallmekoko:developfrom
p-keminer:feat/safe-saved-list-management
Draft

Add transactional named saved-list management#1446
p-keminer wants to merge 3 commits into
justcallmekoko:developfrom
p-keminer:feat/safe-saved-list-management

Conversation

@p-keminer

Copy link
Copy Markdown

Stack

Summary

  • add type-filtered .log pickers and user-defined Save As names for SSID, AP, and AirTag lists
  • write through temporary and backup files with reopen/verification before commit
  • load lists atomically, replace in-memory snapshots, and recover only matching list types
  • report loaded/saved counts and keep status messages visible

Testing

  • Focused K12/K13 ESP32-C5 tests covered type-filtered pickers, atomic rejection of malformed loads, replace and empty-list handling, Save As, targeted SSID/AP/AirTag recovery, status counts, reboot persistence, and serial end-state checks.
  • Not fault-injected: a full SD, isolated flush failure, power loss during commit, or FAT temporary/backup collision.

@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2578 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.35%. Comparing base (66f3c81) to head (0e47bce).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
esp32_marauder/WiFiScan.cpp 0.00% 2036 Missing ⚠️
esp32_marauder/MenuFunctions.cpp 0.00% 490 Missing ⚠️
esp32_marauder/SDInterface.cpp 0.00% 52 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #1446      +/-   ##
==========================================
- Coverage     0.40%   0.35%   -0.05%     
==========================================
  Files           20      20              
  Lines        17176   19451    +2275     
  Branches        11      11              
==========================================
  Hits            69      69              
- Misses       17103   19378    +2275     
  Partials         4       4              
Flag Coverage Δ
firmware-unit-tests 0.35% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@p-keminer
p-keminer marked this pull request as ready for review August 9, 2026 21:09
Copilot AI lite review requested due to automatic review settings August 9, 2026 21:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces transactional, named saved-list management for WiFiScan saved artifacts (SSIDs, APs, AirTags), including SD-backed atomic save/load with recovery behavior and updated UI flows for selecting and naming .log files.

Changes:

  • Add saved-list types/results and new transactional save/load APIs (with temp/backup files, verification, and recovery).
  • Extend SD directory listing to support prefix-filtered, bounded results for type-filtered pickers.
  • Update screen menus to support saved-list pickers, “Save As” naming, replace confirmation, and persistent status screens.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
esp32_marauder/WiFiScan.h Adds saved-list enums/structures and updates save/load method signatures to support transactional behavior and status reporting.
esp32_marauder/WiFiScan.cpp Implements atomic save/load, verification, snapshots, and recovery paths for SSID/AP/AirTag lists.
esp32_marauder/SDInterface.h Expands listDirToLinkedList to support prefix filtering, result limits, truncation reporting, and success/failure return.
esp32_marauder/SDInterface.cpp Implements the enhanced, bounded, filtered SD directory listing with heap guarding.
esp32_marauder/MenuFunctions.h Adds saved-list picker/save/replace/status menu helpers and supporting enum.
esp32_marauder/MenuFunctions.cpp Implements saved-list pickers, Save As flow, replace confirmation, and load/save status screens.
Suppressed comments (2)

esp32_marauder/WiFiScan.cpp:5187

  • DynamicJsonDocument doc(10048) duplicates the MAX_SAVED_AIRTAG_FILE_SIZE constant (also 10048). Using the constant here avoids divergence if the AirTag list size limit changes.
    if (file_size > MAX_SAVED_AIRTAG_FILE_SIZE) {
      file.close();
      return fail(F("file is too large"));
    }

    DynamicJsonDocument doc(10048);
    const DeserializationError error = deserializeJson(doc, file);
    const bool trailing_data_valid = !error &&
      hasOnlySavedListTrailingWhitespace(file);
    file.close();

esp32_marauder/WiFiScan.cpp:5226

  • Parsing the AirTag payload currently creates a temporary String copy of the JSON field (String(object["payload"].as<const char*>())). For large payloads this doubles peak memory and can cause valid loads/verifications to fail under memory pressure. Consider adding an overload of parseSavedAirTagPayload that takes const char* (+ length) so you can parse the ArduinoJson-owned string without allocating a second copy.
        return fail_loaded(F("invalid MAC"));

      const uint16_t payload_size = object["payload_size"].as<uint16_t>();
      std::vector<uint8_t> payload;
      if (!parseSavedAirTagPayload(
            String(object["payload"].as<const char*>()),
            payload_size,
            payload))
        return fail_loaded(F("invalid payload"));

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread esp32_marauder/SDInterface.cpp
Comment thread esp32_marauder/WiFiScan.cpp Outdated
Comment thread esp32_marauder/WiFiScan.cpp
@p-keminer
p-keminer marked this pull request as draft August 9, 2026 21:56
@p-keminer

Copy link
Copy Markdown
Author

All 22 firmware matrix builds and the Native Unity tests pass on this head. The remaining red statuses are limited to the repository-wide Codecov policy addressed separately in #1448.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants