From 870bcb44cfc19d480545c152041852cea57e2a4f Mon Sep 17 00:00:00 2001 From: Ton Huisman Date: Wed, 26 Aug 2026 22:04:29 +0200 Subject: [PATCH] [Log] Refactor code --- src/src/DataStructs/LogBuffer.cpp | 8 +++++--- src/src/DataStructs/LogBuffer.h | 4 ++-- src/src/ESPEasyCore/ESPEasy_Console_Port.cpp | 2 +- src/src/Helpers/LogStreamWriter.cpp | 14 +++++++------ src/src/Helpers/LogStreamWriter.h | 21 +++++++++++--------- src/src/Helpers/Log_Helper.cpp | 15 ++++++-------- src/src/Helpers/Log_Helper.h | 14 ++++++------- src/src/Helpers/SyslogWriter.cpp | 21 ++++++++++---------- src/src/WebServer/Log.cpp | 8 ++++---- 9 files changed, 56 insertions(+), 51 deletions(-) diff --git a/src/src/DataStructs/LogBuffer.cpp b/src/src/DataStructs/LogBuffer.cpp index f3b3e3f06..d07b00b32 100644 --- a/src/src/DataStructs/LogBuffer.cpp +++ b/src/src/DataStructs/LogBuffer.cpp @@ -34,15 +34,17 @@ bool LogBuffer::getNext(LogDestination logDestination, uint32_t& timestamp, Stri return false; } -uint32_t LogBuffer::getNrMessages(LogDestination logDestination) +bool LogBuffer::hasMessages(LogDestination logDestination) { uint32_t res{}; if (logDestination >= NR_LOG_TO_DESTINATIONS) { return res; } + clearExpiredEntries(); // Cleanup the old stuff first + uint32_t pos = cache_iterator_pos[logDestination]; - for (; pos < LogEntries.size(); ++pos) { + for (; pos < LogEntries.size() && !res; ++pos) { if (LogEntries[pos].validForSubscriber(logDestination)) { ++res; } @@ -51,7 +53,7 @@ uint32_t LogBuffer::getNrMessages(LogDestination logDestination) if (!res) { lastReadTimeStamp[logDestination] = millis(); // Reset if we aren't going to fetch a next message } - return res; + return !!res; } bool LogBuffer::logActiveRead(LogDestination logDestination) { diff --git a/src/src/DataStructs/LogBuffer.h b/src/src/DataStructs/LogBuffer.h index 4eeb76779..f55cc6f3b 100644 --- a/src/src/DataStructs/LogBuffer.h +++ b/src/src/DataStructs/LogBuffer.h @@ -52,8 +52,8 @@ struct LogBuffer { String & message, uint8_t & loglevel); - // Return the number of messages left for given log destination. - uint32_t getNrMessages(LogDestination logDestination); + // Return true if messages available for given log destination. + bool hasMessages(LogDestination logDestination); bool logActiveRead(LogDestination logDestination); diff --git a/src/src/ESPEasyCore/ESPEasy_Console_Port.cpp b/src/src/ESPEasyCore/ESPEasy_Console_Port.cpp index 502f8793d..01354863b 100644 --- a/src/src/ESPEasyCore/ESPEasy_Console_Port.cpp +++ b/src/src/ESPEasyCore/ESPEasy_Console_Port.cpp @@ -220,7 +220,7 @@ ESPEasySerialPort EspEasy_Console_Port::getPortType() const bool EspEasy_Console_Port::process_serialWriteBuffer() { - if ((_serial == nullptr) || (_serialWriteBuffer.getNrMessages() == 0)) { + if ((_serial == nullptr) || !_serialWriteBuffer.hasMessages()) { return false; } #ifdef ESP32 diff --git a/src/src/Helpers/LogStreamWriter.cpp b/src/src/Helpers/LogStreamWriter.cpp index 5ee13c1d5..2b5646a19 100644 --- a/src/src/Helpers/LogStreamWriter.cpp +++ b/src/src/Helpers/LogStreamWriter.cpp @@ -20,11 +20,11 @@ bool LogStreamWriter::process(Print*stream, size_t availableForWrite) return write(*stream, availableForWrite) != 0; } -bool LogStreamWriter::process() { return false; } +bool LogStreamWriter::process() { return false; } -uint32_t LogStreamWriter::getNrMessages() const +bool LogStreamWriter::hasMessages() const { - return Logging.getNrMessages(_log_destination); + return Logging.hasMessages(_log_destination); } size_t LogStreamWriter::write(Print& stream, size_t nrBytesToWrite) @@ -84,9 +84,9 @@ size_t LogStreamWriter::write_item(Print& stream, bool done = false; - while (!done && bytesWritten < nrBytesToWrite) { const size_t bytesWritten_startLoop = bytesWritten; + if (!_prefix.isEmpty()) { bytesWritten += write_part(_prefix, stream, nrBytesToWrite - bytesWritten); } @@ -101,10 +101,11 @@ size_t LogStreamWriter::write_item(Print& stream, clear(); done = true; } + if (bytesWritten_startLoop == bytesWritten) - { + { // Nothing written in this loop, retry later. - return bytesWritten; + return bytesWritten; } } return bytesWritten; @@ -119,6 +120,7 @@ size_t LogStreamWriter::write_part(String& str, Print& stream, size_t nrBytesTo bytesWritten = stream.write(&str[_readpos], bytesLeft); _readpos += bytesWritten; } + if (_readpos >= str.length()) { // Clear str str.clear(); diff --git a/src/src/Helpers/LogStreamWriter.h b/src/src/Helpers/LogStreamWriter.h index 40df86e20..712c981e8 100644 --- a/src/src/Helpers/LogStreamWriter.h +++ b/src/src/Helpers/LogStreamWriter.h @@ -11,32 +11,35 @@ public: virtual ~LogStreamWriter() {} - virtual bool process(Print* stream, size_t availableForWrite); - + virtual bool process(Print *stream, + size_t availableForWrite); + // Only use this from derived classes, as we need a Stream to further process - virtual bool process(); + virtual bool process(); - virtual uint32_t getNrMessages() const; + virtual bool hasMessages() const; - virtual void clear(); + virtual void clear(); protected: // Write continuously until either nrBytesToWrite was reached or no new messages were available to process. // @retval Number of bytes written. Zero when no new message was available to process. virtual size_t write(Print& stream, - size_t nrBytesToWrite); + size_t nrBytesToWrite); // Write single item and clear() on return. // This way each call starts with a new item and long messages may get truncated based on nrBytesToWrite // @retval Number of bytes written. Zero when no new message was available to process. virtual size_t write_single_item(Print& stream, - size_t nrBytesToWrite); + size_t nrBytesToWrite); virtual size_t write_item(Print& stream, - size_t nrBytesToWrite); + size_t nrBytesToWrite); - size_t write_part(String& str, Print& stream, size_t nrBytesToWrite); + size_t write_part(String& str, + Print & stream, + size_t nrBytesToWrite); virtual size_t write_skipping(Print& stream); diff --git a/src/src/Helpers/Log_Helper.cpp b/src/src/Helpers/Log_Helper.cpp index bd6a8615f..1871ce0a2 100644 --- a/src/src/Helpers/Log_Helper.cpp +++ b/src/src/Helpers/Log_Helper.cpp @@ -14,6 +14,7 @@ #endif // if FEATURE_SD #if FEATURE_SD + void addToSDLog(uint8_t logLevel, const String& str) { if (!str.isEmpty() && loglevelActiveFor(LOG_TO_SDCARD, logLevel)) { @@ -31,6 +32,7 @@ void addToSDLog(uint8_t logLevel, const String& str) logFile.close(); } } + #endif // if FEATURE_SD void LogHelper::addLogEntry(LogEntry_t&& logEntry) @@ -60,14 +62,12 @@ bool LogHelper::getNext(LogDestination logDestination, uint32_t& timestamp, Stri return _logBuffer.getNext(logDestination, timestamp, message, loglevel); } -uint32_t LogHelper::getNrMessages(LogDestination logDestination) -{ - return _logBuffer.getNrMessages(logDestination); -} +bool LogHelper::hasMessages(LogDestination logDestination) { return _logBuffer.hasMessages(logDestination); } void LogHelper::loop(bool serialOnly) { #if FEATURE_SD + if (!serialOnly) { String message; uint32_t timestamp{}; @@ -78,14 +78,11 @@ void LogHelper::loop(bool serialOnly) addToSDLog(loglevel, message); } } -#endif +#endif // if FEATURE_SD _logBuffer.clearExpiredEntries(); } -bool LogHelper::logActiveRead(LogDestination logDestination) -{ - return _logBuffer.logActiveRead(logDestination); -} +bool LogHelper::logActiveRead(LogDestination logDestination) { return _logBuffer.logActiveRead(logDestination); } void LogHelper::consolePrint(const __FlashStringHelper *text) { _tmpConsoleOutput += text; } diff --git a/src/src/Helpers/Log_Helper.h b/src/src/Helpers/Log_Helper.h index 1de25d19a..7ba705a1c 100644 --- a/src/src/Helpers/Log_Helper.h +++ b/src/src/Helpers/Log_Helper.h @@ -14,16 +14,16 @@ public: void addLogEntry(LogEntry_t&& logEntry); - bool getNext(LogDestination logDestination, - uint32_t& timestamp, - String & message, - uint8_t & loglevel); + bool getNext(LogDestination logDestination, + uint32_t & timestamp, + String & message, + uint8_t & loglevel); - uint32_t getNrMessages(LogDestination logDestination); + bool hasMessages(LogDestination logDestination); - void loop(bool serialOnly); + void loop(bool serialOnly); - bool logActiveRead(LogDestination logDestination); + bool logActiveRead(LogDestination logDestination); // Append to internal buffer, which will only be flushed on consolePrintln diff --git a/src/src/Helpers/SyslogWriter.cpp b/src/src/Helpers/SyslogWriter.cpp index def302801..0752db75e 100644 --- a/src/src/Helpers/SyslogWriter.cpp +++ b/src/src/Helpers/SyslogWriter.cpp @@ -3,19 +3,19 @@ #if FEATURE_SYSLOG -#include "../../ESPEasy/net/ESPEasyNetwork.h" -#include "../../ESPEasy/net/Globals/NetworkState.h" -#include "../Globals/ESPEasy_time.h" -#include "../Globals/Settings.h" -#include "../Helpers/ESPEasy_time_calc.h" -#include "../Helpers/Networking.h" -#include "../Helpers/StringConverter.h" +# include "../../ESPEasy/net/ESPEasyNetwork.h" +# include "../../ESPEasy/net/Globals/NetworkState.h" +# include "../Globals/ESPEasy_time.h" +# include "../Globals/Settings.h" +# include "../Helpers/ESPEasy_time_calc.h" +# include "../Helpers/Networking.h" +# include "../Helpers/StringConverter.h" -#define MAX_LENGTH_SYSLOG_MESSAGE 1000 +# define MAX_LENGTH_SYSLOG_MESSAGE 1000 bool SyslogWriter::process() { - if ((Settings.SyslogLevel == 0) || (getNrMessages() == 0)) { + if ((Settings.SyslogLevel == 0) || !hasMessages()) { return false; } @@ -136,4 +136,5 @@ void SyslogWriter::prepare_prefix() formattedTimestamp.c_str(), hostname.c_str()); } -#endif \ No newline at end of file + +#endif // if FEATURE_SYSLOG diff --git a/src/src/WebServer/Log.cpp b/src/src/WebServer/Log.cpp index 54f6996fb..1b9a01c0e 100644 --- a/src/src/WebServer/Log.cpp +++ b/src/src/WebServer/Log.cpp @@ -111,10 +111,10 @@ void handle_log_JSON() { } } } - const uint32_t nrEntriesLeft = Logging.getNrMessages(LOG_TO_WEBLOG); - int32_t logTimeSpan = timeDiff(firstTimeStamp, lastTimeStamp); - int32_t refreshSuggestion = (nrEntriesLeft > 0) ? 200 : 1000; - int32_t newOptimum = 1000; + const bool entriesAvailable = Logging.hasMessages(LOG_TO_WEBLOG); + const int32_t logTimeSpan = timeDiff(firstTimeStamp, lastTimeStamp); + int32_t refreshSuggestion = entriesAvailable ? 200 : 1000; + int32_t newOptimum = 1000; if ((nrEntries > 2) && (logTimeSpan > 1)) {