From 24c5702b5f216f339543a22f68dc569d02c500af Mon Sep 17 00:00:00 2001 From: patrick kenneally Date: Fri, 1 May 2026 12:14:22 -0600 Subject: [PATCH 1/4] Document design goals at top of eigenSupport The conventions used throughout this header (compile-time shape enforcement, row-major C-array layout, MatrixBase on output side, raw const pointers on input side, etc.) were already in force but only implicit. State them explicitly so future contributions stay consistent and so the deliberate FSW vs simulation tradeoff between fixed-size and dynamic-size variants is recorded in source. --- src/architecture/utilities/eigenSupport.h | 64 +++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/architecture/utilities/eigenSupport.h b/src/architecture/utilities/eigenSupport.h index 54f8b21f28..1e554b4032 100644 --- a/src/architecture/utilities/eigenSupport.h +++ b/src/architecture/utilities/eigenSupport.h @@ -11,6 +11,70 @@ #include #include +// ============================================================================= +// Design goals for this header +// ============================================================================= +// +// This header bridges Eigen types and plain C arrays at the boundary with +// Ada/SWIG/message-buffer code. The functions below already follow a small +// set of conventions; the rules are stated here so that future contributions +// stay consistent and so deviations are easy to spot. +// +// 1. Two directions, two parameter conventions. +// Output-side functions (Eigen -> C array) take the destination as a sized +// array reference `T (&out)[N]` so the buffer length is deduced and bounds +// are checked at compile time. Input-side functions (C array -> Eigen) +// take a `const ScalarT*` because callers commonly hand them stride-indexed +// offsets into larger buffers (e.g. `&GsMatrix_B[i * 3]`) or struct member +// arrays - array-reference parameters would force those callers into +// awkward casts. The `cArrayToEigenVector` overload is the exception (its +// size is part of the contract) and accepts `const ScalarT (&)[size]`. +// +// 2. Compile-time shape enforcement when the type carries it. +// Fixed-size variants (`eigenMatrixToCArray`, `eigenMatrixToCArray2D`, +// `eigenVectorToCArray`, `cArrayToEigenMatrix`, `cArrayToEigenVector3`, +// `cArrayToEigenMatrix3`, `c2DArrayToEigenMatrix3`, `eigenTilde`) use +// `static_assert` on `RowsAtCompileTime` / `ColsAtCompileTime` and on +// destination size. Compile-time fixed sizes are regularly valuable in +// embedded / flight-software contexts - they enable static stack +// reasoning, eliminate heap allocation, and surface shape mismatches +// before the binary leaves the developer's machine - and the FSW side +// of this codebase prefers them by default. Dynamic-size variants (named +// with an `X` infix: `eigenMatrixXToCArray`, `eigenMatrixXToCArray2D`, +// `eigenMatrixXInsertCArray`, `cArrayToEigenMatrixX`) fall back to runtime +// checks and `std::terminate()` on shape or capacity violations. The +// dynamic variants exist for host-PC / Xmera simulation modules where +// shapes legitimately depend on runtime configuration (variable-length +// sensor arrays, scenario-driven reaction wheel counts, etc.) and the +// FSW compile-time guarantees aren't applicable. +// +// 3. Row-major C array convention, regardless of Eigen storage order. +// All matrix <-> C-array conversions read and write row-major. Column- +// major Eigen inputs are transposed internally; row-major inputs go +// through unchanged. Callers don't need to reason about Eigen's default +// storage order. +// +// 4. Accept any Eigen expression on the output side. +// Output-side functions take `const Eigen::MatrixBase&`, not +// concrete `Eigen::Matrix`/`Vector`. This admits `Zero()`, `Ones()`, +// `Constant(...)`, `transpose()`, `block<...>()`, and segment views in +// addition to plain matrix/vector variables. Internal evaluation to a +// `PlainObject` handles non-contiguous expressions safely. +// +// 5. Const correctness on inputs. +// Input parameters are const-qualified (`const ScalarT*`, +// `const ScalarT (&)[N]`, `const Eigen::MatrixBase&`). +// `const`-qualified C arrays must be acceptable - regression tests in +// `tests/test_eigenSupport.cpp` enforce this for each input-side +// function, so removing `const` somewhere will fail to compile. +// +// 6. Fail loudly. +// Compile-time violations use `static_assert` with a message identifying +// which constraint failed. Runtime violations on dynamic variants call +// `std::terminate()` rather than throwing or silently truncating. +// +// ============================================================================= + template inline constexpr bool is_row_major_v = (Eigen::internal::traits::Flags & Eigen::RowMajorBit) != 0; From 9b0f01e65a6b4ea7e124fd4a1a8eed9baae39b38 Mon Sep 17 00:00:00 2001 From: patrick kenneally Date: Fri, 1 May 2026 12:18:44 -0600 Subject: [PATCH 2/4] Accept Eigen expressions in eigenVectorToCArray The previous signature took `const Eigen::Vector&`, which rejected Eigen expression types like `Vector3f::Zero()` and `block<>()` even though every other output-side function in this header already accepts `MatrixBase`. The destination is also tightened from a raw `ScalarT*` to a sized array reference `Scalar (&)[size]`, matching `eigenMatrixToCArray` and giving the same compile-time bounds guarantee. All existing call sites pass fixed-size message struct fields or stack arrays, so none need to change. Add EigenVectorToCArrayAcceptsExpression covering both `Zero()` and a block expression (the PlainObject evaluation path). --- src/architecture/utilities/eigenSupport.h | 28 +++++++++++++------ .../utilities/tests/test_eigenSupport.cpp | 26 +++++++++++++++-- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/architecture/utilities/eigenSupport.h b/src/architecture/utilities/eigenSupport.h index 1e554b4032..cb8fd9beed 100644 --- a/src/architecture/utilities/eigenSupport.h +++ b/src/architecture/utilities/eigenSupport.h @@ -253,16 +253,28 @@ void eigenMatrixXInsertCArray(const Eigen::MatrixBase& inMat, } /** - * @brief Copy a fixed-size Eigen vector into a contiguous C array. + * @brief Copy a fixed-size Eigen column-vector expression into a C array. * - * @tparam ScalarT Scalar type stored in the vector. - * @tparam size Compile-time number of elements. - * @param inVec Vector whose contents should be copied. - * @param outArray Pointer to a C array with at least `size` elements. + * Accepts any fixed-size Eigen expression that resolves to a column vector + * (concrete `Eigen::Vector`, `Vector::Zero()`, `block()`, etc.). + * The destination is a sized C array; its length is enforced at compile time + * to match the vector length. + * + * @tparam Derived Fixed-size Eigen column-vector expression type. + * @tparam size Extent of the destination array (must equal vector length). + * @param inVec Vector expression whose contents should be copied. + * @param out Destination array that receives the entries. */ -template -void eigenVectorToCArray(const Eigen::Vector& inVec, ScalarT* outArray) { - std::copy(inVec.data(), inVec.data() + size, outArray); +template +void eigenVectorToCArray(const Eigen::MatrixBase& inVec, typename Derived::Scalar (&out)[size]) { + static_assert(Derived::RowsAtCompileTime != Eigen::Dynamic && Derived::ColsAtCompileTime != Eigen::Dynamic, + "Input must be a fixed-size Eigen type."); + static_assert(Derived::ColsAtCompileTime == 1, "Input must be a column vector."); + static_assert(static_cast(Derived::RowsAtCompileTime) == size, + "Output array size must equal vector length."); + + const typename Derived::PlainObject evaluated = inVec; + std::copy(evaluated.data(), evaluated.data() + size, out); } /** diff --git a/src/architecture/utilities/tests/test_eigenSupport.cpp b/src/architecture/utilities/tests/test_eigenSupport.cpp index 287fedd122..864259f52f 100644 --- a/src/architecture/utilities/tests/test_eigenSupport.cpp +++ b/src/architecture/utilities/tests/test_eigenSupport.cpp @@ -121,14 +121,36 @@ TYPED_TEST(EigenSupportConversionsTest, EigenVectorToCArrayCopiesElements) { input(i) = static_cast(i - 1); } - std::array output{}; - eigenVectorToCArray(input, output.data()); + Scalar output[size] = {}; + eigenVectorToCArray(input, output); for (int i = 0; i < size; ++i) { EXPECT_EQ(output[i], input(i)); } } +TYPED_TEST(EigenSupportConversionsTest, EigenVectorToCArrayAcceptsExpression) { + using Scalar = TypeParam; + + // Constant expression (Zero) - the original failure mode that prompted + // widening the signature to MatrixBase. + Scalar zeroOut[3] = {static_cast(7), static_cast(7), static_cast(7)}; + eigenVectorToCArray(Eigen::Vector3::Zero(), zeroOut); + for (int i = 0; i < 3; ++i) { + EXPECT_EQ(zeroOut[i], static_cast(0)); + } + + // Block expression - exercises the PlainObject evaluation path. + Eigen::Matrix source; + source << static_cast(1), static_cast(2), static_cast(3), static_cast(4); + + Scalar blockOut[3] = {}; + eigenVectorToCArray(source.template head<3>(), blockOut); + for (int i = 0; i < 3; ++i) { + EXPECT_EQ(blockOut[i], source(i)); + } +} + TYPED_TEST(EigenSupportConversionsTest, cArrayToEigenMatrixPreservesColumnMajorOrdering) { using Scalar = TypeParam; constexpr int rows = 3; From b5b1dde5c32d86fc239e3f43bc9700d0b64dfc71 Mon Sep 17 00:00:00 2001 From: patrick kenneally Date: Fri, 1 May 2026 12:21:26 -0600 Subject: [PATCH 3/4] Tighten c2DArrayToEigenMatrix3 to const array reference The previous parameter `ScalarT in2DArray[3][3]` decays to a non-const `ScalarT (*)[3]` pointer, so the outer dimension was unenforced and const-qualified callers could not bind. Switching to `const ScalarT (&in2DArray)[3][3]` enforces both dimensions at compile time and matches the const-input convention used elsewhere in this header. All existing call sites pass non-const `[3][3]` lvalues, which still bind to the const reference, so no caller needs to change. Add C2DArrayToEigenMatrix3AcceptsConstInput as a regression test in the same style as the other AcceptsConstInput tests - it will fail to compile if the const qualifier is ever removed. --- src/architecture/utilities/eigenSupport.h | 9 ++++++--- .../utilities/tests/test_eigenSupport.cpp | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/architecture/utilities/eigenSupport.h b/src/architecture/utilities/eigenSupport.h index cb8fd9beed..b89eaeb572 100644 --- a/src/architecture/utilities/eigenSupport.h +++ b/src/architecture/utilities/eigenSupport.h @@ -362,14 +362,17 @@ Eigen::Matrix3 cArrayToEigenMatrix3(const ScalarT* inArray) { /** * @brief Copy a 3×3 C two-dimensional array into an Eigen 3×3 matrix. * + * Both dimensions are enforced at compile time via the array reference + * parameter, and the input is const-qualified for consistency with the rest + * of the input-side conversion functions. + * * @tparam ScalarT Scalar type of the matrix. * @param in2DArray Source array with bounds `[3][3]`. * @return Eigen::Matrix3 containing the same entries. */ template -Eigen::Matrix3 c2DArrayToEigenMatrix3(ScalarT in2DArray[3][3]) { - Eigen::Matrix3 outMat = Eigen::Map>(&in2DArray[0][0]); - return outMat; +Eigen::Matrix3 c2DArrayToEigenMatrix3(const ScalarT (&in2DArray)[3][3]) { + return Eigen::Map>(&in2DArray[0][0]); } /** diff --git a/src/architecture/utilities/tests/test_eigenSupport.cpp b/src/architecture/utilities/tests/test_eigenSupport.cpp index 864259f52f..a595e63bbd 100644 --- a/src/architecture/utilities/tests/test_eigenSupport.cpp +++ b/src/architecture/utilities/tests/test_eigenSupport.cpp @@ -303,6 +303,21 @@ TYPED_TEST(EigenSupportConversionsTest, C2DArrayToEigenMatrix3CopiesEntries) { } } +TYPED_TEST(EigenSupportConversionsTest, C2DArrayToEigenMatrix3AcceptsConstInput) { + using Scalar = TypeParam; + const Scalar raw[3][3] = {{static_cast(1), static_cast(2), static_cast(3)}, + {static_cast(4), static_cast(5), static_cast(6)}, + {static_cast(7), static_cast(8), static_cast(9)}}; + + Eigen::Matrix3 reconstructed = c2DArrayToEigenMatrix3(raw); + + for (int i = 0; i < 3; ++i) { + for (int j = 0; j < 3; ++j) { + EXPECT_EQ(reconstructed(i, j), raw[i][j]); + } + } +} + TYPED_TEST(EigenSupportConversionsTest, EigenMatrixToCArray2DColumnMajorInput) { using Scalar = TypeParam; using MatrixType = Eigen::Matrix; From 4bac619efa5488506ab76108ecb5811f55d6a215 Mon Sep 17 00:00:00 2001 From: patrick kenneally Date: Fri, 1 May 2026 12:25:33 -0600 Subject: [PATCH 4/4] Enforce 3-vector shape on eigenTilde eigenTilde previously read vec(0)/(1)/(2) from any vector-shaped input, so a 4-vector or a 1x3 row would silently produce a meaningless skew matrix. Add shape enforcement consistent with the header's fixed-vs-dynamic split: - Fixed-size types are validated at compile time with static_assert on RowsAtCompileTime == 3 and ColsAtCompileTime == 1. - Dynamic-size types (e.g. MatrixXd state property refs in GravityGradientEffector and spacecraft) are still accepted at compile time and validated at runtime via std::terminate() on shape mismatch. No call sites need updating. The relaxation to admit dynamic-size inputs preserves behavior for the two real callers that pass a runtime-sized state property. --- src/architecture/utilities/eigenSupport.h | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/architecture/utilities/eigenSupport.h b/src/architecture/utilities/eigenSupport.h index b89eaeb572..5e9ba7bd87 100644 --- a/src/architecture/utilities/eigenSupport.h +++ b/src/architecture/utilities/eigenSupport.h @@ -435,12 +435,28 @@ Eigen::Matrix3 eigenM3(ScalarT angle) { /** * @brief Construct the skew-symmetric matrix such that `[tilde(vec)] * b = vec × b`. * - * @tparam Derived Eigen vector expression type. + * Accepts either a fixed-size 3-element column vector (shape checked at + * compile time via `static_assert`) or a dynamic-size expression that is + * 3×1 at runtime (shape checked via `std::terminate()`). This mirrors the + * suite-wide split between fixed and dynamic variants documented at the + * top of this header. + * + * @tparam Derived Eigen 3-vector expression type. * @param vec Vector whose associated tilde matrix is requested. * @return Eigen::Matrix3 representing the skew-symmetric cross-product matrix. */ template Eigen::Matrix3::Scalar> eigenTilde(const Eigen::MatrixBase& vec) { + static_assert((Derived::RowsAtCompileTime == 3 || Derived::RowsAtCompileTime == Eigen::Dynamic) && + (Derived::ColsAtCompileTime == 1 || Derived::ColsAtCompileTime == Eigen::Dynamic), + "eigenTilde requires a 3-element column vector (fixed-size or dynamic)."); + + if constexpr (Derived::RowsAtCompileTime == Eigen::Dynamic || Derived::ColsAtCompileTime == Eigen::Dynamic) { + if (vec.rows() != 3 || vec.cols() != 1) { + std::terminate(); + } + } + using Scalar = Eigen::MatrixBase::Scalar; const Scalar vx = vec(0);