docs: add IPC naming convention and debounce semantics documentation

- Document ipc_service_name module with naming convention
  (<subsystem>/<channel>[/<qualifier>]), constraints, and usage table
- Expand HoldTime doc comment with full semantics, when-to-use guidance,
  and edge cases (zero duration, reset behavior)
- Document EdgeWithCooldown::reset() explaining that it anchors a new
  cooldown window rather than returning to the initial state
- Add Send + Sync supertraits to the Debounce trait and remove redundant
  bounds from downstream usages
- Create docs/open-points.md covering three unresolved design items:
  KVS lossy integer casts, PreFailed aging clock reset behavior, and
  MAX_PATH_LENGTH coupling with LongString capacity

Author: bburda42dot

docs/open-points.md

@@ -0,0 +1,85 @@
+# Open Points
+
+Items requiring design decisions or future attention. Each entry includes
+the problem, code location, options considered, and a recommendation.
+
+---
+
+## 1. KVS `as` casts — lossy integer conversions
+
+**Location:** `src/dfm_lib/src/sovd_fault_storage.rs` lines 107-111 (serialize), 132-135 (deserialize)
+
+**Problem:**
+`rust_kvs` only supports `i64` as its numeric type. Fault counters are
+stored as `u32`/`u64`, so serialization uses `as i64` (potentially
+truncating high-bit values) and deserialization uses `as u32`/`as u64`
+(wrapping on negative values, defaulting to 0 on missing keys).
+
+Affected fields:
+- `occurrence_counter` (`u32` → `i64` → `u32`)
+- `aging_counter` (`u32` → `i64` → `u32`)
+- `healing_counter` (`u32` → `i64` → `u32`)
+- `first_occurrence_secs` (`u64` → `i64` → `u64`)
+- `last_occurrence_secs` (`u64` → `i64` → `u64`)
+
+**Options:**
+1. **`TryFrom` with fallback to 0** — safe, but silently loses data on overflow.
+2. **`TryFrom` with `StorageError` propagation** — surfaces the problem to
+
+   the caller; forces handling of corrupted/out-of-range storage.
+
+**Recommendation:** Option 2 — propagate as `StorageError`. Silent
+fallback to 0 can mask data corruption in long-running automotive
+systems where counters may accumulate over thousands of operation
+cycles.
+
+---
+
+## 2. PreFailed resets the aging clock
+
+**Location:** `src/dfm_lib/src/fault_record_processor.rs` line 175 (`mark_aging_active`)
+
+**Problem:**
+When a `PreFailed` event arrives, `mark_aging_active` is called, which
+resets the aging clock to the current operation cycle. This means a
+fault that oscillates between `PreFailed` and `PrePassed` will never
+age out, even if it never reaches a full `Failed` confirmation.
+
+This is the **conservative** (automotive-safe) approach: a fault that is
+still intermittently signaling `PreFailed` should not be silently
+cleared. However, it deviates from a strict reading of ISO 14229 where
+only confirmed DTCs (`Failed`) start the aging counter.
+
+**Options:**
+1. **Keep current behavior** — conservative; any sign of fault activity
+   keeps the DTC alive.
+2. **Only reset aging on `Failed`** — strict ISO 14229 interpretation;
+   `PreFailed` alone does not prevent aging.
+
+**Recommendation:** Option 1 — maintain the current conservative
+behavior. In automotive safety contexts, it is preferable to keep a
+DTC visible longer than to risk clearing it while the underlying
+condition is still intermittently present.
+
+---
+
+## 3. MAX_PATH_LENGTH coupling with LongString capacity
+
+**Location:** `src/fault_lib/src/fault_manager_sink.rs` line 215
+
+**Problem:**
+`MAX_PATH_LENGTH` is a magic constant set to `128`, which must match
+the capacity of `LongString::StaticString<128>` used for IPC. If either
+value changes independently, path validation and IPC serialization will
+disagree, potentially causing silent truncation or runtime panics.
+
+**Options:**
+1. **`const_assert!`** — add a compile-time assertion that
+   `MAX_PATH_LENGTH == LongString::CAPACITY` so any mismatch fails the
+   build.
+2. **Doc comment only** — document the coupling (already done in T55)
+   and rely on code review to keep them in sync.
+
+**Recommendation:** Option 1 if `LongString` exposes a `CAPACITY`
+constant. Otherwise, the existing doc comment (added in T55) is
+sufficient until the upstream type provides a programmatic bound.

