refactor(firmware): centralize configuration and diagnostics

firmware/libs/kaptan-log/src/hub.rs

@@ -4,7 +4,7 @@ use core::fmt;
 
 use crate::{
     DeliveryClass, ErrorAdditionalInformation, ErrorPath, ErrorReport, LogFilter, LogFlags,
-    LogRecord, LogText, Severity, StaticError,
+    LogRecord, LogText, Severity, StaticError, StaticErrorMailbox,
 };
 
 /// Outcome from one bounded diagnostic admission attempt.
@@ -12,9 +12,7 @@ use crate::{
 pub enum PublishOutcome {
     /// The record was retained.
     Queued,
-    /// An adjacent equivalent record absorbed this occurrence.
-    Coalesced,
-    /// An ordinary log record was excluded by the configured verbosity cap.
+    /// A record was excluded by this sink's configured policy.
     Filtered,
     /// A best-effort record was replaced by newer optional information.
     ReplacedOldest,
@@ -26,37 +24,10 @@ pub enum PublishOutcome {
 /// record to its lower transport.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum TransportAdmission {
-    /// The lower transport retained the record, so it may leave the hub.
+    /// The lower transport retained the record, so it may leave the queue.
     Accepted,
     /// The lower transport did not accept it yet; retain it for retry.
     Retry,
-    /// The lower transport will not accept optional detail. This can only
-    /// discard a best-effort record; critical records remain queued.
-    DropBestEffort,
-}
-
-/// Saturating, process-local accounting for bounded diagnostic admission.
-///
-/// These counters intentionally are not part of `KAPTAN_LOG` or
-/// `KAPTAN_ERROR`: neither wire message is a metrics protocol.
-#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
-pub struct HubHealth {
-    /// Optional records rejected by the active verbosity cap.
-    pub filtered: u32,
-    /// Adjacent records folded into an existing entry.
-    pub coalesced: u32,
-    /// Optional entries replaced because their bounded lane was full.
-    pub best_effort_replaced: u32,
-    /// Error entries rejected because their bounded critical lane was full.
-    pub critical_rejected: u32,
-    /// Optional entries the lower transport refused permanently.
-    pub best_effort_transport_dropped: u32,
-}
-
-impl HubHealth {
-    fn increment(value: &mut u32) {
-        *value = value.saturating_add(1);
-    }
 }
 
 struct Lane<const N: usize> {
@@ -76,15 +47,6 @@ impl<const N: usize> Lane<N> {
     }
 
     fn publish(&mut self, record: LogRecord, replace_oldest: bool) -> PublishOutcome {
-        if self.len != 0 {
-            let tail = (self.head + self.len - 1) % N;
-            if let Some(previous) = self.entries[tail].as_mut()
-                && previous.is_equivalent_to(&record)
-            {
-                previous.increment_occurrences();
-                return PublishOutcome::Coalesced;
-            }
-        }
         if self.len == N {
             if replace_oldest {
                 self.entries[self.head] = Some(record);
@@ -99,6 +61,10 @@ impl<const N: usize> Lane<N> {
         PublishOutcome::Queued
     }
 
+    const fn is_full(&self) -> bool {
+        self.len == N
+    }
+
     fn peek(&self) -> Option<LogRecord> {
         self.entries.get(self.head).copied().flatten()
     }
@@ -114,39 +80,78 @@ impl<const N: usize> Lane<N> {
     }
 }
 
-/// One bounded priority hub. It owns no global logger, MAVLink state, or DMA.
-pub struct EventHub<const CRITICAL: usize, const BEST_EFFORT: usize> {
+/// One bounded priority queue owned by a log sink.
+///
+/// It owns no global logger, MAVLink state, DMA, filesystem, or other sink.
+pub struct LogSinkQueue<const CRITICAL: usize, const BEST_EFFORT: usize> {
     filter: LogFilter,
     critical: Lane<CRITICAL>,
     best_effort: Lane<BEST_EFFORT>,
-    health: HubHealth,
 }
 
-impl<const CRITICAL: usize, const BEST_EFFORT: usize> EventHub<CRITICAL, BEST_EFFORT> {
-    /// Creates one hub with its immutable boot-active verbosity cap.
+impl<const CRITICAL: usize, const BEST_EFFORT: usize> LogSinkQueue<CRITICAL, BEST_EFFORT> {
+    /// Creates one sink queue with its immutable boot-active policy.
     pub const fn new(filter: LogFilter) -> Self {
         Self {
             filter,
             critical: Lane::new(),
             best_effort: Lane::new(),
-            health: HubHealth {
-                filtered: 0,
-                coalesced: 0,
-                best_effort_replaced: 0,
-                critical_rejected: 0,
-                best_effort_transport_dropped: 0,
-            },
         }
     }
 
-    /// Immutable best-effort verbosity policy.
+    /// Immutable sink policy.
     pub const fn filter(&self) -> LogFilter {
         self.filter
     }
 
-    /// Process-local bounded-admission accounting.
-    pub const fn health(&self) -> HubHealth {
-        self.health
+    /// Whether an error source may be consumed without losing an enabled
+    /// critical record. Filtered errors may be consumed intentionally.
+    pub const fn can_publish_error(&self) -> bool {
+        !self.filter.allows(Severity::Error) || !self.critical.is_full()
+    }
+
+    /// Consumes at most one source-local static-error mailbox in round-robin
+    /// order. An enabled source remains untouched while the critical lane is
+    /// full; a filtered source is consumed intentionally.
+    pub fn drain_next_error_mailbox<const SOURCES: usize>(
+        &mut self,
+        mailboxes: [&StaticErrorMailbox; SOURCES],
+        cursor: &mut u8,
+    ) -> bool {
+        assert!(
+            SOURCES != 0 && SOURCES <= u8::MAX as usize,
+            "error mailbox fan-in must fit its u8 cursor"
+        );
+        if !self.can_publish_error() {
+            return false;
+        }
+        if usize::from(*cursor) >= SOURCES {
+            *cursor = 0;
+        }
+
+        let mut checked = 0;
+        while checked < SOURCES {
+            let index = usize::from(*cursor);
+            *cursor = if index + 1 == SOURCES { 0 } else { *cursor + 1 };
+            checked += 1;
+
+            let (error, coalesced) = mailboxes[index].take();
+            let Some(error) = error else {
+                continue;
+            };
+            let flags = if coalesced {
+                LogFlags::COALESCED
+            } else {
+                LogFlags::NONE
+            };
+            let outcome = self.publish_error_static(flags, error);
+            debug_assert!(matches!(
+                outcome,
+                PublishOutcome::Queued | PublishOutcome::Filtered
+            ));
+            return true;
+        }
+        false
     }
 
     /// Formats and retains one ordinary task/worker diagnostic. Error-level
@@ -164,7 +169,7 @@ impl<const CRITICAL: usize, const BEST_EFFORT: usize> EventHub<CRITICAL, BEST_EF
     /// Retains one static derived error without formatting or exposing source
     /// values. The adapter maps it to `KAPTAN_ERROR`.
     pub fn publish_error<E: ErrorReport>(&mut self, flags: LogFlags, error: &E) -> PublishOutcome {
-        self.publish_error_static(flags, error.static_error())
+        self.publish_record(LogRecord::from_error(flags, error))
     }
 
     /// Retains one static mailbox error. This is the ISR-safe counterpart to
@@ -174,16 +179,13 @@ impl<const CRITICAL: usize, const BEST_EFFORT: usize> EventHub<CRITICAL, BEST_EF
         flags: LogFlags,
         error: &'static StaticError,
     ) -> PublishOutcome {
-        self.publish_error_text(
-            flags,
-            ErrorPath::from_message(error.path()),
-            ErrorAdditionalInformation::from_message(error.additional_information()),
-        )
+        self.publish_record(LogRecord::from_static_error(flags, error))
     }
 
     /// Retains a preformatted error identity and optional detail. Both
-    /// components stay transport-neutral here; the MAVLink adapter enforces
-    /// its independent on-wire capacities.
+    /// transport-neutral components already have the exact common diagnostic
+    /// envelope capacities, so a sink adapter need not retain or clip them
+    /// again.
     pub fn publish_error_text(
         &mut self,
         flags: LogFlags,
@@ -194,38 +196,29 @@ impl<const CRITICAL: usize, const BEST_EFFORT: usize> EventHub<CRITICAL, BEST_EF
     }
 
     /// Retains one preformatted bounded text payload. Error-or-higher
-    /// severities are independently retained and sent as `KAPTAN_ERROR`.
+    /// severities are independently encoded as `KAPTAN_ERROR`.
     pub fn publish_text(
         &mut self,
         severity: Severity,
         flags: LogFlags,
         text: LogText,
     ) -> PublishOutcome {
-        let delivery = delivery_for(severity);
-        if delivery == DeliveryClass::BestEffort && !self.filter.allows(severity) {
-            HubHealth::increment(&mut self.health.filtered);
-            return PublishOutcome::Filtered;
-        }
         self.publish_record(LogRecord::new(severity, flags, text))
     }
 
-    fn publish_record(&mut self, record: LogRecord) -> PublishOutcome {
+    /// Applies this sink's policy and retains one already-constructed event.
+    ///
+    /// Composition may submit the same transport-neutral record to multiple
+    /// sink queues without coupling event creation to any sink policy.
+    pub fn publish_record(&mut self, record: LogRecord) -> PublishOutcome {
+        if !self.filter.allows(record.severity()) {
+            return PublishOutcome::Filtered;
+        }
         let delivery = record.delivery();
-        let outcome = match delivery {
+        match delivery {
             DeliveryClass::Critical => self.critical.publish(record, false),
             DeliveryClass::BestEffort => self.best_effort.publish(record, true),
-        };
-        match outcome {
-            PublishOutcome::Coalesced => HubHealth::increment(&mut self.health.coalesced),
-            PublishOutcome::ReplacedOldest => {
-                HubHealth::increment(&mut self.health.best_effort_replaced);
-            }
-            PublishOutcome::RejectedFull => {
-                HubHealth::increment(&mut self.health.critical_rejected);
-            }
-            PublishOutcome::Queued | PublishOutcome::Filtered => {}
         }
-        outcome
     }
 
     /// Highest-priority record without relinquishing it.
@@ -233,9 +226,9 @@ impl<const CRITICAL: usize, const BEST_EFFORT: usize> EventHub<CRITICAL, BEST_EF
         self.critical.peek().or_else(|| self.best_effort.peek())
     }
 
-    /// Submits the current record through the sole telemetry owner. The hub
-    /// removes it only after `Accepted`, and only permits `DropBestEffort` for
-    /// optional detail. This keeps encoder/admission retry semantics local.
+    /// Submits the current record through the sink owner and removes it only
+    /// after `Accepted`. This keeps encoder/admission retry semantics local
+    /// for every severity.
     pub fn submit_next<R>(
         &mut self,
         submit: impl FnOnce(&LogRecord) -> (TransportAdmission, R),
@@ -246,13 +239,7 @@ impl<const CRITICAL: usize, const BEST_EFFORT: usize> EventHub<CRITICAL, BEST_EF
             TransportAdmission::Accepted => {
                 let _ = self.pop();
             }
-            TransportAdmission::DropBestEffort
-                if record.delivery() == DeliveryClass::BestEffort =>
-            {
-                let _ = self.best_effort.pop();
-                HubHealth::increment(&mut self.health.best_effort_transport_dropped);
-            }
-            TransportAdmission::Retry | TransportAdmission::DropBestEffort => {}
+            TransportAdmission::Retry => {}
         }
         Some(result)
     }
@@ -262,27 +249,30 @@ impl<const CRITICAL: usize, const BEST_EFFORT: usize> EventHub<CRITICAL, BEST_EF
     }
 }
 
-fn delivery_for(severity: Severity) -> DeliveryClass {
-    if severity <= Severity::Error {
-        DeliveryClass::Critical
-    } else {
-        DeliveryClass::BestEffort
-    }
-}
-
 #[cfg(test)]
 mod tests {
-    use super::{EventHub, PublishOutcome, TransportAdmission};
+    use super::{LogSinkQueue, PublishOutcome, TransportAdmission};
     use crate::{
-        DeliveryClass, ErrorAdditionalInformation, ErrorPath, LogFilter, LogFlags, LogText,
-        RecordChannel, Severity, StaticError,
+        DeliveryClass, ErrorAdditionalInformation, ErrorPath, LogFilter, LogFlags, LogRecord,
+        LogText, RecordChannel, Severity, StaticError,
     };
 
     #[test]
-    fn errors_bypass_filter_but_debug_does_not() {
-        let mut hub = EventHub::<1, 1>::new(LogFilter::new(Severity::Info));
+    fn sink_policy_applies_to_errors_and_ordinary_logs() {
+        let mut disabled = LogSinkQueue::<1, 1>::new(LogFilter::disabled());
         assert_eq!(
-            hub.publish_text(
+            disabled.publish_text(
+                Severity::Error,
+                LogFlags::NONE,
+                LogText::from_message("error")
+            ),
+            PublishOutcome::Filtered
+        );
+        assert!(disabled.peek().is_none());
+
+        let mut queue = LogSinkQueue::<1, 1>::new(LogFilter::up_to(Severity::Info));
+        assert_eq!(
+            queue.publish_text(
                 Severity::Debug,
                 LogFlags::NONE,
                 LogText::from_message("debug")
@@ -290,43 +280,110 @@ mod tests {
             PublishOutcome::Filtered
         );
         assert_eq!(
-            hub.publish_text(
+            queue.publish_text(
                 Severity::Error,
                 LogFlags::NONE,
                 LogText::from_message("error")
             ),
             PublishOutcome::Queued
         );
-        let record = hub.peek().unwrap();
+        let record = queue.peek().unwrap();
         assert_eq!(record.error_path().unwrap().as_str(), "error");
         assert!(record.additional_information().unwrap().is_empty());
         assert_eq!(record.channel(), RecordChannel::Error);
     }
 
     #[test]
-    fn static_messages_and_repeats_are_bounded() {
-        static FAULT: StaticError = StaticError::new("driver::Fault");
-        let mut hub = EventHub::<1, 1>::new(LogFilter::default());
+    fn full_critical_lane_backpressures_only_an_enabled_error_source() {
+        let mut enabled = LogSinkQueue::<1, 1>::new(LogFilter::up_to(Severity::Info));
+        assert!(enabled.can_publish_error());
         assert_eq!(
-            hub.publish_error_static(LogFlags::NONE, &FAULT),
+            enabled.publish_text(
+                Severity::Error,
+                LogFlags::NONE,
+                LogText::from_message("pending")
+            ),
+            PublishOutcome::Queued
+        );
+        assert!(!enabled.can_publish_error());
+
+        let disabled = LogSinkQueue::<1, 1>::new(LogFilter::disabled());
+        assert!(disabled.can_publish_error());
+    }
+
+    #[test]
+    fn mailbox_fan_in_survives_backpressure_and_is_round_robin() {
+        static FIRST: StaticError = StaticError::new("first");
+        static SECOND: StaticError = StaticError::new("second");
+        static THIRD: StaticError = StaticError::new("third");
+        let first = crate::StaticErrorMailbox::new();
+        let second = crate::StaticErrorMailbox::new();
+        let third = crate::StaticErrorMailbox::new();
+        let mailboxes = [&first, &second, &third];
+        first.record(&FIRST);
+        second.record(&SECOND);
+        third.record(&THIRD);
+
+        let mut queue = LogSinkQueue::<1, 1>::new(LogFilter::up_to(Severity::Info));
+        let mut cursor = 0;
+        assert!(queue.drain_next_error_mailbox(mailboxes, &mut cursor));
+        assert_eq!(
+            queue.peek().unwrap().error_path().unwrap().as_str(),
+            "first"
+        );
+
+        first.record(&FIRST);
+        assert!(!queue.drain_next_error_mailbox(mailboxes, &mut cursor));
+        assert_eq!(
+            queue.peek().unwrap().error_path().unwrap().as_str(),
+            "first"
+        );
+
+        let mut expect = |path| {
+            assert_eq!(queue.peek().unwrap().error_path().unwrap().as_str(), path);
+            assert_eq!(
+                queue.submit_next(|_| (TransportAdmission::Accepted, ())),
+                Some(())
+            );
+            assert!(queue.drain_next_error_mailbox(mailboxes, &mut cursor));
+        };
+        expect("first");
+        expect("second");
+        expect("third");
+        assert_eq!(
+            queue.peek().unwrap().error_path().unwrap().as_str(),
+            "first"
+        );
+    }
+
+    #[test]
+    fn identical_events_remain_distinct_records() {
+        static FAULT: StaticError = StaticError::new("driver::Fault");
+        let mut queue = LogSinkQueue::<2, 1>::new(LogFilter::up_to(Severity::Debug));
+        assert_eq!(
+            queue.publish_error_static(LogFlags::NONE, &FAULT),
             PublishOutcome::Queued
         );
         assert_eq!(
-            hub.publish_error_static(LogFlags::NONE, &FAULT),
-            PublishOutcome::Coalesced
+            queue.publish_error_static(LogFlags::NONE, &FAULT),
+            PublishOutcome::Queued
         );
-        let record = hub.peek().unwrap();
-        assert_eq!(record.occurrences, 2);
-        assert!(record.flags().contains(LogFlags::COALESCED));
+        let record = queue.peek().unwrap();
+        assert!(!record.flags().contains(LogFlags::COALESCED));
         assert_eq!(record.error_path().unwrap().as_str(), "driver::Fault");
         assert!(record.additional_information().unwrap().is_empty());
+        assert_eq!(
+            queue.submit_next(|_| (TransportAdmission::Accepted, ())),
+            Some(())
+        );
+        assert_eq!(queue.peek(), Some(record));
     }
 
     #[test]
     fn static_error_detail_needs_no_producer_formatting() {
         static FAULT: StaticError =
             StaticError::with_additional_information("driver::Dma::Fault", "stream 7");
-        let mut hub = EventHub::<1, 1>::new(LogFilter::default());
+        let mut hub = LogSinkQueue::<1, 1>::new(LogFilter::up_to(Severity::Debug));
         assert_eq!(
             hub.publish_error_static(LogFlags::NONE, &FAULT),
             PublishOutcome::Queued
@@ -341,7 +398,7 @@ mod tests {
 
     #[test]
     fn error_identity_and_detail_are_independent_record_components() {
-        let mut hub = EventHub::<1, 1>::new(LogFilter::default());
+        let mut hub = LogSinkQueue::<1, 1>::new(LogFilter::up_to(Severity::Debug));
         let _ = hub.publish_error_text(
             LogFlags::NONE,
             ErrorPath::from_message("driver::Icm42688::Transfer"),
@@ -360,15 +417,15 @@ mod tests {
     }
 
     #[test]
-    fn retry_retains_critical_and_only_best_effort_can_be_discarded() {
-        let mut hub = EventHub::<1, 1>::new(LogFilter::default());
+    fn retry_retains_every_severity() {
+        let mut hub = LogSinkQueue::<1, 1>::new(LogFilter::up_to(Severity::Debug));
         let _ = hub.publish_text(
             Severity::Error,
             LogFlags::NONE,
             LogText::from_message("fault"),
         );
         assert_eq!(
-            hub.submit_next(|_| (TransportAdmission::DropBestEffort, ())),
+            hub.submit_next(|_| (TransportAdmission::Retry, ())),
             Some(())
         );
         assert_eq!(hub.peek().unwrap().delivery(), DeliveryClass::Critical);
@@ -384,50 +441,30 @@ mod tests {
             LogText::from_message("detail"),
         );
         assert_eq!(
-            hub.submit_next(|_| (TransportAdmission::DropBestEffort, ())),
+            hub.submit_next(|_| (TransportAdmission::Retry, ())),
+            Some(())
+        );
+        assert_eq!(hub.peek().unwrap().delivery(), DeliveryClass::BestEffort);
+        assert_eq!(
+            hub.submit_next(|_| (TransportAdmission::Accepted, ())),
             Some(())
         );
         assert!(hub.peek().is_none());
-        assert_eq!(hub.health().best_effort_transport_dropped, 1);
     }
 
     #[test]
-    fn admission_losses_are_retained_as_local_saturating_accounting() {
-        let mut hub = EventHub::<1, 1>::new(LogFilter::new(Severity::Info));
-        let _ = hub.publish_text(
-            Severity::Debug,
-            LogFlags::NONE,
-            LogText::from_message("filtered"),
-        );
-        let _ = hub.publish_text(
+    fn independent_sink_policies_do_not_suppress_each_other() {
+        let record = LogRecord::new(
             Severity::Info,
             LogFlags::NONE,
-            LogText::from_message("first optional"),
-        );
-        let _ = hub.publish_text(
-            Severity::Info,
-            LogFlags::NONE,
-            LogText::from_message("newer optional"),
-        );
-        let _ = hub.publish_text(
-            Severity::Error,
-            LogFlags::NONE,
-            LogText::from_message("first error"),
-        );
-        let _ = hub.publish_text(
-            Severity::Error,
-            LogFlags::NONE,
-            LogText::from_message("second error"),
-        );
-        assert_eq!(
-            hub.health(),
-            super::HubHealth {
-                filtered: 1,
-                coalesced: 0,
-                best_effort_replaced: 1,
-                critical_rejected: 1,
-                best_effort_transport_dropped: 0,
-            }
+            LogText::from_message("flight ready"),
         );
+        let mut mavlink = LogSinkQueue::<1, 1>::new(LogFilter::disabled());
+        let mut blackbox = LogSinkQueue::<1, 1>::new(LogFilter::up_to(Severity::Debug));
+
+        assert_eq!(mavlink.publish_record(record), PublishOutcome::Filtered);
+        assert_eq!(blackbox.publish_record(record), PublishOutcome::Queued);
+        assert!(mavlink.peek().is_none());
+        assert_eq!(blackbox.peek(), Some(record));
     }
 }