From 7bd6631366cd74de225bdb8ab4b44f4b84ed236e Mon Sep 17 00:00:00 2001 From: zekageri Date: Tue, 24 Feb 2026 12:50:59 +0100 Subject: [PATCH] Implement teardown contract for ESPLogger --- README.md | 10 ++ examples/basic_usage/basic_usage.ino | 10 ++ examples/custom_sync/custom_sync.ino | 14 +++ src/esp_logger/logger.cpp | 84 +++++++++++------ test/logger_tests.cpp | 132 +++++++++++++++++++++++++++ 5 files changed, 220 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 154e915..1f53bba 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,14 @@ void loop() { } ``` +Teardown explicitly when your component shuts down or reconfigures: + +```cpp +void stopLogging() { + logger.deinit(); +} +``` + Scope a logger inside your own class: ```cpp @@ -91,6 +99,7 @@ Prefer the ESP-IDF logging macros? Define `ESPLOGGER_USE_ESP_LOG=1` in your buil ## Gotchas - Keep `ESPLogger` instances alive for as long as their sync worker may run; destroying the object stops the worker. +- Call `logger.deinit()` during shutdown/reconfiguration so pending buffered logs are flushed and callbacks are detached deterministically. - When `enableSyncTask` is `false`, remember to call `logger.sync()` yourself or logs will stay buffered forever. - `setLogLevel` only affects console output; all logs remain available inside the RAM buffer until purged. - Inside `onSync`, the internal buffer has already been cleared—use the static helper overloads that take the `logs` snapshot to count or filter entries. @@ -99,6 +108,7 @@ Prefer the ESP-IDF logging macros? Define `ESPLOGGER_USE_ESP_LOG=1` in your buil ## API Reference - `bool init(const LoggerConfig& cfg = {})` – configure sync cadence, stack size, priorities, and thresholds. +- `void deinit()` / `bool isInitialized() const` – tear down runtime resources and inspect lifecycle state. - `void debug/info/warn/error(const char* tag, const char* fmt, ...)` – emit formatted logs. - `void attach(LiveCallback cb)` / `void detach()` – register or remove a per-entry live callback invoked on every emitted log entry. - `void setLogLevel(LogLevel level)` / `LogLevel logLevel() const` – adjust console verbosity at runtime. diff --git a/examples/basic_usage/basic_usage.ino b/examples/basic_usage/basic_usage.ino index a78a531..2ba6964 100644 --- a/examples/basic_usage/basic_usage.ino +++ b/examples/basic_usage/basic_usage.ino @@ -28,6 +28,7 @@ static HeartbeatReporter heartbeat(logger); // Optional: store a snapshot of logs the last time we synced. static std::vector lastSyncedLogs; +static bool loggerStopped = false; void logSyncCallback(const std::vector& logs) { lastSyncedLogs = logs; @@ -55,6 +56,10 @@ void setup() { void loop() { static uint32_t counter = 0; + if (loggerStopped) { + delay(1000); + return; + } logger.debug("LOOP", "This debug message only shows when consoleLogLevel <= Debug (%lu)", static_cast(counter)); @@ -62,5 +67,10 @@ void loop() { heartbeat.log(counter); counter++; + if (counter >= 30) { + logger.deinit(); + loggerStopped = true; + Serial.println("Logger deinitialized after demo run"); + } delay(1000); } diff --git a/examples/custom_sync/custom_sync.ino b/examples/custom_sync/custom_sync.ino index c1cb8fb..c0f3ebb 100644 --- a/examples/custom_sync/custom_sync.ino +++ b/examples/custom_sync/custom_sync.ino @@ -22,6 +22,8 @@ static SensorSampler sampler(logger); namespace { constexpr uint32_t kManualSyncIntervalMS = 5000; uint32_t lastSyncMs = 0; +uint32_t sampleCount = 0; +bool loggerStopped = false; } // namespace void persistLogs(const std::vector& logs) { @@ -55,12 +57,24 @@ void setup() { } void loop() { + if (loggerStopped) { + delay(1000); + return; + } + sampler.logReading(); + sampleCount++; if (millis() - lastSyncMs >= kManualSyncIntervalMS) { lastSyncMs = millis(); logger.sync(); // trigger persistence callback immediately } + if (sampleCount >= 40) { + logger.deinit(); + loggerStopped = true; + Serial.println("Logger deinitialized after manual-sync demo"); + } + delay(250); } diff --git a/src/esp_logger/logger.cpp b/src/esp_logger/logger.cpp index 0745e2f..d4862cb 100644 --- a/src/esp_logger/logger.cpp +++ b/src/esp_logger/logger.cpp @@ -114,6 +114,21 @@ static void invokeLiveCallback(const LiveCallback &callback, const Log &entry) { #endif } +static void invokeSyncCallback(const SyncCallback &callback, const std::vector &logs) { + if (!callback) { + return; + } + +#if defined(__cpp_exceptions) + try { + callback(logs); + } catch (...) { + // Never let user callbacks unwind through logger code. + } +#else + callback(logs); +#endif +} ESPLogger::~ESPLogger() { deinit(); @@ -161,18 +176,24 @@ bool ESPLogger::init(const LoggerConfig &config) { if (created != pdPASS) { _running = false; - LockGuard guard(_mutex); - _logs = InternalLogDeque(_logAllocator); - _config = LoggerConfig{}; - _logLevel = _config.consoleLogLevel; + { + LockGuard guard(_mutex); + _logs = InternalLogDeque(_logAllocator); + _syncCallback = nullptr; + _liveCallback = nullptr; + _config = LoggerConfig{}; + _logLevel = _config.consoleLogLevel; + } _usePSRAMBuffers = false; - _logAllocator = LoggerAllocator(_usePSRAMBuffers); - _charAllocator = LoggerAllocator(_usePSRAMBuffers); + _logAllocator = LoggerAllocator(_usePSRAMBuffers); + _charAllocator = LoggerAllocator(_usePSRAMBuffers); + if (_mutex != nullptr) { vSemaphoreDelete(_mutex); _mutex = nullptr; - _syncTask = nullptr; - return false; } + _syncTask = nullptr; + return false; + } } _initialized = true; @@ -180,10 +201,6 @@ bool ESPLogger::init(const LoggerConfig &config) { } void ESPLogger::deinit() { - if (!_initialized) { - return; - } - _running = false; if (_syncTask != nullptr) { @@ -197,25 +214,30 @@ void ESPLogger::deinit() { } } - performSync(); - - { - LockGuard guard(_mutex); - _logs = InternalLogDeque(_logAllocator); - _syncCallback = nullptr; - _liveCallback = nullptr; - _config = LoggerConfig{}; - _logLevel = _config.consoleLogLevel; - _usePSRAMBuffers = false; - _logAllocator = LoggerAllocator(_usePSRAMBuffers); - _charAllocator = LoggerAllocator(_usePSRAMBuffers); - } - if (_mutex != nullptr) { + performSync(); + { + LockGuard guard(_mutex); + _logs = InternalLogDeque(_logAllocator); + _syncCallback = nullptr; + _liveCallback = nullptr; + _config = LoggerConfig{}; + _logLevel = _config.consoleLogLevel; + } vSemaphoreDelete(_mutex); _mutex = nullptr; } + + _usePSRAMBuffers = false; + _logAllocator = LoggerAllocator(_usePSRAMBuffers); + _charAllocator = LoggerAllocator(_usePSRAMBuffers); + _logs = InternalLogDeque(_logAllocator); + _syncCallback = nullptr; + _liveCallback = nullptr; + _config = LoggerConfig{}; + _logLevel = _config.consoleLogLevel; _initialized = false; + _syncTask = nullptr; } void ESPLogger::onSync(SyncCallback callback) { @@ -429,15 +451,17 @@ void ESPLogger::performSync() { } callback = _syncCallback; + if (!callback) { + _logs.clear(); + return; + } logsSnapshot.reserve(_logs.size()); std::move(_logs.begin(), _logs.end(), std::back_inserter(logsSnapshot)); _logs.clear(); } - if (callback) { - std::vector callbackLogs(logsSnapshot.begin(), logsSnapshot.end()); - callback(callbackLogs); - } + std::vector callbackLogs(logsSnapshot.begin(), logsSnapshot.end()); + invokeSyncCallback(callback, callbackLogs); } void ESPLogger::syncTaskThunk(void *arg) { diff --git a/test/logger_tests.cpp b/test/logger_tests.cpp index 0047754..3a7428b 100644 --- a/test/logger_tests.cpp +++ b/test/logger_tests.cpp @@ -71,6 +71,66 @@ void test_init_applies_normalized_config() { expect_true(!logger.isInitialized(), "ESPLogger should be deinitialized"); } +void test_deinit_is_safe_before_init() { + ESPLogger logger; + expect_true(!logger.isInitialized(), "Logger should start deinitialized"); + + logger.deinit(); + expect_true(!logger.isInitialized(), "deinit should be safe before init"); + expect_true(logger.getAllLogs().empty(), "Pre-init deinit should leave log storage empty"); +} + +void test_deinit_is_idempotent() { + ESPLogger logger; + LoggerConfig config; + config.enableSyncTask = false; + config.maxLogInRam = 4; + config.consoleLogLevel = LogLevel::Debug; + + if (!logger.init(config)) { + fail("ESPLogger failed to initialize"); + } + + logger.info("TEST", "first"); + logger.deinit(); + expect_true(!logger.isInitialized(), "Logger should be deinitialized after first deinit"); + + logger.deinit(); + expect_true(!logger.isInitialized(), "Logger should remain deinitialized after second deinit"); + expect_true(logger.getAllLogs().empty(), "Idempotent deinit should not leave buffered logs"); +} + +void test_reinit_after_deinit() { + ESPLogger logger; + + LoggerConfig firstConfig; + firstConfig.enableSyncTask = false; + firstConfig.maxLogInRam = 3; + firstConfig.consoleLogLevel = LogLevel::Warn; + + if (!logger.init(firstConfig)) { + fail("First init failed"); + } + logger.warn("REINIT", "one"); + logger.deinit(); + expect_true(!logger.isInitialized(), "Logger should be deinitialized before second init"); + + LoggerConfig secondConfig = firstConfig; + secondConfig.maxLogInRam = 8; + secondConfig.consoleLogLevel = LogLevel::Error; + + if (!logger.init(secondConfig)) { + fail("Second init failed"); + } + + expect_true(logger.isInitialized(), "Logger should reinitialize successfully"); + expect_true(logger.getAllLogs().empty(), "Reinit should start with an empty log buffer"); + expect_equal(logger.currentConfig().maxLogInRam, static_cast(8), "Second init config should be applied"); + expect_equal(logger.logLevel(), LogLevel::Error, "Second init log level should be applied"); + + logger.deinit(); +} + void test_stores_logs_up_to_configured_capacity() { test_support::resetMillis(); @@ -135,6 +195,50 @@ void test_sync_callback_receives_buffered_logs() { logger.deinit(); } +void test_deinit_flushes_and_clears_callbacks() { + test_support::resetMillis(); + + ESPLogger logger; + LoggerConfig config; + config.enableSyncTask = false; + config.maxLogInRam = 8; + config.consoleLogLevel = LogLevel::Debug; + + if (!logger.init(config)) { + fail("ESPLogger failed to initialize"); + } + + size_t liveCallCount = 0; + size_t syncCallCount = 0; + size_t lastSyncedBatchSize = 0; + + logger.attach([&liveCallCount](const Log &) { ++liveCallCount; }); + logger.onSync([&syncCallCount, &lastSyncedBatchSize](const std::vector &logs) { + ++syncCallCount; + lastSyncedBatchSize = logs.size(); + }); + + logger.info("TEARDOWN", "before deinit"); + expect_equal(liveCallCount, static_cast(1), "Live callback should run before deinit"); + + logger.deinit(); + expect_true(!logger.isInitialized(), "Logger should be deinitialized"); + expect_equal(syncCallCount, static_cast(1), "deinit should flush buffered logs exactly once"); + expect_equal(lastSyncedBatchSize, static_cast(1), "deinit flush should include pending log entries"); + + if (!logger.init(config)) { + fail("ESPLogger failed to reinitialize"); + } + + logger.info("TEARDOWN", "after deinit"); + logger.sync(); + + expect_equal(liveCallCount, static_cast(1), "Live callback should be cleared by deinit"); + expect_equal(syncCallCount, static_cast(1), "Sync callback should be cleared by deinit"); + + logger.deinit(); +} + void test_live_callback_receives_logs_immediately() { test_support::resetMillis(); @@ -324,14 +428,41 @@ void test_static_helpers_on_snapshot() { expect_equal(warnLogs.front().message, std::string("c"), "Static getLogs should preserve message order"); } +void test_destructor_calls_deinit_and_flushes_pending_logs() { + test_support::resetMillis(); + + std::vector flushedLogs; + { + ESPLogger logger; + LoggerConfig config; + config.enableSyncTask = false; + config.maxLogInRam = 4; + config.consoleLogLevel = LogLevel::Debug; + + if (!logger.init(config)) { + fail("ESPLogger failed to initialize"); + } + + logger.onSync([&flushedLogs](const std::vector &logs) { flushedLogs = logs; }); + logger.info("DTOR", "pending"); + } + + expect_equal(flushedLogs.size(), static_cast(1), "Destructor should deinit and flush pending logs"); + expect_equal(flushedLogs.front().message, std::string("pending"), "Destructor flush should preserve message"); +} + } // namespace int main() { try { test_init_with_default_config(); test_init_applies_normalized_config(); + test_deinit_is_safe_before_init(); + test_deinit_is_idempotent(); + test_reinit_after_deinit(); test_stores_logs_up_to_configured_capacity(); test_sync_callback_receives_buffered_logs(); + test_deinit_flushes_and_clears_callbacks(); test_live_callback_receives_logs_immediately(); test_detach_disables_live_callback(); test_live_and_sync_callbacks_can_be_used_together(); @@ -339,6 +470,7 @@ int main() { test_multiple_logger_instances_operate_independently(); test_get_logs_by_level(); test_static_helpers_on_snapshot(); + test_destructor_calls_deinit_and_flushes_pending_logs(); } catch (const std::exception &ex) { std::cerr << "Test failure: " << ex.what() << '\n'; return 1;