src/common/src/debounce.rs

@@ -69,7 +69,7 @@ pub enum DebounceMode {
 
 impl DebounceMode {
     /// Create a boxed [`Debounce`] implementation matching this mode.
-    pub fn into_debouncer(self) -> Box<dyn Debounce + Send + Sync> {
+    pub fn into_debouncer(self) -> Box<dyn Debounce> {
         match self {
             DebounceMode::CountWithinWindow { min_count, window } => Box::new(CountWithinWindow::new(min_count, window.into())),
             DebounceMode::HoldTime { duration } => Box::new(HoldTime::new(duration.into())),
@@ -89,7 +89,10 @@ pub struct DebouncePolicy {
 }
 
 /// Trait for debounce algorithm implementations.
-pub trait Debounce {
+///
+/// All implementations must be [`Send`] + [`Sync`] to support use in
+/// multi-threaded fault-reporting pipelines (e.g. behind `Arc<Mutex<_>>`).
+pub trait Debounce: Send + Sync {
     /// Called on each fault occurrence. Returns true if the event should be reported.
     fn on_event(&mut self, now: Instant) -> bool;
 
@@ -141,6 +144,27 @@ impl Debounce for CountWithinWindow {
 ///
 /// Confirms a fault only after the condition persists continuously for
 /// the configured duration.
+///
+/// # Semantics
+///
+/// - The first call to [`on_event`](Debounce::on_event) starts an internal
+///   timer but returns `false` (not yet confirmed).
+/// - Subsequent calls return `true` only when the elapsed time since the
+///   first event meets or exceeds `duration`.
+/// - If [`reset`](Debounce::reset) is called (e.g. the fault condition
+///   clears), the timer restarts from zero on the next `on_event`.
+///
+/// # When to use
+///
+/// Use `HoldTime` for "stuck-at" faults where a condition must remain
+/// continuously active for a minimum period before it should be reported.
+/// This prevents transient glitches from triggering fault confirmation.
+///
+/// # Edge cases
+///
+/// - A `duration` of zero confirms on the **second** call (the first
+///   call always records the start time and returns `false`).
+/// - Calling `reset` and then immediately `on_event` restarts the timer.
 pub struct HoldTime {
     duration: Duration,
     start_time: Option<Instant>,
@@ -196,6 +220,20 @@ impl Debounce for EdgeWithCooldown {
         }
     }
 
+    /// Reset the edge-with-cooldown debouncer.
+    ///
+    /// Sets `last_report` to `now`, which means the cooldown window
+    /// restarts from this instant. Any `on_event` call arriving before
+    /// `now + cooldown` will be suppressed.
+    ///
+    /// This is the correct behavior when the underlying fault clears:
+    /// the reporter should not immediately re-fire on the next edge.
+    ///
+    /// # Difference from other debouncers
+    ///
+    /// Unlike [`HoldTime::reset`] (which clears the timer entirely),
+    /// this method *anchors* a new cooldown window — the debouncer
+    /// does **not** return to the "never reported" initial state.
     fn reset(&mut self, now: Instant) {
         self.last_report = Some(now);
     }

src/common/src/ipc_service_name.rs

@@ -10,9 +10,49 @@
 // SPDX-License-Identifier: Apache-2.0
 //
 
+//! IPC service name constants for iceoryx2 publish-subscribe channels.
+//!
+//! # Naming convention
+//!
+//! All service names use a hierarchical slash-separated format:
+//!
+//! ```text
+//! <subsystem>/<channel>[/<qualifier>]
+//! ```
+//!
+//! - **`<subsystem>`** — logical component, e.g. `dfm` (Diagnostic Fault Manager).
+//! - **`<channel>`** — communication purpose, e.g. `event`, `enabling_condition`.
+//! - **`<qualifier>`** — optional sub-channel, e.g. `hash/response`, `notification`.
+//!
+//! # Constraints
+//!
+//! - Must be valid iceoryx2 [`ServiceName`](iceoryx2::prelude::ServiceName) values.
+//! - Maximum length is 255 bytes (iceoryx2 limit).
+//! - Characters allowed: alphanumeric, `/`, `_`, `-`, `.`.
+//! - Must not start or end with `/`.
+//!
+//! # Examples
+//!
+//! | Constant | Value | Direction |
+//! |----------|-------|-----------|
+//! | `DIAGNOSTIC_FAULT_MANAGER_EVENT_SERVICE_NAME` | `dfm/event` | reporter → DFM |
+//! | `DIAGNOSTIC_FAULT_MANAGER_HASH_CHECK_RESPONSE_SERVICE_NAME` | `dfm/event/hash/response` | DFM → reporter |
+//! | `ENABLING_CONDITION_NOTIFICATION_SERVICE_NAME` | `dfm/enabling_condition/notification` | DFM → reporters |
+
 /// Iceoryx2 service name for the main diagnostic-event channel (reporter → DFM).
+///
+/// Reporters publish [`DiagnosticEvent`](crate::types::DiagnosticEvent) messages on this
+/// channel. The Diagnostic Fault Manager subscribes and processes them.
 pub const DIAGNOSTIC_FAULT_MANAGER_EVENT_SERVICE_NAME: &str = "dfm/event";
+
 /// Iceoryx2 service name for hash-check responses (DFM → reporter).
+///
+/// After a reporter registers, the DFM replies on this channel with a
+/// hash-check response confirming catalog consistency.
 pub const DIAGNOSTIC_FAULT_MANAGER_HASH_CHECK_RESPONSE_SERVICE_NAME: &str = "dfm/event/hash/response";
+
 /// Iceoryx2 service name for enabling-condition notifications (DFM → reporters).
+///
+/// The DFM publishes [`EnablingConditionNotification`](crate::EnablingConditionNotification)
+/// messages whenever an enabling condition changes state.
 pub const ENABLING_CONDITION_NOTIFICATION_SERVICE_NAME: &str = "dfm/enabling_condition/notification";

src/dfm_lib/src/fault_record_processor.rs

@@ -55,7 +55,7 @@ pub struct FaultRecordProcessor<S: SovdFaultStateStorage> {
     storage: Arc<S>,
     catalog_registry: Arc<FaultCatalogRegistry>,
     /// Per-source, per-fault debounce state. Lazily populated from catalog on first event.
-    debouncers: HashMap<FaultKey, Box<dyn Debounce + Send>>,
+    debouncers: HashMap<FaultKey, Box<dyn Debounce>>,
     /// Tracks last confirmed (post-debounce) lifecycle stage per fault key.
     /// Used to detect transitions that should reset the debouncer.
     last_stages: HashMap<FaultKey, LifecycleStage>,
@@ -217,7 +217,7 @@ impl<S: SovdFaultStateStorage> FaultRecordProcessor<S> {
     /// Lazily looks up the debouncer for a fault key, creating it from
     /// the catalog's `manager_side_debounce` config on first access.
     /// Returns `None` when no manager-side debounce is configured.
-    fn get_or_create_debouncer(&mut self, key: &FaultKey, fault_id: &FaultId) -> Option<&mut Box<dyn Debounce + Send>> {
+    fn get_or_create_debouncer(&mut self, key: &FaultKey, fault_id: &FaultId) -> Option<&mut Box<dyn Debounce>> {
         if self.debouncers.contains_key(key) {
             return self.debouncers.get_mut(key);
         }

src/fault_lib/src/reporter.rs

@@ -39,7 +39,7 @@ pub struct Reporter {
     /// Runtime debounce state derived from `descriptor.reporter_side_debounce`.
     /// When `Some`, `publish()` filters events through the debouncer before
     /// forwarding to the sink, reducing IPC traffic to the DFM.
-    pub(crate) debouncer: Option<Box<dyn Debounce + Send + Sync>>,
+    pub(crate) debouncer: Option<Box<dyn Debounce>>,
     /// Tracks the last published lifecycle stage so that stage transitions
     /// (e.g. Passed → Failed) can reset the debouncer.
     pub(crate) last_stage: LifecycleStage,