feat(hal): add composable I2C hardware abstraction traits and clarify layering

.github/workflows/ci.yml

@@ -115,7 +115,7 @@ jobs:
 
     - name: Run security-focused clippy lints
       run: |
-        cargo clippy --all-targets --all-features -- \
+        cargo clippy --all-targets --all-features --workspace --exclude openprot-platform-mock -- \
           -D warnings \
           -W clippy::arithmetic_side_effects \
           -W clippy::float_arithmetic \
@@ -125,7 +125,24 @@ jobs:
           -W clippy::panic \
           -W clippy::mem_forget \
           -W clippy::multiple_unsafe_ops_per_block \
-          -W clippy::undocumented_unsafe_blocks
+          -W clippy::undocumented_unsafe_blocks \
+          -A clippy::assertions_on_constants \
+          -A clippy::needless_return
+
+    - name: Run strict security lints on non-test code
+      run: |
+        cargo clippy --lib --bins --workspace --exclude openprot-platform-mock -- \
+          -D warnings \
+          -D clippy::arithmetic_side_effects \
+          -D clippy::float_arithmetic \
+          -D clippy::indexing_slicing \
+          -D clippy::unwrap_used \
+          -D clippy::expect_used \
+          -D clippy::panic \
+          -D clippy::mem_forget \
+          -D clippy::multiple_unsafe_ops_per_block \
+          -D clippy::undocumented_unsafe_blocks \
+          -D clippy::assertions_on_constants
 
     - name: Run semgrep security scan
       uses: returntocorp/semgrep-action@v1

Cargo.toml

@@ -23,6 +23,9 @@ repository = "https://github.com/rusty1968/openprot"
 edition = "2021"
 
 [workspace.dependencies]
-zerocopy = { version = "0.8.27", features = ["derive"] }
+zerocopy = { version = "0.8", features = ["derive"] }
 zeroize = { version = "1.7", default-features = false, features = ["derive"] }
-subtle = { version = "2.5", default-features = false }
+subtle = { version = "2", default-features = false }
+# Pin to match Hubris ecosystem
+rand_core = { version = "0.6", default-features = false }
+embedded-hal = "1.0"

hal/blocking/Cargo.toml

@@ -8,8 +8,8 @@ description = "Blocking/synchronous HAL traits for OpenPRoT"
 license = "Apache-2.0"
 
 [dependencies]
-embedded-hal = "1.0"
 zerocopy = { workspace = true }
 zeroize = { workspace = true }
 subtle = { workspace = true }
-rand_core = "0.9"
+rand_core = { workspace = true }
+embedded-hal = { workspace = true }

hal/blocking/src/ecdsa.rs

@@ -96,9 +96,9 @@
 
 use crate::digest::DigestAlgorithm;
 use core::fmt::Debug;
+use zerocopy::Immutable;
 use zerocopy::{FromBytes, IntoBytes};
 use zeroize::Zeroize;
-
 /// Trait for converting implementation-specific ECDSA errors into generic error kinds.
 ///
 /// This trait allows HAL implementations to define their own detailed error types
@@ -818,6 +818,93 @@ impl Curve for P384 {
     type Scalar = [u8; 48];
 }
 
