From bddece0970ea385ef563fe06ca24eaec42f311c2 Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Mon, 10 Feb 2025 17:28:29 -0700 Subject: [PATCH] :sparkles: Allow overriding the mipi message type Problem: - Mipi short messages do not contain anything except the string id: no module id, no severity, etc. A client may want this information, so may not want to automatically use the short message. Solution: - Allow the logging environment to supply a message builder for mipi. --- CMakeLists.txt | 4 +- include/log/catalog/mipi_builder.hpp | 116 +++++++++++++++++++++ include/log/catalog/mipi_encoder.hpp | 139 +++++--------------------- include/log/catalog/mipi_messages.hpp | 67 +++++++++++++ test/log/mipi_encoder.cpp | 60 ++++++++--- 5 files changed, 253 insertions(+), 133 deletions(-) create mode 100644 include/log/catalog/mipi_builder.hpp create mode 100644 include/log/catalog/mipi_messages.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 732a5fbb..772af74f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -207,7 +207,9 @@ target_sources( include FILES include/log/catalog/catalog.hpp - include/log/catalog/mipi_encoder.hpp) + include/log/catalog/mipi_builder.hpp + include/log/catalog/mipi_encoder.hpp + include/log/catalog/mipi_messages.hpp) add_library(cib_nexus INTERFACE) target_compile_features(cib_nexus INTERFACE cxx_std_20) diff --git a/include/log/catalog/mipi_builder.hpp b/include/log/catalog/mipi_builder.hpp new file mode 100644 index 00000000..75f4f05f --- /dev/null +++ b/include/log/catalog/mipi_builder.hpp @@ -0,0 +1,116 @@ +#pragma once + +#include +#include + +#include +#include + +#include +#include +#include +#include +#include + +namespace logging::mipi { +template struct builder; + +template <> struct builder { + template ... Ts> + ALWAYS_INLINE static auto build(string_id id, module_id, Ts...) { + using namespace msg; + return owning{"payload"_field = id}; + } +}; + +template struct catalog_builder { + template ... Ts> + ALWAYS_INLINE static auto build(string_id id, module_id m, Ts... msg_data) { + using namespace msg; + defn::catalog_msg_t::owner_t message{"severity"_field = Level, + "module_id"_field = m}; + + constexpr auto header_size = + defn::catalog_msg_t::size::value; + constexpr auto copy_arg = [](std::uint32_t arg, auto &dest) { + std::memcpy(dest, &arg, sizeof(std::uint32_t)); + dest += sizeof(std::uint32_t); + }; + auto dest = &message.data()[header_size]; + copy_arg(stdx::to_le(id), dest); + (copy_arg(stdx::to_le(msg_data), dest), ...); + + return message; + } +}; + +template + requires std::same_as +struct catalog_builder { + template ... Ts> + ALWAYS_INLINE static auto build(string_id id, module_id m, Ts... msg_data) { + using namespace msg; + defn::catalog_msg_t::owner_t message{"severity"_field = Level, + "module_id"_field = m}; + + constexpr auto header_size = + defn::catalog_msg_t::size::value; + auto dest = &message.data()[header_size]; + *dest++ = stdx::to_le(id); + ((*dest++ = stdx::to_le(msg_data)), ...); + + return message; + } +}; + +template <> struct builder { + template ... Ts> + ALWAYS_INLINE static auto build(string_id id, module_id m, Ts... msg_data) { + using namespace msg; + if constexpr (sizeof...(Ts) <= 2u) { + constexpr auto header_size = + defn::catalog_msg_t::size::value; + constexpr auto payload_len = 1 + sizeof...(Ts); + using storage_t = + std::array; + return catalog_builder{}.template build( + id, m, msg_data...); + } else { + constexpr auto header_size = + defn::catalog_msg_t::size::value; + constexpr auto payload_len = (sizeof(id) + ... + sizeof(Ts)); + using storage_t = + std::array; + return catalog_builder{}.template build( + id, m, msg_data...); + } + } +}; + +struct default_builder { + template ... Ts> + ALWAYS_INLINE static auto build(string_id id, module_id m, Ts... msg_data) { + if constexpr (sizeof...(Ts) == 0u) { + return builder{}.template build( + id, m, msg_data...); + } else { + return builder{}.template build( + id, m, msg_data...); + } + } +}; + +[[maybe_unused]] constexpr inline struct get_builder_t { + template + requires true + CONSTEVAL auto operator()(T &&t) const noexcept( + noexcept(std::forward(t).query(std::declval()))) + -> decltype(std::forward(t).query(*this)) { + return std::forward(t).query(*this); + } + + CONSTEVAL auto operator()(auto &&) const { + return stdx::ct(); + } +} get_builder; +} // namespace logging::mipi diff --git a/include/log/catalog/mipi_encoder.hpp b/include/log/catalog/mipi_encoder.hpp index b29a3c4a..b95222a6 100644 --- a/include/log/catalog/mipi_encoder.hpp +++ b/include/log/catalog/mipi_encoder.hpp @@ -2,9 +2,10 @@ #include #include +#include +#include #include #include -#include #include #include @@ -37,68 +38,6 @@ template constexpr auto to_module() { } } // namespace detail -namespace defn { -using msg::at; -using msg::dword_index_t; -using msg::field; -using msg::message; -using msg::operator""_msb; -using msg::operator""_lsb; - -enum struct type : uint8_t { Build = 0, Short32 = 1, Catalog = 3 }; -enum struct build_subtype : uint8_t { Compact32 = 0, Compact64 = 1, Long = 2 }; -enum struct catalog_subtype : uint8_t { Id32_Pack32 = 1 }; - -using type_f = field<"type", type>::located; -using opt_len_f = - field<"opt_len", bool>::located; -using payload_len_f = - field<"payload_len", - std::uint16_t>::located; - -using build_subtype_f = - field<"subtype", - build_subtype>::located; -using compact32_build_id_f = field<"build_id", std::uint32_t>::located< - at{dword_index_t{0}, 31_msb, 30_lsb}, at{dword_index_t{0}, 23_msb, 4_lsb}>; -using compact64_build_id_f = field<"build_id", std::uint64_t>::located< - at{dword_index_t{1}, 31_msb, 0_lsb}, at{dword_index_t{0}, 31_msb, 30_lsb}, - at{dword_index_t{0}, 23_msb, 4_lsb}>; - -using normal_build_msg_t = - message<"normal_build", type_f::with_required, - opt_len_f::with_required, - build_subtype_f::with_required, payload_len_f>; -using compact32_build_msg_t = - message<"compact32_build", type_f::with_required, - build_subtype_f::with_required, - compact32_build_id_f>; -using compact64_build_msg_t = - message<"compact64_build", type_f::with_required, - build_subtype_f::with_required, - compact64_build_id_f>; - -using short32_payload_f = - field<"payload", - std::uint32_t>::located; -using short32_msg_t = - message<"short32", type_f::with_required, short32_payload_f>; - -using catalog_subtype_f = - field<"subtype", - catalog_subtype>::located; -using severity_f = field<"severity", std::uint8_t>::located; -using module_id_f = - field<"module_id", - std::uint8_t>::located; - -using catalog_msg_t = - message<"catalog", type_f::with_required, severity_f, - module_id_f, - catalog_subtype_f::with_required>; -} // namespace defn - template struct log_handler { constexpr explicit log_handler(TDestinations &&ds) : dests{std::move(ds)} {} @@ -116,8 +55,10 @@ template struct log_handler { using Message = decltype(detail::to_message()); using Module = decltype(detail::to_module()); - dispatch_message(catalog(), module(), - static_cast(args)...); + auto builder = get_builder(Env{}).value; + write( + builder.template build(catalog(), module(), + static_cast(args)...)); }); } @@ -126,11 +67,11 @@ template struct log_handler { if constexpr (S.empty() and stdx::bit_width(Version) <= 22) { owning message{"build_id"_field = Version}; - dispatch_pass_by_args(message.data()[0]); + write(message); } else if constexpr (S.empty() and stdx::bit_width(Version) <= 54) { owning message{"build_id"_field = Version}; - dispatch_pass_by_args(message.data()[0], message.data()[1]); + write(message); } else { constexpr auto header_size = defn::normal_build_msg_t::size::value; @@ -146,25 +87,13 @@ template struct log_handler { dest = std::copy_n(stdx::bit_cast(&ver), sizeof(std::uint64_t), dest); std::copy_n(std::cbegin(S.value), S.size(), dest); - dispatch_pass_by_buffer(message.data()); + write(message); } } private: - template - NEVER_INLINE auto - dispatch_pass_by_args(MsgDataTypes &&...msg_data) -> void { - stdx::for_each( - [&](Dest &dest) { - conc::call_in_critical_section([&] { - dest.log_by_args(std::forward(msg_data)...); - }); - }, - dests); - } - - NEVER_INLINE auto - dispatch_pass_by_buffer(stdx::span msg) -> void { + template + NEVER_INLINE auto write(stdx::span msg) -> void { stdx::for_each( [&](Dest &dest) { conc::call_in_critical_section( @@ -173,42 +102,20 @@ template struct log_handler { dests); } - template ... MsgDataTypes> - ALWAYS_INLINE auto dispatch_message(string_id id, - [[maybe_unused]] module_id m, - MsgDataTypes... msg_data) -> void { - using namespace msg; - if constexpr (sizeof...(msg_data) == 0u) { - owning message{"payload"_field = id}; - dispatch_pass_by_args(message.data()[0]); - } else if constexpr (sizeof...(MsgDataTypes) <= 2u) { - owning message{"severity"_field = Level, - "module_id"_field = m}; - dispatch_pass_by_args( - message.data()[0], stdx::to_le(id), - stdx::to_le(std::forward(msg_data))...); - } else { - constexpr auto header_size = - defn::catalog_msg_t::size::value; - constexpr auto payload_len = - (sizeof(id) + ... + sizeof(MsgDataTypes)); - using storage_t = - std::array; - - defn::catalog_msg_t::owner_t message{ - "severity"_field = Level, "module_id"_field = m}; - - constexpr auto copy_arg = [](std::uint32_t arg, auto &dest) { - std::memcpy(dest, &arg, sizeof(std::uint32_t)); - dest += sizeof(std::uint32_t); - }; - auto dest = &message.data()[header_size]; - copy_arg(stdx::to_le(id), dest); - (copy_arg(stdx::to_le(msg_data), dest), ...); - dispatch_pass_by_buffer(message.data()); - } + template + NEVER_INLINE auto write(stdx::span msg) -> void { + [&](std::index_sequence) { + stdx::for_each( + [&](Dest &dest) { + conc::call_in_critical_section( + [&] { dest.log_by_args(msg[Is]...); }); + }, + dests); + }(std::make_index_sequence{}); } + auto write(auto const &msg) -> void { write(msg.as_const_view().data()); } + TDestinations dests; }; diff --git a/include/log/catalog/mipi_messages.hpp b/include/log/catalog/mipi_messages.hpp new file mode 100644 index 00000000..b89c34b1 --- /dev/null +++ b/include/log/catalog/mipi_messages.hpp @@ -0,0 +1,67 @@ +#pragma once + +#include + +namespace logging::mipi { +namespace defn { +using msg::at; +using msg::dword_index_t; +using msg::field; +using msg::message; +using msg::operator""_msb; +using msg::operator""_lsb; + +enum struct type : uint8_t { Build = 0, Short32 = 1, Catalog = 3 }; +enum struct build_subtype : uint8_t { Compact32 = 0, Compact64 = 1, Long = 2 }; +enum struct catalog_subtype : uint8_t { Id32_Pack32 = 1 }; + +using type_f = field<"type", type>::located; +using opt_len_f = + field<"opt_len", bool>::located; +using payload_len_f = + field<"payload_len", + std::uint16_t>::located; + +using build_subtype_f = + field<"subtype", + build_subtype>::located; +using compact32_build_id_f = field<"build_id", std::uint32_t>::located< + at{dword_index_t{0}, 31_msb, 30_lsb}, at{dword_index_t{0}, 23_msb, 4_lsb}>; +using compact64_build_id_f = field<"build_id", std::uint64_t>::located< + at{dword_index_t{1}, 31_msb, 0_lsb}, at{dword_index_t{0}, 31_msb, 30_lsb}, + at{dword_index_t{0}, 23_msb, 4_lsb}>; + +using normal_build_msg_t = + message<"normal_build", type_f::with_required, + opt_len_f::with_required, + build_subtype_f::with_required, payload_len_f>; +using compact32_build_msg_t = + message<"compact32_build", type_f::with_required, + build_subtype_f::with_required, + compact32_build_id_f>; +using compact64_build_msg_t = + message<"compact64_build", type_f::with_required, + build_subtype_f::with_required, + compact64_build_id_f>; + +using short32_payload_f = + field<"payload", + std::uint32_t>::located; +using short32_msg_t = + message<"short32", type_f::with_required, short32_payload_f>; + +using catalog_subtype_f = + field<"subtype", + catalog_subtype>::located; +using severity_f = field<"severity", std::uint8_t>::located; +using module_id_f = + field<"module_id", + std::uint8_t>::located; + +using catalog_msg_t = + message<"catalog", type_f::with_required, severity_f, + module_id_f, + catalog_subtype_f::with_required>; +} // namespace defn +} // namespace logging::mipi diff --git a/test/log/mipi_encoder.cpp b/test/log/mipi_encoder.cpp index cd04479a..68b64823 100644 --- a/test/log/mipi_encoder.cpp +++ b/test/log/mipi_encoder.cpp @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -67,8 +68,7 @@ constexpr auto check = [](auto value, auto expected) { CHECK(value == expected); }; -constexpr auto check_at = [](stdx::span span, auto dw_idx, - std::uint32_t expected) { +constexpr auto check_at = [](auto span, auto dw_idx, std::uint32_t expected) { auto idx = dw_idx * sizeof(std::uint32_t); auto sz = std::min(sizeof(std::uint32_t), span.size() - idx); @@ -78,13 +78,13 @@ constexpr auto check_at = [](stdx::span span, auto dw_idx, }; template -constexpr auto check_buffer = [](stdx::span data) { +constexpr auto check_buffer = [](auto data) { REQUIRE(data.size() > (sizeof...(Expected) - 1) * sizeof(std::uint32_t)); auto idx = std::size_t{}; (check_at(data, idx++, Expected), ...); }; -template +template struct test_log_args_destination { template auto log_by_args(std::uint32_t header, Args... args) { @@ -98,7 +98,8 @@ struct test_log_args_destination { template struct test_log_buf_destination { - auto log_by_buf(stdx::span data) const { + template + auto log_by_buf(stdx::span data) const { constexpr auto Header = expected_msg_header(Level, test_module_id, sizeof...(ExpectedArgs)); check_buffer(data); @@ -115,7 +116,8 @@ struct test_log_version_destination { ++num_log_args_calls; } - auto log_by_buf(stdx::span data) const { + template + auto log_by_buf(stdx::span data) const { check_buffer(data); ++num_log_args_calls; } @@ -130,7 +132,7 @@ TEST_CASE("log zero arguments", "[mipi]") { CIB_LOG_ENV(logging::get_level, logging::level::TRACE); test_critical_section::count = 0; auto cfg = logging::mipi::config{ - test_log_args_destination{}}; + test_log_args_destination{}}; cfg.logger.log_msg("Hello"_sc); CHECK(test_critical_section::count == 2); } @@ -139,7 +141,7 @@ TEST_CASE("log one argument", "[mipi]") { CIB_LOG_ENV(logging::get_level, logging::level::TRACE); test_critical_section::count = 0; auto cfg = logging::mipi::config{ - test_log_args_destination{}}; + test_log_args_destination{}}; cfg.logger.log_msg(format("{}"_sc, 17u)); CHECK(test_critical_section::count == 2); } @@ -148,8 +150,7 @@ TEST_CASE("log two arguments", "[mipi]") { CIB_LOG_ENV(logging::get_level, logging::level::TRACE); test_critical_section::count = 0; auto cfg = logging::mipi::config{ - test_log_args_destination{}}; + test_log_args_destination{}}; cfg.logger.log_msg(format("{} {}"_sc, 17u, 18u)); CHECK(test_critical_section::count == 2); } @@ -180,10 +181,8 @@ TEST_CASE("log to multiple destinations", "[mipi]") { test_critical_section::count = 0; num_log_args_calls = 0; auto cfg = logging::mipi::config{ - test_log_args_destination{}, - test_log_args_destination{}}; + test_log_args_destination{}, + test_log_args_destination{}}; cfg.logger.log_msg(format("{} {}"_sc, 17u, 18u)); CHECK(test_critical_section::count == 4); @@ -229,11 +228,40 @@ TEST_CASE("log version information (long with string)", "[mipi]") { } template <> -inline auto logging::config<> = logging::mipi::config{ - test_log_args_destination{}}; +inline auto logging::config<> = + logging::mipi::config{test_log_args_destination{}}; TEST_CASE("injection", "[mipi]") { test_critical_section::count = 0; CIB_TRACE("Hello"); CHECK(test_critical_section::count == 2); } + +namespace { +int num_catalog_args_calls{}; + +template struct test_catalog_args_destination { + template + auto log_by_args(std::uint32_t header, std::uint32_t id, Args...) { + constexpr auto Header = + expected_catalog32_header(Level, test_module_id); + CHECK(header == Header); + CHECK(id == test_string_id); + static_assert(sizeof...(Args) == 0); + ++num_catalog_args_calls; + } +}; +} // namespace + +TEST_CASE("log with overridden builder", "[mipi]") { + using catalog_env = logging::make_env_t< + logging::get_level, logging::level::TRACE, logging::mipi::get_builder, + logging::mipi::builder{}>; + + num_catalog_args_calls = 0; + auto cfg = logging::mipi::config{ + test_catalog_args_destination{}}; + + cfg.logger.log_msg("Hello"_sc); + CHECK(num_catalog_args_calls == 1); +}