feat(logger): make the tag's mutability a compile-time option (off on ESP-IDF)

components/base_component/include/base_component.hpp

@@ -2,6 +2,8 @@
 
 #include <chrono>
 #include <string>
+#include <string_view>
+#include <type_traits>
 
 #include "logger.hpp"
 
@@ -15,9 +17,23 @@ class BaseComponent {
   /// \note This is the tag of the logger
   const std::string &get_name() const { return logger_.get_tag(); }
 
+#if ESPP_LOGGER_MUTABLE_TAG
   /// Set the tag for the logger
   /// \param tag The tag to use for the logger
+  /// \note Only with ESPP_LOGGER_MUTABLE_TAG (on ESP-IDF the
+  ///       CONFIG_ESPP_LOGGER_MUTABLE_TAG Kconfig option); otherwise pass
+  ///       the tag to the constructor.
   void set_log_tag(const std::string_view &tag) { logger_.set_tag(tag); }
+#else
+  /// Not available: the tag is fixed at construction (pass it to the
+  /// constructor). Enable CONFIG_ESPP_LOGGER_MUTABLE_TAG (menuconfig: ESPP
+  /// Logger -> Mutable tag) to change it afterwards.
+  template <typename T = void> void set_log_tag(const std::string_view &) {
+    static_assert(!std::is_same_v<T, T>,
+                  "BaseComponent::set_log_tag() needs CONFIG_ESPP_LOGGER_MUTABLE_TAG "
+                  "(menuconfig: ESPP Logger -> Mutable tag); or pass the tag to the constructor");
+  }
+#endif
 
   /// Get the log level for the logger
   /// \return The verbosity level of the logger

components/iperf_menu/example/CMakeLists.txt

@@ -6,6 +6,7 @@ include($ENV{IDF_PATH}/tools/cmake/project.cmake)
 
 # add the component directories that we want to use
 set(EXTRA_COMPONENT_DIRS
+  "../../../components/base_component"
   "../../../components/cli"
   "../../../components/format"
   "../../../components/iperf_menu"

components/logger/Kconfig

@@ -22,4 +22,16 @@ menu "ESPP Logger"
           Enable cursor commands for the ESPP Logger. This will allow the user to
           move the cursor around the terminal and clear the screen.
 
+    config ESPP_LOGGER_MUTABLE_TAG
+        bool "Mutable tag (Logger::set_tag(), BaseComponent::set_log_tag())"
+        default n
+        help
+          Allow a Logger's tag to change after construction, with
+          Logger::set_tag() / BaseComponent::set_log_tag(). The tag then needs
+          a mutex, and on ESP-IDF each mutex allocates a FreeRTOS semaphore:
+          about 116 bytes of internal RAM for every Logger (every espp
+          component has one). Off, the tag is set once, in the Logger's
+          Config (or BaseComponent's constructor), and set_tag() /
+          set_log_tag() do not compile.
+
 endmenu

components/logger/include/logger.hpp

@@ -5,6 +5,7 @@
 #include <mutex>
 #include <string>
 #include <string_view>
+#include <type_traits>
 
 #if defined(ESP_PLATFORM)
 // include sdkconfig before the rest
@@ -69,6 +70,21 @@ class Logger {
 #define ESPP_LOGGER_WARN_ENABLED (CONFIG_ESPP_LOGGER_LOG_LEVEL >= ESPP_LOGGER_LOG_LEVEL_WARN)
 #define ESPP_LOGGER_ERROR_ENABLED (CONFIG_ESPP_LOGGER_LOG_LEVEL >= ESPP_LOGGER_LOG_LEVEL_ERROR)
 
+// Whether a Logger's tag can change after construction (set_tag()). Changing
+// it needs a mutex, which on ESP-IDF allocates a FreeRTOS semaphore (about
+// 116 B of internal RAM) for every Logger, so there it is the
+// CONFIG_ESPP_LOGGER_MUTABLE_TAG Kconfig option, off by default. Elsewhere
+// (host / python builds) it defaults on; define ESPP_LOGGER_MUTABLE_TAG to 0 or
+// 1 to choose.
+#ifndef ESPP_LOGGER_MUTABLE_TAG
+#if (defined(CONFIG_ESPP_LOGGER_MUTABLE_TAG) && CONFIG_ESPP_LOGGER_MUTABLE_TAG) ||                 \
+    !defined(ESP_PLATFORM)
+#define ESPP_LOGGER_MUTABLE_TAG 1
+#else
+#define ESPP_LOGGER_MUTABLE_TAG 0
+#endif
+#endif
+
 // If the cursor commands haven't been configured as a define, check the config
 #if !defined(ESPP_LOGGER_CURSOR_COMMANDS_ENABLED)
 #if !defined(CONFIG_ESPP_LOGGER_ENABLE_CURSOR_COMMANDS)
@@ -134,7 +150,7 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
    */
   Logger(Logger &&other) noexcept // cppcheck-suppress missingMemberCopy
       : tag_([&other] {
-        std::scoped_lock lock(other.tag_mutex_);
+        [[maybe_unused]] auto lock = other.lock_tag();
         return std::move(other.tag_);
       }())
       , rate_limit_(other.rate_limit_)
@@ -151,7 +167,9 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
    */
   Logger &operator=(const Logger &other) { // cppcheck-suppress operatorEqVarError
     if (this != &other) {
+#if ESPP_LOGGER_MUTABLE_TAG
       std::scoped_lock lock(tag_mutex_, other.tag_mutex_);
+#endif
       tag_ = other.tag_;
       rate_limit_ = other.rate_limit_;
       include_time_ = other.include_time_.load();
@@ -167,7 +185,9 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
    */
   Logger &operator=(Logger &&other) noexcept {
     if (this != &other) {
+#if ESPP_LOGGER_MUTABLE_TAG
       std::scoped_lock lock(tag_mutex_, other.tag_mutex_);
+#endif
       tag_ = std::move(other.tag_);
       rate_limit_ = other.rate_limit_;
       last_print_.store(other.last_print_.load());
@@ -191,14 +211,30 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
    */
   void set_verbosity(const espp::Logger::Verbosity level) { level_ = level; }
 
+#if ESPP_LOGGER_MUTABLE_TAG
   /**
    * @brief Change the tag for the logger.
    * @param tag The new tag.
+   * @note Only with ESPP_LOGGER_MUTABLE_TAG (on ESP-IDF the
+   *       CONFIG_ESPP_LOGGER_MUTABLE_TAG Kconfig option); otherwise set the
+   *       tag in the Config.
    */
   void set_tag(const std::string_view tag) {
     std::scoped_lock lock(tag_mutex_);
     tag_ = tag;
   }
+#else
+  /**
+   * @brief Not available: the tag is fixed at construction (set it in the
+   *        Config). Enable CONFIG_ESPP_LOGGER_MUTABLE_TAG (menuconfig: ESPP
+   *        Logger -> Mutable tag) to change it afterwards.
+   */
+  template <typename T = void> void set_tag(const std::string_view) {
+    static_assert(!std::is_same_v<T, T>,
+                  "Logger::set_tag() needs CONFIG_ESPP_LOGGER_MUTABLE_TAG (menuconfig: ESPP "
+                  "Logger -> Mutable tag); or set the tag in the Logger's Config");
+  }
+#endif
 
   /**
    * @brief Get the current tag for the logger.
@@ -255,10 +291,10 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
       return;
     auto msg = format(rt_fmt_str, std::forward<Args>(args)...);
     if (include_time_) {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::color::gray), "[{}/D][{}]: {}\n", tag_, get_time(), msg);
     } else {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::color::gray), "[{}/D]:{}\n", tag_, msg);
     }
 #endif
@@ -275,10 +311,10 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
       return;
     auto msg = format(rt_fmt_str, std::forward<Args>(args)...);
     if (include_time_) {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::terminal_color::green), "[{}/I][{}]: {}\n", tag_, get_time(), msg);
     } else {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::terminal_color::green), "[{}/I]:{}\n", tag_, msg);
     }
 #endif
@@ -295,10 +331,10 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
       return;
     auto msg = format(rt_fmt_str, std::forward<Args>(args)...);
     if (include_time_) {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::terminal_color::yellow), "[{}/W][{}]: {}\n", tag_, get_time(), msg);
     } else {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::terminal_color::yellow), "[{}/W]:{}\n", tag_, msg);
     }
 #endif
@@ -315,10 +351,10 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
       return;
     auto msg = format(rt_fmt_str, std::forward<Args>(args)...);
     if (include_time_) {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::terminal_color::red), "[{}/E][{}]: {}\n", tag_, get_time(), msg);
     } else {
-      std::scoped_lock lock(tag_mutex_);
+      [[maybe_unused]] auto lock = lock_tag();
       fmt::print(fg(fmt::terminal_color::red), "[{}/E]:{}\n", tag_, msg);
     }
 #endif
@@ -492,8 +528,18 @@ rate limit. @note Only calls that have _rate_limited suffixed will be rate limit
    */
   static std::chrono::steady_clock::time_point start_time_;
 
+#if ESPP_LOGGER_MUTABLE_TAG
   mutable std::mutex tag_mutex_; ///< Mutex for the tag.
-  std::string tag_;              ///< Name of the logger to be prepended to all logs.
+  /// Lock the tag against set_tag() while it is read.
+  [[nodiscard]] std::unique_lock<std::mutex> lock_tag() const {
+    return std::unique_lock<std::mutex>(tag_mutex_);
+  }
+#else
+  /// The tag is fixed at construction: nothing to lock.
+  struct NoTagLock {};
+  [[nodiscard]] NoTagLock lock_tag() const { return {}; }
+#endif
+  std::string tag_; ///< Name of the logger to be prepended to all logs.
   clock_t::duration rate_limit_{
       clock_t::duration::zero()}; ///< Rate limit for the logger. If zero, no rate limiting is done.
   std::atomic<clock_t::rep> last_print_{

components/motorgo-mini/include/motorgo-mini.hpp

@@ -194,8 +194,11 @@ class MotorGoMini : public BaseComponent {
       .kv_rating = 320,
       .current_limit = 1.0f,
       .foc_type = espp::detail::FocType::SPACE_VECTOR_PWM,
-      .driver = motor1_driver_, // NOTE: user cannot override this
-      .sensor = encoder1_,      // NOTE: user cannot override this
+      // set by init_motor_channel_1() (the user cannot override them); not
+      // copied from motor1_driver_ / encoder1_ here, which are declared (so
+      // constructed) after this member
+      .driver = nullptr,
+      .sensor = nullptr,
       .velocity_pid_config =
           {
               .kp = 0.010f,
@@ -227,8 +230,11 @@ class MotorGoMini : public BaseComponent {
       .kv_rating = 320,
       .current_limit = 1.0f,
       .foc_type = espp::detail::FocType::SPACE_VECTOR_PWM,
-      .driver = motor2_driver_, // NOTE: user cannot override this
-      .sensor = encoder2_,      // NOTE: user cannot override this
+      // set by init_motor_channel_2() (the user cannot override them); not
+      // copied from motor2_driver_ / encoder2_ here, which are declared (so
+      // constructed) after this member
+      .driver = nullptr,
+      .sensor = nullptr,
       .velocity_pid_config =
           {
               .kp = 0.010f,

components/persistent_data/include/persistent_data.hpp

@@ -90,11 +90,10 @@ template <persistent_data::VersionedType... Versions> class PersistentData : pub
    * @param config The configuration.
    */
   explicit PersistentData(const Config &config)
-      : BaseComponent("PersistentData", config.log_level)
+      : BaseComponent("PersistentData:" + config.file_path.filename().string(), config.log_level)
       , file_path_(config.file_path)
       , save_after_migration_(config.save_after_migration)
       , max_file_size_(config.max_file_size) {
-    set_log_tag("PersistentData:" + file_path_.filename().string());
     if (config.auto_load) {
       std::error_code ec;
       load(ec);

components/rtsp/include/rtsp_session.hpp

@@ -145,6 +145,11 @@ class RtspSession : public BaseComponent {
   bool send_rtcp_packet(const espp::RtcpPacket &packet);
 
 protected:
+  /// Construct with a given session id (it is part of the logger tag, which
+  /// is set before the members are)
+  RtspSession(std::shared_ptr<espp::TcpSocket> control_socket,
+              const espp::RtspSession::Config &config, uint32_t session_id);
+
   /// Send a response to a RTSP request
   /// @param code The response code
   /// @param message The response message
@@ -216,7 +221,7 @@ class RtspSession : public BaseComponent {
   /// Session IDs are generated randomly when a client sends a SETUP request and are
   /// used to identify the client in subsequent requests when managing the RTP session.
   /// @return The new session id
-  uint32_t generate_session_id();
+  static uint32_t generate_session_id();
 
   /// Parse the RTSP command sequence number from a request
   /// @param request The request to parse

components/rtsp/src/rtsp_session.cpp

@@ -38,15 +38,18 @@ bool parse_decimal_int(std::string_view text, int &value) {
 } // namespace
 
 RtspSession::RtspSession(std::shared_ptr<TcpSocket> control_socket, const Config &config)
-    : BaseComponent("RtspSession", config.log_level)
+    : RtspSession(std::move(control_socket), config, generate_session_id()) {}
+
+RtspSession::RtspSession(std::shared_ptr<TcpSocket> control_socket, const Config &config,
+                         uint32_t session_id)
+    // the logger tag includes the session id
+    : BaseComponent("RtspSession " + std::to_string(session_id), config.log_level)
     , control_socket_(std::move(control_socket))
-    , session_id_(generate_session_id())
+    , session_id_(session_id)
     , server_address_(config.server_address)
     , rtsp_path_(config.rtsp_path)
     , client_address_(control_socket_->get_remote_info().address)
     , sdp_generator_(config.sdp_generator) {
-  // set the logger tag to include the session id
-  logger_.set_tag("RtspSession " + std::to_string(session_id_));
   // ensure there is a timeout on the control socket receive
   control_socket_->set_receive_timeout(config.receive_timeout);
   // start the session task to handle RTSP commands