From 9907b7de960e649ed8a6dc9e94e5cfae0000120b Mon Sep 17 00:00:00 2001
From: Ben Deane <ben@elbeno.com>
Date: Fri, 22 Aug 2025 09:38:00 -0600
Subject: [PATCH] :sparkles: Add unit to logging

Problem:
- Multiple processes may emit the same log calls, and there is no way to tell
  them apart. The MIPI spec uses a unit field in the catalog message for this
  purpose.

Solution:
- Add support for `unit` in a catalog message.

Note:
- The value of unit is not known at compile time, but it is static for the
  program, so a function can obtain it.
---
 include/log/catalog/encoder.hpp       |  5 +--
 include/log/catalog/mipi_builder.hpp  | 20 +++++------
 include/log/catalog/mipi_messages.hpp | 18 +++++++---
 include/log/unit.hpp                  | 22 ++++++++++++
 test/log/CMakeLists.txt               |  3 +-
 test/log/encoder.cpp                  |  5 +--
 test/log/unit.cpp                     | 52 +++++++++++++++++++++++++++
 7 files changed, 106 insertions(+), 19 deletions(-)
 create mode 100644 include/log/unit.hpp
 create mode 100644 test/log/unit.cpp

diff --git a/include/log/catalog/encoder.hpp b/include/log/catalog/encoder.hpp
index 49e09d20..95859190 100644
--- a/include/log/catalog/encoder.hpp
+++ b/include/log/catalog/encoder.hpp
@@ -8,6 +8,7 @@
 #include <log/module.hpp>
 #include <log/module_id.hpp>
 #include <log/string_id.hpp>
+#include <log/unit.hpp>
 
 #include <stdx/ct_string.hpp>
 #include <stdx/span.hpp>
@@ -17,7 +18,6 @@
 #include <conc/concurrency.hpp>
 
 #include <cstddef>
-#include <cstdint>
 #include <string_view>
 #include <type_traits>
 #include <utility>
@@ -81,9 +81,10 @@ template <writer_like Writer> struct log_handler {
             using Module =
                 decltype(detail::to_module<get_module(Env{}),
                                            logging::get_module_id(Env{})>());
+            auto const unit = get_unit(Env{})();
             auto const pkt =
                 builder.template build<L>(catalog<Message>(), module<Module>(),
-                                          std::forward<Args>(args)...);
+                                          unit, std::forward<Args>(args)...);
             writer(pkt.as_const_view().data());
         });
     }