+/// A serializable public key for the P384 elliptic curve.
+///
+/// This implementation provides both coordinate access and serialization
+/// capabilities for P384 public keys, supporting the standard 96-byte
+/// uncompressed format (48 bytes each for x and y coordinates).
+#[derive(Clone, Debug, IntoBytes, FromBytes, Immutable)]
+#[repr(C)]
+pub struct P384PublicKey {
+    /// X coordinate (48 bytes for P384)
+    x: [u8; 48],
+    /// Y coordinate (48 bytes for P384)  
+    y: [u8; 48],
+}
+
+impl P384PublicKey {
+    /// Create a new P384 public key from raw coordinates
+    pub fn new(x: [u8; 48], y: [u8; 48]) -> Self {
+        Self { x, y }
+    }
+}
+
+impl PublicKey<P384> for P384PublicKey {
+    fn coordinates(
+        &self,
+        x_out: &mut <P384 as Curve>::Scalar,
+        y_out: &mut <P384 as Curve>::Scalar,
+    ) {
+        // P384::Scalar is [u8; 48], so we can copy directly
+        *x_out = self.x;
+        *y_out = self.y;
+    }
+
+    fn from_coordinates(
+        x: <P384 as Curve>::Scalar,
+        y: <P384 as Curve>::Scalar,
+    ) -> Result<Self, ErrorKind> {
+        Ok(Self::new(x, y))
+    }
+}
+
+impl SerializablePublicKey<P384> for P384PublicKey {}
+
+/// A serializable signature for the P384 elliptic curve.
+///
+/// This implementation provides both signature validation and serialization
+/// capabilities for P384 ECDSA signatures, supporting the standard 96-byte
+/// format (48 bytes each for r and s components).
+#[derive(Clone, Debug, IntoBytes, FromBytes, Immutable)]
+#[repr(C)]
+pub struct P384Signature {
+    /// R component (48 bytes for P384)
+    r: [u8; 48],
+    /// S component (48 bytes for P384)
+    s: [u8; 48],
+}
+
+impl P384Signature {
+    /// Create a new P384 signature from r and s components
+    pub fn new(r: [u8; 48], s: [u8; 48]) -> Self {
+        Self { r, s }
+    }
+}
+
+impl Signature<P384> for P384Signature {
+    fn from_coordinates(
+        r: <P384 as Curve>::Scalar,
+        s: <P384 as Curve>::Scalar,
+    ) -> Result<Self, ErrorKind> {
+        // TODO: Add proper signature validation here
+        // For now, we accept any r,s values but in a real implementation
+        // we should validate that 1 ≤ r,s < curve_order
+        Ok(Self::new(r, s))
+    }
+
+    fn coordinates(
+        &self,
+        r_out: &mut <P384 as Curve>::Scalar,
+        s_out: &mut <P384 as Curve>::Scalar,
+    ) {
+        // P384::Scalar is [u8; 48], so we can copy directly
+        *r_out = self.r;
+        *s_out = self.s;
+    }
+}
+
+impl SerializableSignature<P384> for P384Signature {}
+
 /// NIST P-521 elliptic curve marker type.
 ///
 /// This zero-sized type represents the NIST P-521 elliptic curve (secp521r1).

hal/blocking/src/i2c_target.rs → hal/blocking/src/i2c_device.rs

@@ -1,5 +1,116 @@
 // Licensed under the Apache-2.0 license
 
