Fix tests and add CI for Rust crates (#54)

* Fix tests and add CI for Rust crates

* Format crates

* Work around an error of incomplete code

* Disable yamcs integration tests in coverage

Author: Kronos3

.github/workflows/build-rust-crates.yml

@@ -0,0 +1,116 @@
+name: Build and Test Rust Crates
+
+on:
+  push:
+    branches:
+      - main
+    paths:
+      - 'crates/**'
+      - 'Cargo.toml'
+      - 'Cargo.lock'
+      - '.github/workflows/build-rust-crates.yml'
+  pull_request:
+    paths:
+      - 'crates/**'
+      - 'Cargo.toml'
+      - 'Cargo.lock'
+      - '.github/workflows/build-rust-crates.yml'
+  workflow_dispatch:
+
+jobs:
+  test:
+    name: Test Rust Crates
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest]
+        rust: [stable]
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: ${{ matrix.rust }}
+          components: rustfmt, clippy
+
+      - name: Install Protocol Buffers Compiler
+        uses: arduino/setup-protoc@v3
+        with:
+          version: "25.x"
+          repo-token: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Cache cargo registry
+        uses: actions/cache@v4
+        with:
+          path: ~/.cargo/registry
+          key: ${{ runner.os }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-cargo-registry-
+
+      - name: Cache cargo index
+        uses: actions/cache@v4
+        with:
+          path: ~/.cargo/git
+          key: ${{ runner.os }}-cargo-git-${{ hashFiles('**/Cargo.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-cargo-git-
+
+      - name: Cache cargo build
+        uses: actions/cache@v4
+        with:
+          path: target
+          key: ${{ runner.os }}-cargo-build-target-${{ hashFiles('**/Cargo.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-cargo-build-target-
+
+      - name: Build all crates
+        run: cargo build --workspace --all-features --verbose
+
+      - name: Check formatting
+        run: cargo fmt --all -- --check
+
+      - name: Run clippy
+        run: cargo clippy --all-targets --all-features --no-deps -- -W clippy::all
+        continue-on-error: true
+
+      - name: Run unit tests
+        run: cargo test --workspace --all-features --lib --verbose
+
+      - name: Run integration tests (excluding yamcs-http)
+        run: cargo test --workspace --all-features --test '*' --exclude yamcs-http --verbose
+
+      - name: Run doc tests
+        run: cargo test --workspace --all-features --doc --verbose
+
+  coverage:
+    name: Code Coverage
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Install Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+
+      - name: Install Protocol Buffers Compiler
+        uses: arduino/setup-protoc@v3
+        with:
+          version: "25.x"
+          repo-token: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Install cargo-tarpaulin
+        run: cargo install cargo-tarpaulin
+
+      - name: Generate coverage
+        run: cargo tarpaulin --workspace --all-features --exclude yamcs-http --out xml --timeout 300
+
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v4
+        with:
+          files: ./cobertura.xml
+          flags: rust
+          fail_ci_if_error: false

crates/hermes-data/src/de/context.rs

@@ -131,7 +131,11 @@ impl<'a> Context<'a> {
     pub(crate) fn get_bits(&mut self, num_bits: usize, order: ByteOrder) -> Result<u64> {
         assert!(num_bits <= 64, "Invalid bit count {}", num_bits);
 
-        if (self.position + num_bits) / 8 + 1 >= self.data.len() {
+        let end_pos = self.position + num_bits;
+
+        if (end_pos % 8 == 0) && (end_pos / 8) > self.data.len() {
+            Err(Error::Eos)
+        } else if (end_pos % 8 > 0) && (end_pos / 8) >= self.data.len() {
             Err(Error::Eos)
         } else {
             let r = self.data.get(self.position, num_bits, order);

crates/hermes-data/src/de/expr.rs

@@ -1,5 +1,6 @@
 use crate::de::Context;
 use crate::de::ParameterRefOrValue;
+use crate::error::InvalidComparison;
 use crate::{Error, ParameterInstanceRef, Value};
 use tracing::warn;
 
@@ -89,23 +90,23 @@ fn comparison(
         (Value::Enumerated(l), Value::Enumerated(r)) => Ok(builtin_comparison(op, l, r)),
 
         // TODO(tumbar) We can probably implement more comparisons
-        (_, _) => Err(Error::InvalidComparison(
-            op.clone(),
-            left.clone(),
-            right.clone(),
-        )), // (Value::Array(l), Value::Array(r)) => {
-            //     if l.len() != r.len() || *op != hermes_xtce::ComparisonOperatorsType::Eq {
-            //         Err(Error::InvalidComparison(
-            //             op.clone(),
-            //             left.clone(),
-            //             right.clone(),
-            //         ))
-            //     } else {
-            //         Ok(l.iter()
-            //             .zip(r)
-            //             .all(|(l, r)| comparison(&hermes_xtce::ComparisonOperatorsType::Eq, left, right)))
-            //     }
-            // }
+        (_, _) => Err(Error::InvalidComparison(Box::new(InvalidComparison {
+            op: op.clone(),
+            left: left.clone(),
+            right: right.clone(),
+        }))), // (Value::Array(l), Value::Array(r)) => {
+              //     if l.len() != r.len() || *op != hermes_xtce::ComparisonOperatorsType::Eq {
+              //         Err(Error::InvalidComparison(
+              //             op.clone(),
+              //             left.clone(),
+              //             right.clone(),
+              //         ))
+              //     } else {
+              //         Ok(l.iter()
+              //             .zip(r)
+              //             .all(|(l, r)| comparison(&hermes_xtce::ComparisonOperatorsType::Eq, left, right)))
+              //     }
+              // }
     }
 }
 

crates/hermes-data/src/error.rs

@@ -1,10 +1,24 @@
+use std::fmt::Display;
 use thiserror::Error;
 
 use crate::Value;
 
 /// Result type alias for hermes-data operations
 pub type Result<T> = std::result::Result<T, Error>;
 
+#[derive(Debug)]
+pub struct InvalidComparison {
+    pub op: hermes_xtce::ComparisonOperatorsType,
+    pub left: Value,
+    pub right: Value,
+}
+
+impl Display for InvalidComparison {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_fmt(format_args!("{} {} {}", self.left, self.op, self.right))
+    }
+}
+
 /// Error types for hermes-data operations
 #[derive(Error, Debug)]
 pub enum Error {
@@ -44,8 +58,8 @@ pub enum Error {
     #[error("Operation Not Implemented: {0}")]
     NotImplemented(&'static str),
 
-    #[error("Invalid comparison '{0}' between values: {1}, {2}")]
-    InvalidComparison(hermes_xtce::ComparisonOperatorsType, Value, Value),
+    #[error("Invalid comparison: {0}")]
+    InvalidComparison(Box<InvalidComparison>),
 
     #[error("Mismatching checksum, computed {0}, received {1}")]
     ChecksumMismatch(u16, u16),

crates/hermes-data/src/lib.rs

@@ -3,16 +3,16 @@ mod command;
 pub mod de;
 mod error;
 mod framing;
+mod item;
 mod types;
 mod util;
 mod xtce;
-mod item;
 
 pub use calibrator::*;
 pub use error::{Error, Result};
 pub use framing::*;
-pub use types::*;
 pub use item::*;
+pub use types::*;
 
 use std::collections::HashMap;
 use std::sync::Arc;

crates/hermes-data/src/xtce/collection.rs

@@ -1,7 +1,6 @@
 /// Functions for traversing the XTCE SpaceSystem tree and collecting items
 use std::collections::HashMap;
 
-
 use super::utils::make_qualified_name;
 
 /// Intermediate structure holding a container during Pass 1 before dependencies are resolved

crates/hermes-data/src/xtce/construction.rs

@@ -2,18 +2,17 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
-
 use super::collection::{UnresolvedContainer, UnresolvedParameter, UnresolvedParameterType};
 use super::conversion::convert_restriction_criteria;
 use super::resolution::{resolve_container_reference, resolve_parameter_type_name};
 use crate::de::{RestrictionCriteria, SequenceContainerType};
-use crate::{Item, Parameter};
 use crate::xtce::container::convert_entry;
 use crate::xtce::types::{
     convert_parameter_type_set, convert_parameter_type_set_with_context,
     convert_parameter_type_set_with_parameters,
 };
 use crate::{Error, Result};
+use crate::{Item, Parameter};
 
 /// Construct parameter types in multiple passes:
 /// Pass 1: Simple types (int, float, string, bool, enum, time) - no dependencies
@@ -70,35 +69,68 @@ pub(crate) fn construct_parameter_types_pass1(
 }
 
 /// Pass 2: Construct Aggregate types (after simple types are available)
+/// This uses multi-pass construction to handle dependencies between aggregate types.
 pub(crate) fn construct_parameter_types_pass2_aggregates(
     deferred_aggregate_types: Vec<(String, UnresolvedParameterType)>,
     completed: &mut HashMap<String, Arc<crate::Type>>,
 ) -> Result<()> {
-    for (qualified_name, unresolved_type) in deferred_aggregate_types {
-        match convert_parameter_type_set_with_context(
-            &unresolved_type.xml,
-            &unresolved_type.space_system_path,
-            &completed,
-        ) {
-            Ok(type_) => {
-                completed.insert(qualified_name, Arc::new(type_));
-            }
-            Err(Error::NotImplemented(msg)) => {
-                tracing::warn!(
-                    "Skipping unsupported parameter type '{}': {}",
-                    qualified_name,
-                    msg
-                );
+    let mut remaining = deferred_aggregate_types;
+    let mut failed_permanently = Vec::new();
+
+    // Keep trying until we make no more progress
+    while !remaining.is_empty() {
+        let mut still_deferred = Vec::new();
+        let mut made_progress = false;
+
+        for (qualified_name, unresolved_type) in remaining {
+            match convert_parameter_type_set_with_context(
+                &unresolved_type.xml,
+                &unresolved_type.space_system_path,
+                &completed,
+            ) {
+                Ok(type_) => {
+                    completed.insert(qualified_name, Arc::new(type_));
+                    made_progress = true;
+                }
+                Err(Error::NotImplemented(msg)) => {
+                    // These are permanently unsupported types
+                    tracing::warn!(
+                        "Skipping unsupported parameter type '{}': {}",
+                        qualified_name,
+                        msg
+                    );
+                    failed_permanently.push(qualified_name);
+                }
+                Err(Error::InvalidXtce(ref msg)) if msg.contains("not found") => {
+                    // This is likely a missing dependency - defer for next pass
+                    still_deferred.push((qualified_name, unresolved_type));
+                }
+                Err(e) => {
+                    // Other errors should be logged but not retried
+                    tracing::warn!(
+                        "Failed to construct aggregate type '{}': {}",
+                        qualified_name,
+                        e
+                    );
+                    failed_permanently.push(qualified_name);
+                }
             }
-            Err(e) => {
+        }
+
+        // If we didn't make progress and still have deferred types, they must have unresolvable dependencies
+        if !made_progress && !still_deferred.is_empty() {
+            for (qualified_name, _) in still_deferred {
                 tracing::warn!(
-                    "Failed to construct aggregate type '{}': {}",
-                    qualified_name,
-                    e
+                    "Failed to construct aggregate type '{}': unresolvable type dependencies",
+                    qualified_name
                 );
             }
+            break;
         }
+
+        remaining = still_deferred;
     }
+
     Ok(())
 }
 

crates/hermes-data/src/xtce/container.rs

@@ -1,7 +1,7 @@
+use crate::Parameter;
 use crate::de::{
     ContainerRef, EntryKind, EntryType, LocationInContainerInBits, ReferenceLocation, Repeat,
 };
-use crate::Parameter;
 
 use crate::{Error, IntegerValue, ParameterInstanceRef, ParameterRef};
 

crates/hermes-data/src/xtce/conversion.rs

@@ -2,11 +2,10 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
-
+use crate::Parameter;
 use crate::de::{
     BooleanExpression, Comparison, ComparisonCheck, ParameterRefOrValue, RestrictionCriteria,
 };
-use crate::Parameter;
 
 use crate::{Error, ParameterInstanceRef, ParameterRef, Result, Value};
 

crates/hermes-data/src/xtce/mod.rs

@@ -17,7 +17,7 @@ mod utils;
 
 // Re-export public types and functions
 pub(crate) use collection::{
-    collect_containers, collect_parameter_types, collect_parameters, UnresolvedContainer,
+    UnresolvedContainer, collect_containers, collect_parameter_types, collect_parameters,
 };
 pub(crate) use construction::{
     build_dependency_graph, construct_containers, construct_parameter_types_pass1,