From 88aec010488506819826ae9764082c07e30b6998 Mon Sep 17 00:00:00 2001 From: Peter Barker Date: Wed, 3 Nov 2021 12:34:52 +1100 Subject: [PATCH] AP_Logger: tidy construction of backends --- libraries/AP_Logger/AP_Logger.cpp | 95 ++++++----------------- libraries/AP_Logger/AP_Logger_DataFlash.h | 4 + libraries/AP_Logger/AP_Logger_File.cpp | 5 +- libraries/AP_Logger/AP_Logger_File.h | 8 +- libraries/AP_Logger/AP_Logger_MAVLink.h | 5 ++ libraries/AP_Logger/AP_Logger_SITL.h | 6 ++ 6 files changed, 47 insertions(+), 76 deletions(-) diff --git a/libraries/AP_Logger/AP_Logger.cpp b/libraries/AP_Logger/AP_Logger.cpp index a1b3e338f4600..27ff5ada5f771 100644 --- a/libraries/AP_Logger/AP_Logger.cpp +++ b/libraries/AP_Logger/AP_Logger.cpp @@ -172,95 +172,48 @@ void AP_Logger::Init(const struct LogStructure *structures, uint8_t num_types) validate_structures(structures, num_types); dump_structures(structures, num_types); #endif - if (_next_backend == LOGGER_MAX_BACKENDS) { - AP_HAL::panic("Too many backends"); - return; - } _num_types = num_types; _structures = structures; + // the "main" logging type needs to come before mavlink so that + // index 0 is correct + static const struct { + Backend_Type type; + AP_Logger_Backend* (*probe_fn)(AP_Logger&, LoggerMessageWriter_DFLogStart*); + } backend_configs[] { #if HAL_LOGGING_FILESYSTEM_ENABLED - if (_params.backend_types & uint8_t(Backend_Type::FILESYSTEM)) { - LoggerMessageWriter_DFLogStart *message_writer = - new LoggerMessageWriter_DFLogStart(); - if (message_writer != nullptr) { - backends[_next_backend] = new AP_Logger_File(*this, - message_writer, - HAL_BOARD_LOG_DIRECTORY); - } - if (backends[_next_backend] == nullptr) { - hal.console->printf("Unable to open AP_Logger_File"); - // note that message_writer is leaked here; costs several - // hundred bytes to fix for marginal utility - } else { - _next_backend++; - } - } -#endif // HAL_LOGGING_FILESYSTEM_ENABLED - + { Backend_Type::FILESYSTEM, AP_Logger_File::probe }, +#endif #if HAL_LOGGING_DATAFLASH_ENABLED - if (_params.backend_types & uint8_t(Backend_Type::BLOCK)) { - if (_next_backend == LOGGER_MAX_BACKENDS) { - AP_HAL::panic("Too many backends"); - return; - } - LoggerMessageWriter_DFLogStart *message_writer = - new LoggerMessageWriter_DFLogStart(); - if (message_writer != nullptr) { - backends[_next_backend] = new AP_Logger_DataFlash(*this, message_writer); - } - if (backends[_next_backend] == nullptr) { - hal.console->printf("Unable to open AP_Logger_DataFlash"); - // note that message_writer is leaked here; costs several - // hundred bytes to fix for marginal utility - } else { - _next_backend++; - } - } + { Backend_Type::BLOCK, AP_Logger_DataFlash::probe }, #endif - #if HAL_LOGGING_SITL_ENABLED - if (_params.backend_types & uint8_t(Backend_Type::BLOCK)) { - if (_next_backend == LOGGER_MAX_BACKENDS) { - AP_HAL::panic("Too many backends"); - return; - } - LoggerMessageWriter_DFLogStart *message_writer = - new LoggerMessageWriter_DFLogStart(); - if (message_writer != nullptr) { - backends[_next_backend] = new AP_Logger_SITL(*this, message_writer); - } - if (backends[_next_backend] == nullptr) { - hal.console->printf("Unable to open AP_Logger_SITL"); - // note that message_writer is leaked here; costs several - // hundred bytes to fix for marginal utility - } else { - _next_backend++; - } - } + { Backend_Type::BLOCK, AP_Logger_SITL::probe }, #endif - // the "main" logging type needs to come before mavlink so that index 0 is correct #if HAL_LOGGING_MAVLINK_ENABLED - if (_params.backend_types & uint8_t(Backend_Type::MAVLINK)) { + { Backend_Type::MAVLINK, AP_Logger_MAVLink::probe }, +#endif +}; + + for (const auto &backend_config : backend_configs) { + if ((_params.backend_types & uint8_t(backend_config.type)) == 0) { + continue; + } if (_next_backend == LOGGER_MAX_BACKENDS) { - AP_HAL::panic("Too many backends"); + AP_BoardConfig::config_error("Too many backends"); return; } LoggerMessageWriter_DFLogStart *message_writer = new LoggerMessageWriter_DFLogStart(); - if (message_writer != nullptr) { - backends[_next_backend] = new AP_Logger_MAVLink(*this, - message_writer); + if (message_writer == nullptr) { + AP_BoardConfig::allocation_error("mesage writer"); } + backends[_next_backend] = backend_config.probe_fn(*this, message_writer); if (backends[_next_backend] == nullptr) { - hal.console->printf("Unable to open AP_Logger_MAVLink"); - // note that message_writer is leaked here; costs several - // hundred bytes to fix for marginal utility - } else { - _next_backend++; + AP_BoardConfig::allocation_error("logger backend"); } + _next_backend++; } -#endif for (uint8_t i=0; i<_next_backend; i++) { backends[i]->Init(); diff --git a/libraries/AP_Logger/AP_Logger_DataFlash.h b/libraries/AP_Logger/AP_Logger_DataFlash.h index 3fe85ba4430af..ff702b0974ffb 100644 --- a/libraries/AP_Logger/AP_Logger_DataFlash.h +++ b/libraries/AP_Logger/AP_Logger_DataFlash.h @@ -13,6 +13,10 @@ class AP_Logger_DataFlash : public AP_Logger_Block { public: AP_Logger_DataFlash(AP_Logger &front, LoggerMessageWriter_DFLogStart *writer) : AP_Logger_Block(front, writer) {} + static AP_Logger_Backend *probe(AP_Logger &front, + LoggerMessageWriter_DFLogStart *ls) { + return new AP_Logger_DataFlash(front, ls); + } void Init(void) override; bool CardInserted() const override { return !flash_died && df_NumPages > 0; } diff --git a/libraries/AP_Logger/AP_Logger_File.cpp b/libraries/AP_Logger/AP_Logger_File.cpp index 4b5a43b3836f5..cf2f8bb5a04c2 100644 --- a/libraries/AP_Logger/AP_Logger_File.cpp +++ b/libraries/AP_Logger/AP_Logger_File.cpp @@ -40,10 +40,9 @@ extern const AP_HAL::HAL& hal; constructor */ AP_Logger_File::AP_Logger_File(AP_Logger &front, - LoggerMessageWriter_DFLogStart *writer, - const char *log_directory) : + LoggerMessageWriter_DFLogStart *writer) : AP_Logger_Backend(front, writer), - _log_directory(log_directory) + _log_directory(HAL_BOARD_LOG_DIRECTORY) { df_stats_clear(); } diff --git a/libraries/AP_Logger/AP_Logger_File.h b/libraries/AP_Logger/AP_Logger_File.h index 499fe80ba0cec..5f76f4fb92592 100644 --- a/libraries/AP_Logger/AP_Logger_File.h +++ b/libraries/AP_Logger/AP_Logger_File.h @@ -22,8 +22,12 @@ class AP_Logger_File : public AP_Logger_Backend public: // constructor AP_Logger_File(AP_Logger &front, - LoggerMessageWriter_DFLogStart *, - const char *log_directory); + LoggerMessageWriter_DFLogStart *); + + static AP_Logger_Backend *probe(AP_Logger &front, + LoggerMessageWriter_DFLogStart *ls) { + return new AP_Logger_File(front, ls); + } // initialisation void Init() override; diff --git a/libraries/AP_Logger/AP_Logger_MAVLink.h b/libraries/AP_Logger/AP_Logger_MAVLink.h index edd60cc8bc797..f379f121f276e 100644 --- a/libraries/AP_Logger/AP_Logger_MAVLink.h +++ b/libraries/AP_Logger/AP_Logger_MAVLink.h @@ -27,6 +27,11 @@ class AP_Logger_MAVLink : public AP_Logger_Backend // ::fprintf(stderr, "DM: Using %u blocks\n", _blockcount); } + static AP_Logger_Backend *probe(AP_Logger &front, + LoggerMessageWriter_DFLogStart *ls) { + return new AP_Logger_MAVLink(front, ls); + } + // initialisation void Init() override; diff --git a/libraries/AP_Logger/AP_Logger_SITL.h b/libraries/AP_Logger/AP_Logger_SITL.h index b5a7060ebe1ce..c9c6145d39905 100644 --- a/libraries/AP_Logger/AP_Logger_SITL.h +++ b/libraries/AP_Logger/AP_Logger_SITL.h @@ -14,6 +14,12 @@ class AP_Logger_SITL : public AP_Logger_Block { public: AP_Logger_SITL(AP_Logger &front, LoggerMessageWriter_DFLogStart *writer) : AP_Logger_Block(front, writer) {} + + static AP_Logger_Backend *probe(AP_Logger &front, + LoggerMessageWriter_DFLogStart *ls) { + return new AP_Logger_SITL(front, ls); + } + void Init() override; bool CardInserted() const override; static constexpr const char *filename = "dataflash.bin";