Firmware JSON document memory management

LoggerFirmware/ReleaseNotes.md

@@ -1,5 +1,13 @@
 # Release Notes: Logger Firmware
 
+## Firmware 1.6.2
+
+Firmware 1.6.2 addresses [issue 107](https://github.com/CCOMJHC/WIBL/issues/107) in the repository, where the size of the JSON documents being constructed in the firmware were getting to the size where they took up enough memory that they caused problems when generating SSL connections during upload.  This also seemed to have significantly slowed down the webserver, so this version of the firmware should show much snappier connections on the server as a (very welcome) side effect.
+
+In addition to managing the memory better in the firmware, the modifications here change the status message so that it no longer provides all of the file information (names, sizes, checksums, etc.)  The code adds a "catalog" command that now provides this information; the "snapshot catalog" command can also be used to generate this information as a file that can be downloaded using the standard browser mechanism.
+
+The [original PR](https://github.com/CCOMJHC/WIBL/pull/108) for this (which is used here with slight adjustments for the upload server end of the connection) was contributed by the awesome team at [Spatialnetics](https://spatialnetics.com).
+
 ## Firmware 1.6.1
 
 Firmware 1.6.1 addresses [issue 85](https://github.com/CCOMJHC/WIBL/issues/85) in the repository, which is a bug in the generation of "last known good" data in the JSON response to the "status" command, which shows up as failure to parse the JSON in the JavaScript.  A consequence of this is that the hardware data simulator required updates to (a) add a Depth datagram in the NMEA2000 output (fixed depth rather than fully simulated like the NMEA0183 output), and (b) flushing of buffers to ensure that NMEA0183 messages are sent correctly on the GGA/ZDA channel without problems.

LoggerFirmware/include/Configuration.h

@@ -38,7 +38,7 @@ namespace logger {
 // Firmware software version (i.e., overall firmware, rather than components like Command Processor, etc.)
 const int firmware_major = 1;
 const int firmware_minor = 6;
-const int firmware_patch = 1;
+const int firmware_patch = 2;
 
 /// @brief Stringify the version information for the firmware itself
 String FirmwareVersion(void);

LoggerFirmware/include/JSONUtilities.h

@@ -0,0 +1,28 @@
+
+
+#ifndef JSONUTILITIES_H
+#define JSONUTILITIES_H
+
+#include <ArduinoJson.h>
+
+inline bool GrowJsonDocumentBy(DynamicJsonDocument& json, size_t capacityIncrease)
+{
+    auto newCapacity = json.capacity() + capacityIncrease;
+    DynamicJsonDocument temp(newCapacity);
+
+    // Allocation failed. Nothing we can do.
+    if (temp.capacity() == 0) return false;
+
+    temp.set(json);
+    json = std::move(temp);
+    return true;
+}
+
+inline bool GrowJsonDocument(DynamicJsonDocument& json)
+{
+    // Grow capacity by 50%. A growth rate less than the golden ratio allows
+    // the document to potentially reuse allocations it previously occupied.
+    return GrowJsonDocumentBy(json, json.capacity() / 2);
+}
+
+#endif // JSONUTILITIES_H

LoggerFirmware/include/LogManager.h

@@ -74,9 +74,10 @@ class Manager {
     void RemoveAllLogfiles(void);
 
     /// \brief Count the number of log files on the system
-    uint32_t CountLogFiles(uint32_t filenumbers[MaxLogFiles]);
-    /// \brief Count the number of log files on the system
-    uint32_t CountLogFiles(void);
+    uint32_t CountLogFiles(uint64_t * fileSize = nullptr);
+
+    /// \brief Collect the file numbers of all log files on the system
+    std::vector<uint32_t> GetLogFileNumbers();
 
     class MD5Hash {
     public:
@@ -160,7 +161,7 @@ class Manager {
         bool Update(uint32_t filenum, MD5Hash *hash = nullptr);
         void RemoveLogFile(uint32_t filenum);
         uint32_t CountLogFiles(uint32_t filenumbers[MaxLogFiles]);
-        uint32_t CountLogFiles(void);
+        uint32_t CountLogFiles(uint64_t *totalFileSizes);
         uint32_t GetNextLogNumber(void);
         uint32_t Filesize(uint32_t filenum);
         uint16_t UploadCount(uint32_t filenum);
@@ -192,7 +193,7 @@ class Manager {
     /// \brief Extract a log number from a filename (if valid)
     int32_t ExtractLogNumber(String const& filename);
     /// \brief Count the number of log files on the system
-    uint32_t count(uint32_t *filenumbers);
+    uint32_t ScanLogFolder(uint32_t fileNumbers[MaxLogFiles], uint64_t *totalSize);
     /// \brief Extract information on a single log file
     void enumerate(uint32_t lognumber, String& filename, uint32_t& filesize);
     /// \brief Generate a hash for a given file 

LoggerFirmware/include/SerialCommand.h

@@ -166,6 +166,8 @@ class SerialCommand {
     void ReportScalesElement(CommandSource src);
     /// \brief Report the number of log files available on the SD card
     void ReportFileCount(CommandSource src);
+    /// \brief Report the properties of all log files on the SD card
+    void ReportCatalog(CommandSource src);
     /// \brief Report the configuration of the web-server
     void ReportWebserverConfig(CommandSource src);
     /// \brief Configure the web-server for system config/management
@@ -193,10 +195,10 @@ class SerialCommand {
 
     /// \brief Generate a string on the appropriate output stream
     void EmitMessage(String const& msg, CommandSource src);
-    /// \brief Convert a stringified JSON into a document, with error reporting
+    /// \brief Send stringified JSON, with error reporting
     bool EmitJSON(String const& source, CommandSource src);
-    /// @brief Generate a list of files into a JSON document
-    DynamicJsonDocument GenerateFilelist(void);
+    /// \brief Send a JSON document, with error reporting
+    void EmitJSON(DynamicJsonDocument && json, CommandSource src);
     /// @brief Display a NMEA0183 filter ID list
     void DisplayNMEAFilter(logger::N0183IDStore& filter, CommandSource src);
     /// @brief Display an Algorithm Store list

LoggerFirmware/include/WiFiAdapter.h

@@ -57,7 +57,8 @@ class WiFiAdapter {
     void AddMessage(String const& message);
 
     /// \brief Replace the entire message to be returned to the client for the current transaction
-    void SetMessage(DynamicJsonDocument const& message);
+    void SetMessage(JsonDocument const& message) { setMessage(message); }
+    void SetMessage(DynamicJsonDocument && message) { setMessage(std::move(message)); }
 
     enum HTTPReturnCodes {
         OK              = 200,  // The request succeeded
@@ -103,7 +104,8 @@ class WiFiAdapter {
     virtual void accumulateMessage(String const& message) = 0;
 
     /// \brief Sub-class implementation of code to replace message for transmission
-    virtual void setMessage(DynamicJsonDocument const& message) = 0;
+    virtual void setMessage(JsonDocument const& message) = 0;
+    virtual void setMessage(DynamicJsonDocument && message) = 0;
 
     /// \brief Sub-class implementation of code to set the status code for the transaction
     virtual void setStatusCode(HTTPReturnCodes status_code) = 0;

LoggerFirmware/src/AutoUpload.cpp

@@ -81,16 +81,14 @@ void UploadManager::UploadCycle(void)
             m_lastUploadCycle);
         return;
     }
-    DynamicJsonDocument files(logger::status::GenerateFilelist(m_logManager));
-    int filecount = files["files"]["count"].as<int>();
-    for (int n = 0; n < filecount; ++n) {
-        uint32_t file_id = files["files"]["detail"][n]["id"].as<uint32_t>();
-        if (TransferFile(m_logManager->FileSystem(), file_id)) {
+
+    for (auto fileNumber : m_logManager->GetLogFileNumbers()) {
+        bool success = TransferFile(m_logManager->FileSystem(), fileNumber);
+        if (success) {
             // File transferred to the server successfully, so we can delete locally
-            m_logManager->RemoveLogFile(file_id);
+            m_logManager->RemoveLogFile(fileNumber);
         } else {
             // File did not transfer, so we update the upload attempt metadata and move on
-
         }
         unsigned long current_elapsed = millis();
         if ((current_elapsed - start_time) > m_uploadDuration) {

LoggerFirmware/src/LogManager.cpp

@@ -99,8 +99,7 @@ Manager::Inventory::~Inventory(void)
 bool Manager::Inventory::Reinitialise(void)
 {
     uint32_t    *filenumbers = new uint32_t[MaxLogFiles];
-    uint32_t    filecount = m_logManager->count(filenumbers);
-    String      filename;
+    auto filecount = m_logManager->ScanLogFolder(filenumbers, nullptr);
     Manager::MD5Hash emptyhash;
 
     if (m_verbose)
@@ -166,12 +165,15 @@ uint32_t Manager::Inventory::CountLogFiles(uint32_t filenumbers[MaxLogFiles])
     return filecount;
 }
 
-uint32_t Manager::Inventory::CountLogFiles(void)
+uint32_t Manager::Inventory::CountLogFiles(uint64_t *totalFileSizes)
 {
+    uint64_t totalSize = 0;
     uint32_t filecount = 0;
-    for (uint32_t entry = 0; entry < MaxLogFiles; ++entry) {
-        if (m_filesize[entry] != 0) ++filecount;
+    for (auto fileSize : m_filesize) {
+        totalSize += fileSize;
+        filecount += (fileSize != 0);
     }
+    if (totalFileSizes) *totalFileSizes = totalSize;
     return filecount;
 }
 
@@ -297,15 +299,14 @@ void Manager::RemoveAllLogfiles(void)
 /// Normally, this finds all of the log files on the SD card, counts the total number, and returns
 /// the internal reference numbers that are being used so that the user can request more details
 /// about a specific file later.  In this dummy version, it simply prints a debug message, and returns
-/// zero.
+/// an empty vector.
 ///
-/// \param filenumbers  Fixed size array (of \a MaxLogFiles) written with the internal reference numbers for the files
-/// \return Total number of files available on the SD card
+/// \return An empty vector
 
-int Manager::CountLogFiles(int filenumbers[MaxLogFiles])
+std::vector<uint32_t> Manager::GetLogFileNumbers()
 {
-    Serial.println("DBG: Call to count log files; returning zero.");
-    return 0;
+    Serial.println("DBG: Call to get log files numbers; returning empty vector.");
+    return {};
 }
 
 /// \brief Determine the size and file name of the specified file
@@ -506,54 +507,46 @@ bool Manager::RemoveLogFile(uint32_t file_num)
 
 void Manager::RemoveAllLogfiles(void)
 {
-    uint32_t *filenumbers = new uint32_t[MaxLogFiles];
+    auto fileNumbers = GetLogFileNumbers();
 
     CloseLogfile(); // All means all ...
 
-    uint32_t filecount = CountLogFiles(filenumbers);
     uint32_t files_closed = 0;
-    for (uint32_t f = 0; f < filecount; ++f) {
-        String filename = MakeLogName(filenumbers[f]);
+    for (auto fileNumber : fileNumbers) {
+        String filename = MakeLogName(fileNumber);
         Serial.printf("INFO: erasing log file: \"%s\".\n", filename.c_str());
         bool rc = m_storage->Controller().remove(filename);
         if (rc) {
             m_consoleLog.printf("INFO: erased log file \"%s\" by user command.\n", filename.c_str());
             ++files_closed;
-            if (m_inventory != nullptr) m_inventory->RemoveLogFile(filenumbers[f]);
+            if (m_inventory != nullptr) m_inventory->RemoveLogFile(fileNumber);
         } else {
             m_consoleLog.printf("ERR: failed to erase log file \"%s\" by user command.\n", filename.c_str());
         }
     }
-    delete[] filenumbers;
-    m_consoleLog.printf("INFO: erased %u log files of %u.\n", files_closed, filecount);
+
+    m_consoleLog.printf("INFO: erased %u log files of %u.\n", files_closed, fileNumbers.size());
     m_consoleLog.flush();
     StartNewLog(); // We need to have something running for the logging effort!
 }
 
-/// Count the number of log files on the SD card, so that the client can enumerate them
-/// and report to the user.
-///
-/// \param filenumbers  (Out) Array of the log file numbers on card
-/// \return Number of files on the SD card
-
-uint32_t Manager::CountLogFiles(uint32_t filenumbers[MaxLogFiles])
+uint32_t Manager::CountLogFiles(uint64_t *fileSize)
 {
-    uint32_t filecount;
-    if (m_inventory != nullptr) {
-        filecount = m_inventory->CountLogFiles(filenumbers);
-    } else
-        filecount = count(filenumbers);
-    return filecount;
+    return m_inventory ? m_inventory->CountLogFiles(fileSize)
+                       : ScanLogFolder(nullptr, fileSize);
 }
 
-uint32_t Manager::CountLogFiles(void)
+std::vector<uint32_t> Manager::GetLogFileNumbers()
 {
-    uint32_t filecount;
-    if (m_inventory != nullptr) {
-        filecount = m_inventory->CountLogFiles();
-    } else
-        filecount = count(nullptr);
-    return filecount;
+    // Temporarily allocate an array of max size...
+    uint32_t *temp = new uint32_t[logger::MaxLogFiles];
+
+    uint32_t fileCount = m_inventory ? m_inventory->CountLogFiles(temp)
+                                 : ScanLogFolder(temp, nullptr);
+    auto fileNumbers = std::vector<uint32_t>(temp, temp + fileCount);
+
+    delete [] temp;
+    return fileNumbers;
 }
 
 /// Make a list of all of the files that exist on the SD card in the log directory, along with their
@@ -818,23 +811,22 @@ void Manager::TransferLogFile(uint32_t file_num, MD5Hash const& filehash, Stream
     Serial.printf("Sent %u B in %lu s.\n", bytes_transferred, duration);
 }
 
-uint32_t Manager::count(uint32_t *filenumbers)
+uint32_t Manager::ScanLogFolder(uint32_t fileNumbers[MaxLogFiles], uint64_t * totalSize)
 {
     uint32_t file_count = 0;
     File logdir = m_storage->Controller().open("/logs");
-    File entry = logdir.openNextFile();
-
-    while (entry) {
+    while (File entry = logdir.openNextFile()) {
         // Because we can store snapshots of configuration information in the /logs directory
         // (so that they can be seen through the webserver's static website for download), we
         // need to count only the valid log files in the directory.
         int32_t lognumber = ExtractLogNumber(String(entry.name()));
-        if (lognumber >= 0) {
-            if (filenumbers != nullptr) filenumbers[file_count] = lognumber;
+        auto size = entry.size();
+        if (lognumber >= 0 && size > 0) {
+            if (fileNumbers != nullptr) fileNumbers[file_count] = lognumber;
+            if (totalSize) *totalSize += size;
             ++file_count;
         }
         entry.close();
-        entry = logdir.openNextFile();
     }
     logdir.close();
     return file_count;

LoggerFirmware/src/NVMFile.cpp

@@ -26,6 +26,7 @@
 
 #include <set>
 #include "NVMFile.h"
+#include "JSONUtilities.h"
 #include "LittleFS.h"
 #include "LogManager.h"
 #include "serialisation.h"
@@ -348,10 +349,7 @@ void AlgoRequestStore::AddAlgorithm(String const& alg_name, String const& alg_pa
     entry["parameters"] = alg_params;
     if ((doc.memoryUsage() + entry.capacity()) > 0.95*doc.capacity()) {
         // The document needs to be bigger to handle the new entry
-        int capacity = doc.memoryUsage() + entry.capacity() + 256;
-        DynamicJsonDocument new_doc(capacity);
-        new_doc.set(doc);
-        doc = new_doc;
+        GrowJsonDocumentBy(doc, entry.capacity());
     }
     doc["algorithm"].add(entry.as<JsonObject>());
     doc["count"] = count + 1;