From 8e837e173d3dfa7fce70c66dab7d0faf8b6e6957 Mon Sep 17 00:00:00 2001 From: Francisco Paisana Date: Tue, 24 Mar 2020 17:57:15 +0000 Subject: [PATCH] printf has already a way to pad spaces. If we use it, we can avoid accidentally adding extra logs because we did srslte::logmap::get("MAC") instead of srslte::logmap::get("MAC ") --- lib/include/srslte/common/logmap.h | 2 -- lib/include/srslte/common/test_common.h | 8 ++++---- lib/src/common/log_filter.cc | 7 ++++++- lib/src/common/logmap.cc | 7 ++++--- lib/test/common/log_filter_test.cc | 16 ++++++++++++++++ srsue/hdr/stack/ue_stack_lte.h | 22 +++++++++++----------- srsue/src/stack/ue_stack_lte.cc | 12 ++---------- 7 files changed, 43 insertions(+), 31 deletions(-) diff --git a/lib/include/srslte/common/logmap.h b/lib/include/srslte/common/logmap.h index dc88e4cb0..4ddc50df4 100644 --- a/lib/include/srslte/common/logmap.h +++ b/lib/include/srslte/common/logmap.h @@ -60,8 +60,6 @@ private: class logmap : public singleton_t { - static const size_t servicename_minimum_length = 4; - public: // Access to log map by servicename. If servicename does not exist, create a new log_filter with default cfg // Access to the map is protected by a mutex diff --git a/lib/include/srslte/common/test_common.h b/lib/include/srslte/common/test_common.h index 5b3ac0269..c6ca5d569 100644 --- a/lib/include/srslte/common/test_common.h +++ b/lib/include/srslte/common/test_common.h @@ -159,16 +159,16 @@ public: { std::unique_ptr l{new Log{std::forward(args)...}}; // store previous log, and register the newly created one - prev_log = srslte::logmap::get_instance()->deregister_log(l->get_service_name()); + prev_log = srslte::logmap::deregister_log(l->get_service_name()); current_log = l.get(); - srslte::logmap::get_instance()->register_log(std::move(l)); + srslte::logmap::register_log(std::move(l)); } scoped_log(scoped_log&&) noexcept = default; ~scoped_log() { - srslte::logmap::get_instance()->deregister_log(current_log->get_service_name()); + srslte::logmap::deregister_log(current_log->get_service_name()); if (prev_log != nullptr) { - srslte::logmap::get_instance()->register_log(std::move(prev_log)); + srslte::logmap::register_log(std::move(prev_log)); } } diff --git a/lib/src/common/log_filter.cc b/lib/src/common/log_filter.cc index 30ace57bb..e83b2f24a 100644 --- a/lib/src/common/log_filter.cc +++ b/lib/src/common/log_filter.cc @@ -65,6 +65,11 @@ log_filter::log_filter(std::string layer, logger* logger_, bool tti) : log() void log_filter::init(std::string layer, logger* logger_, bool tti) { + // strip trailing white spaces + size_t last_char_pos = layer.find_last_not_of(' '); + if (last_char_pos != layer.size() - 1) { + layer.erase(last_char_pos + 1, layer.size()); + } service_name = std::move(layer); logger_h = logger_; do_tti = tti; @@ -99,7 +104,7 @@ void log_filter::all_log(srslte::LOG_LEVEL_ENUM level, snprintf(log_str->str(), log_str->get_buffer_size(), - "%s [%s] %s %s%s%s%s%s", + "%s [%-4s] %s %s%s%s%s%s", buffer_time, get_service_name().c_str(), log_level_text_short[level], diff --git a/lib/src/common/logmap.cc b/lib/src/common/logmap.cc index 1a3e5131b..617ff67f7 100644 --- a/lib/src/common/logmap.cc +++ b/lib/src/common/logmap.cc @@ -40,9 +40,10 @@ logmap::logmap() : logger_stdout_val(new logger_stdout{}) log_ref SRSLTE_EXPORT logmap::get(std::string servicename) { logmap* pool = get_instance(); - if (servicename.size() < servicename_minimum_length) { - // pad right of string with spaces - servicename.insert(servicename.end(), servicename_minimum_length - servicename.length(), ' '); + // strip trailing white spaces + size_t last_char_pos = servicename.find_last_not_of(' '); + if (last_char_pos != servicename.size() - 1) { + servicename.erase(last_char_pos + 1, servicename.size()); } return pool->get_impl(std::move(servicename)); } diff --git a/lib/test/common/log_filter_test.cc b/lib/test/common/log_filter_test.cc index 14af41922..27ac75672 100644 --- a/lib/test/common/log_filter_test.cc +++ b/lib/test/common/log_filter_test.cc @@ -166,6 +166,21 @@ int test_log_singleton() return SRSLTE_SUCCESS; } +int test_log_ref() +{ + // Check if trailing whitespaces are correctly removed + srslte::log_ref t1_log{"T1"}; + TESTASSERT(t1_log->get_service_name() == "T1"); + TESTASSERT(t1_log.get() == srslte::logmap::get("T1").get()); + TESTASSERT(t1_log.get() == srslte::logmap::get("T1 ").get()); + { + scoped_log null_log{"T2"}; + TESTASSERT(null_log->get_service_name() == "T2"); + } + + return SRSLTE_SUCCESS; +} + int full_test() { bool result; @@ -191,6 +206,7 @@ int main(int argc, char** argv) TESTASSERT(basic_hex_test() == SRSLTE_SUCCESS); TESTASSERT(full_test() == SRSLTE_SUCCESS); TESTASSERT(test_log_singleton() == SRSLTE_SUCCESS); + TESTASSERT(test_log_ref() == SRSLTE_SUCCESS); return SRSLTE_SUCCESS; } diff --git a/srsue/hdr/stack/ue_stack_lte.h b/srsue/hdr/stack/ue_stack_lte.h index 573d6e8e8..f84d5cf60 100644 --- a/srsue/hdr/stack/ue_stack_lte.h +++ b/srsue/hdr/stack/ue_stack_lte.h @@ -139,24 +139,24 @@ private: const std::chrono::milliseconds TTI_WARN_THRESHOLD_MS{5}; const uint32_t SYNC_QUEUE_WARN_THRESHOLD = 5; - bool running; - srsue::stack_args_t args; + bool running; + srsue::stack_args_t args; - srslte::tti_point current_tti; + srslte::tti_point current_tti; // timers srslte::timer_handler timers; // UE stack logging srslte::logger* logger = nullptr; - srslte::log_ref stack_log; ///< our own log filter - srslte::log_ref mac_log; - srslte::log_ref rlc_log; - srslte::log_ref pdcp_log; - srslte::log_ref rrc_log; - srslte::log_ref usim_log; - srslte::log_ref nas_log; - srslte::log_ref pool_log; + srslte::log_ref stack_log{"STCK"}; ///< our own log filter + srslte::log_ref mac_log{"MAC"}; + srslte::log_ref rlc_log{"RLC"}; + srslte::log_ref pdcp_log{"PDCP"}; + srslte::log_ref rrc_log{"RRC"}; + srslte::log_ref usim_log{"USIM"}; + srslte::log_ref nas_log{"NAS"}; + srslte::log_ref pool_log{"POOL"}; // stack components srsue::mac mac; diff --git a/srsue/src/stack/ue_stack_lte.cc b/srsue/src/stack/ue_stack_lte.cc index 0cb7419a3..4ed2351c9 100644 --- a/srsue/src/stack/ue_stack_lte.cc +++ b/srsue/src/stack/ue_stack_lte.cc @@ -35,14 +35,6 @@ ue_stack_lte::ue_stack_lte() : running(false), args(), logger(nullptr), - stack_log("STCK"), - pool_log("POOL"), - mac_log("MAC "), - rlc_log("RLC "), - pdcp_log("PDCP"), - rrc_log("RRC "), - usim_log("USIM"), - nas_log("NAS "), usim(nullptr), phy(nullptr), rlc(rlc_log.get()), @@ -96,11 +88,11 @@ int ue_stack_lte::init(const stack_args_t& args_, srslte::logger* logger_) // init own log stack_log->set_level(srslte::LOG_LEVEL_INFO); - pool_log->set_level(srslte::LOG_LEVEL_ERROR); + pool_log->set_level(srslte::LOG_LEVEL_WARNING); byte_buffer_pool::get_instance()->set_log(pool_log.get()); // init layer logs - srslte::logmap::register_log(std::unique_ptr{new srslte::log_filter{"MAC ", logger, true}}); + srslte::logmap::register_log(std::unique_ptr{new srslte::log_filter{"MAC", logger, true}}); mac_log->set_level(args.log.mac_level); mac_log->set_hex_limit(args.log.mac_hex_limit); rlc_log->set_level(args.log.rlc_level);