+//! # I2C Device Implementation Traits
+//!
+//! This module provides application-level traits for implementing I2C devices such as sensors,
+//! EEPROMs, display controllers, ADCs, DACs, and other peripheral devices that respond
+//! to I2C master/controller requests.
+//!
+//! ## Purpose
+//!
+//! These traits focus on the **application logic** of how devices should respond to I2C
+//! transactions initiated by a master/controller. They are **not** for controlling I2C
+//! hardware controllers - that's handled by hardware abstraction layer (HAL) traits.
+//!
+//! ## Layer Distinction
+//!
+//! ```text
+//! ┌─────────────────────────────────────────────────────────────────┐
+//! │                    Application Layer                            │
+//! │  i2c_device.rs - Implement device behavior (this module)       │
+//! │  Examples: Sensor readings, EEPROM storage, register access    │
+//! ├─────────────────────────────────────────────────────────────────┤
+//! │                Hardware Abstraction Layer                      │
+//! │  i2c_hardware.rs - Control I2C controller peripherals          │
+//! │  Examples: Buffer management, interrupt handling, bus control   │
+//! └─────────────────────────────────────────────────────────────────┘
+//! ```
+//!
+//! ## Design Philosophy
+//!
+//! The traits in this module are designed around **device behavior patterns**:
+//!
+//! - **Event-driven**: Devices respond to master-initiated transactions
+//! - **Callback-based**: Use `on_*` methods to handle different transaction types
+//! - **Stateful**: Devices can maintain internal state between transactions
+//! - **Protocol-aware**: Support common I2C device patterns (register access, etc.)
+//!
+//! ## Common Use Cases
+//!
+//! ### Temperature Sensor
+//! ```rust,ignore
+//! struct TemperatureSensor {
+//!     temperature: f32,
+//!     address: u8,
+//! }
+//!
+//! impl I2CCoreTarget for TemperatureSensor {
+//!     fn on_read(&mut self, buffer: &mut [u8]) -> Result<usize, Self::Error> {
+//!         let temp_bytes = self.temperature.to_le_bytes();
+//!         buffer[..4].copy_from_slice(&temp_bytes);
+//!         Ok(4)
+//!     }
+//! }
+//! ```
+//!
+//! ### EEPROM Device
+//! ```rust,ignore
+//! struct EepromDevice {
+//!     memory: [u8; 256],
+//!     address_pointer: u8,
+//! }
+//!
+//! impl WriteTarget for EepromDevice {
+//!     fn on_write(&mut self, data: &[u8]) -> Result<(), Self::Error> {
+//!         if let Some(&addr) = data.first() {
+//!             self.address_pointer = addr;
+//!             // Write remaining data to memory starting at addr
+//!             for (i, &byte) in data[1..].iter().enumerate() {
+//!                 if let Some(mem_slot) = self.memory.get_mut(addr as usize + i) {
+//!                     *mem_slot = byte;
+//!                 }
+//!             }
+//!         }
+//!         Ok(())
+//!     }
+//! }
+//!
+//! impl ReadTarget for EepromDevice {
+//!     fn on_read(&mut self, buffer: &mut [u8]) -> Result<usize, Self::Error> {
+//!         let start = self.address_pointer as usize;
+//!         let end = (start + buffer.len()).min(self.memory.len());
+//!         let bytes_to_read = end - start;
+//!         buffer[..bytes_to_read].copy_from_slice(&self.memory[start..end]);
+//!         Ok(bytes_to_read)
+//!     }
+//! }
+//! ```
+//!
+//! ## Trait Overview
+//!
+//! The traits are organized in a composable hierarchy:
+//!
+//! - [`I2CCoreTarget`]: Core transaction lifecycle (required for all devices)
+//! - [`WriteTarget`]: Handle write operations from master
+//! - [`ReadTarget`]: Handle read operations from master  
+//! - [`WriteReadTarget`]: Handle combined write-read transactions
+//! - [`RegisterAccess`]: Higher-level register-based access patterns
+//! - [`I2CTarget`]: Convenience trait combining all capabilities
+//!
+//! ## Transaction Flow
+//!
+//! A typical I2C transaction from the device perspective:
+//!
+//! 1. **Address Match**: `on_address_match(address)` - Should this device respond?
+//! 2. **Transaction Start**: `on_transaction_start(direction, repeated)` - Initialize for transaction
+//! 3. **Data Phase**: `on_write(data)` or `on_read(buffer)` - Handle the actual data
+//! 4. **Transaction End**: `on_stop()` - Clean up after transaction
+//!
+//! ## Error Handling
+//!
+//! All traits use associated `Error` types that must implement `embedded_hal::i2c::Error`,
+//! providing standard I2C error conditions while allowing device-specific error extensions.
+
 #![allow(clippy::doc_overindented_list_items)]
 use embedded_hal::i2c::ErrorType as I2CErrorType;
 
@@ -20,32 +131,46 @@ pub enum TransactionDirection {
     Read,
 }
 
-/// A convenience trait alias that represents a fully-featured I2C target device.
+/// A convenience trait alias that represents a fully-featured I2C device implementation.
 ///
-/// This trait combines all the core and extended I2C target traits into a single interface:
+/// This trait combines all the core and extended I2C device traits into a single interface:
 ///
 /// - [`I2CCoreTarget`]: Handles transaction lifecycle and address matching.