diff --git a/include/log/catalog/mipi_builder.hpp b/include/log/catalog/mipi_builder.hpp
index f9971130..bacbea00 100644
--- a/include/log/catalog/mipi_builder.hpp
+++ b/include/log/catalog/mipi_builder.hpp
@@ -24,7 +24,7 @@ concept packer = std::integral<typename T::template pack_as_t<int>> and
 template <typename, packer> struct builder;
 
 template <packer P> struct builder<defn::short32_msg_t, P> {
-    template <auto Level> static auto build(string_id id, module_id) {
+    template <auto Level> static auto build(string_id id, module_id, unit_t) {
         using namespace msg;
         return owning<defn::short32_msg_t>{"payload"_field = id};
     }
@@ -32,10 +32,10 @@ template <packer P> struct builder<defn::short32_msg_t, P> {
 
 template <typename Storage, packer P> struct catalog_builder {
     template <auto Level, packable... Ts>
-    static auto build(string_id id, module_id m, Ts... args) {
+    static auto build(string_id id, module_id m, unit_t u, Ts... args) {
         using namespace msg;
-        defn::catalog_msg_t::owner_t<Storage> message{"severity"_field = Level,
-                                                      "module_id"_field = m};
+        defn::catalog_msg_t::owner_t<Storage> message{
+            "severity"_field = Level, "module_id"_field = m, "unit"_field = u};
 
         using V = typename Storage::value_type;
         constexpr auto header_size = defn::catalog_msg_t::size<V>::value;
@@ -65,7 +65,7 @@ template <typename Storage, packer P> struct catalog_builder {
 
 template <packer P> struct builder<defn::catalog_msg_t, P> {
     template <auto Level, typename... Ts>
-    static auto build(string_id id, module_id m, Ts... args) {
+    static auto build(string_id id, module_id m, unit_t u, Ts... args) {
         using namespace msg;
         constexpr auto payload_size =
             (sizeof(id) + ... + sizeof(typename P::template pack_as_t<Ts>));
@@ -74,7 +74,7 @@ template <packer P> struct builder<defn::catalog_msg_t, P> {
         using storage_t =
             std::array<std::uint32_t, header_size + stdx::sized8{payload_size}
                                                         .in<std::uint32_t>()>;
-        return catalog_builder<storage_t, P>{}.template build<Level>(id, m,
+        return catalog_builder<storage_t, P>{}.template build<Level>(id, m, u,
                                                                      args...);
     }
 };
@@ -115,13 +115,13 @@ template <packer P> struct builder<defn::normal_build_msg_t, P> {
 
 template <packer P = logging::default_arg_packer> struct default_builder {
     template <auto Level, packable... Ts>
-    static auto build(string_id id, module_id m, Ts... args) {
+    static auto build(string_id id, module_id m, unit_t unit, Ts... args) {
         if constexpr (sizeof...(Ts) == 0u) {
-            return builder<defn::short32_msg_t, P>{}.template build<Level>(id,
-                                                                           m);
+            return builder<defn::short32_msg_t, P>{}.template build<Level>(
+                id, m, unit);
         } else {
             return builder<defn::catalog_msg_t, P>{}.template build<Level>(
-                id, m, args...);
+                id, m, unit, args...);
         }
     }
 
diff --git a/include/log/catalog/mipi_messages.hpp b/include/log/catalog/mipi_messages.hpp
index b89c34b1..b5550a25 100644
--- a/include/log/catalog/mipi_messages.hpp
+++ b/include/log/catalog/mipi_messages.hpp
@@ -2,7 +2,11 @@
 
 #include <msg/message.hpp>
 
+#include <cstdint>
+
 namespace logging::mipi {
+using unit_t = std::uint32_t;
+
 namespace defn {
 using msg::at;
 using msg::dword_index_t;
@@ -11,9 +15,13 @@ 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 };
+enum struct type : std::uint8_t { Build = 0, Short32 = 1, Catalog = 3 };
+enum struct build_subtype : std::uint8_t {
+    Compact32 = 0,
+    Compact64 = 1,
+    Long = 2
+};
+enum struct catalog_subtype : std::uint8_t { Id32_Pack32 = 1 };
 
 using type_f = field<"type", type>::located<at{dword_index_t{0}, 3_msb, 0_lsb}>;
 using opt_len_f =
@@ -58,9 +66,11 @@ using severity_f = field<"severity", std::uint8_t>::located<at{dword_index_t{0},
 using module_id_f =
     field<"module_id",
           std::uint8_t>::located<at{dword_index_t{0}, 22_msb, 16_lsb}>;
+using unit_f =
+    field<"unit", std::uint16_t>::located<at{dword_index_t{0}, 15_msb, 12_lsb}>;
 
 using catalog_msg_t =
-    message<"catalog", type_f::with_required<type::Catalog>, severity_f,
+    message<"catalog", type_f::with_required<type::Catalog>, severity_f, unit_f,
             module_id_f,
             catalog_subtype_f::with_required<catalog_subtype::Id32_Pack32>>;
 } // namespace defn
diff --git a/include/log/unit.hpp b/include/log/unit.hpp
new file mode 100644
index 00000000..3a7d5c36
--- /dev/null
+++ b/include/log/unit.hpp
@@ -0,0 +1,22 @@
+#pragma once
+
+#include <stdx/compiler.hpp>
+#include <stdx/utility.hpp>
+
+#include <utility>
+
+namespace logging {
+[[maybe_unused]] constexpr inline struct get_unit_t {
+    template <typename T>
+        requires true
+    CONSTEVAL auto operator()(T &&t) const
+        noexcept(noexcept(std::forward<T>(t).query(std::declval<get_unit_t>())))
+            -> decltype(std::forward<T>(t).query(*this)) {
+        return std::forward<T>(t).query(*this);
+    }
+
+    CONSTEVAL auto operator()(auto &&) const {
+        return [] { return 0; };
+    }
+} get_unit;
+} // namespace logging
diff --git a/test/log/CMakeLists.txt b/test/log/CMakeLists.txt
index fb643da5..aab0a43c 100644
--- a/test/log/CMakeLists.txt
+++ b/test/log/CMakeLists.txt
@@ -1,9 +1,10 @@
 add_tests(
     FILES
+    env
     level
     log
     module_id
-    env
+    unit
     LIBRARIES
     cib_log)
 add_tests(FILES fmt_logger LIBRARIES cib_log_fmt)
diff --git a/test/log/encoder.cpp b/test/log/encoder.cpp
index d2d14465..70c1b99d 100644
--- a/test/log/encoder.cpp
+++ b/test/log/encoder.cpp
@@ -370,10 +370,11 @@ template <logging::level Level> struct test_catalog_args_destination {
 
 struct custom_builder : logging::mipi::default_builder<> {
     template <auto Level, logging::packable... Ts>
-    static auto build(string_id id, module_id m, Ts... args) {
+    static auto build(string_id id, module_id m, logging::mipi::unit_t u,
+                      Ts... args) {
         return logging::mipi::builder<logging::mipi::defn::catalog_msg_t,
                                       logging::default_arg_packer>{}
-            .template build<Level>(id, m, args...);
+            .template build<Level>(id, m, u, args...);
     }
 };
 } // namespace
diff --git a/test/log/unit.cpp b/test/log/unit.cpp
new file mode 100644
index 00000000..20110739
--- /dev/null
+++ b/test/log/unit.cpp
@@ -0,0 +1,52 @@
+#include <log/catalog/encoder.hpp>
+#include <log/level.hpp>
+#include <log/unit.hpp>
+
+#include <stdx/ct_conversions.hpp>
+#include <stdx/ct_format.hpp>
+#include <stdx/ct_string.hpp>
+#include <stdx/span.hpp>
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <cstddef>
+#include <cstdint>
+
+template <typename> auto catalog() -> string_id { return 0xdeadbeef; }
+template <typename> auto module() -> module_id { return 0x5a; }
+
+namespace {
+int log_calls{};
+logging::mipi::unit_t test_unit{};
+
+struct test_destination {
+    template <std::size_t N>
+    auto operator()(stdx::span<std::uint32_t const, N> pkt) const {
+        using namespace msg;
+        auto const msg =
+            msg::const_view<logging::mipi::defn::catalog_msg_t>{pkt};
+        CHECK(msg.get("unit"_f) == test_unit);
+        ++log_calls;
+    }
+};
+
+} // namespace
+
+TEST_CASE("mipi logger works with init-time unit", "[unit]") {
+    log_calls = 0;
+    test_unit = 5;
+    CIB_LOG_ENV(logging::get_level, logging::level::TRACE, logging::get_unit,
+                [] { return test_unit; });
+    auto cfg = logging::binary::config{test_destination{}};
+    cfg.logger.log_msg<cib_log_env_t>(stdx::ct_format<"Hello {} {}">(42, 17));
+    CHECK(log_calls == 1);
+}
+
+TEST_CASE("default unit is 0", "[unit]") {
+    log_calls = 0;
+    test_unit = 0;
+    CIB_LOG_ENV(logging::get_level, logging::level::TRACE);
+    auto cfg = logging::binary::config{test_destination{}};
+    cfg.logger.log_msg<cib_log_env_t>(stdx::ct_format<"Hello {} {}">(42, 17));
+    CHECK(log_calls == 1);
+}