-/// - [`ReadTarget`]: Supports reading data from the target.
-/// - [`WriteTarget`]: Supports writing data to the target.
+/// - [`ReadTarget`]: Supports reading data from the device.
+/// - [`WriteTarget`]: Supports writing data to the device.
 /// - [`WriteReadTarget`]: Supports combined write-read transactions.
 /// - [`RegisterAccess`]: Supports register-level read/write operations.
 ///
-/// Implementing this trait means the device is capable of handling all standard I2C target behaviors,
+/// Implementing this trait means the device is capable of handling all standard I2C device behaviors,
 /// making it suitable for use in generic drivers or frameworks that require full I2C functionality.
 ///
-/// # Example
+/// ## When to Use
+///
+/// Use this trait when you need a device that supports all I2C interaction patterns:
+/// - Simple read/write operations
+/// - Register-based access (common for sensors, configuration devices)
+/// - Complex transaction sequences
+/// - Generic device drivers that work with multiple device types
+///
+/// ## Implementation Pattern
+///
+/// Most implementations will implement the individual traits and get `I2CTarget` automatically:
+///
 /// ```rust,ignore
-/// struct MyDevice { /* ... */ }
+/// struct MyComplexDevice {
+///     registers: [u8; 256],
+///     current_register: u8,
+/// }
 ///
-/// impl I2CCoreTarget for MyDevice { /* ... */ }
-/// impl ReadTarget for MyDevice { /* ... */ }
-/// impl WriteTarget for MyDevice { /* ... */ }
-/// impl WriteReadTarget for MyDevice { /* ... */ }
-/// impl RegisterAccess for MyDevice { /* ... */ }
+/// impl I2CCoreTarget for MyComplexDevice { /* ... */ }
+/// impl ReadTarget for MyComplexDevice { /* ... */ }
+/// impl WriteTarget for MyComplexDevice { /* ... */ }
+/// impl WriteReadTarget for MyComplexDevice { /* ... */ }
+/// impl RegisterAccess for MyComplexDevice { /* ... */ }
 ///
-/// // Now MyDevice automatically implements FullI2CTarget
-/// fn use_device<T: I2CTarget>(dev: &mut T) {
-///     // Use all I2C capabilities
+/// // Now MyComplexDevice automatically implements I2CTarget
+/// fn use_any_device<T: I2CTarget>(device: &mut T) {
+///     // Can use all I2C device capabilities
 /// }
 /// ```
 pub trait I2CTarget:
@@ -58,11 +183,27 @@ impl<T> I2CTarget for T where
 {
 }
 
-/// Trait representing a target (slave) I2C device behavior.
+/// Trait representing core I2C device behavior.
+///
+/// This trait defines the fundamental methods that an I2C device must implement to handle
+/// transactions initiated by an I2C master/controller. It covers the essential lifecycle
+/// of I2C transactions from the device's perspective.
+///
+/// ## Core Responsibilities
+///
+/// - **Address matching**: Decide whether to respond to a specific I2C address
+/// - **Transaction initialization**: Set up device state for incoming transactions  
+/// - **Transaction lifecycle**: Handle start conditions, repeated starts, and stop conditions
+///
+/// ## Implementation Notes
+///
+/// This trait focuses on **device logic**, not hardware control. Implementations should:
+/// - Maintain device state (registers, memory, sensor readings, etc.)
+/// - Implement device-specific address matching logic
+/// - Handle transaction setup and teardown
+/// - Prepare for data exchange (actual data handling is in `ReadTarget`/`WriteTarget`)
 ///
-/// This trait defines the core methods that an I2C target device must implement to handle
-/// transactions initiated by an I2C master. It includes methods for handling stop conditions,
-/// transaction starts, and address match events.
+/// For hardware I2C controller management, use hardware abstraction layer traits instead.
 pub trait I2CCoreTarget: I2CErrorType {
     /// Initialize the target with a specific address.
     fn init(&mut self, address: u8) -> Result<(), Self::Error>;