Added status field to field access messages (#7)

raffael0 · web-flow · commit c66b7136350a · 2026-02-28T18:14:41.000+01:00
This PR fixes the issue that we previously did not define what the server should respond, when it does not have corresponding field registered. To fix this I added a status field to the response messages

Additionally i've added a ci script, so that you can run and fix formatting/linting issues locally. See: ci-rust.sh
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -17,20 +17,14 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: Build
-      working-directory: liquidcan_rust
-      run: cargo build --verbose
+      run: ./ci-rust.sh build
     - name: Run tests
-      working-directory: liquidcan_rust
-      run: cargo test --verbose
+      run: ./ci-rust.sh test
     - name: Run macro tests
-      working-directory: liquidcan_rust/liquidcan_rust_macros
-      run: cargo test --verbose
+      run: ./ci-rust.sh test-macros
     - name: Check formatting
-      working-directory: liquidcan_rust
-      run: cargo fmt --all -- --check
+      run: ./ci-rust.sh fmt
     - name: Run clippy
-      working-directory: liquidcan_rust
-      run: cargo clippy --all-targets --all-features -- -D warnings
+      run: ./ci-rust.sh clippy
     - name: Run clippy on macros
-      working-directory: liquidcan_rust/liquidcan_rust_macros
-      run: cargo clippy --all-targets --all-features -- -D warnings
+      run: ./ci-rust.sh clippy-macros
diff --git a/.gitignore b/.gitignore
@@ -343,3 +343,4 @@ TSWLatexianTemp*
 
 # MSVC Windows builds of rustc generate these, which store debugging information
 *.pdb
+**/.idea
diff --git a/LiquidCAN.pdf b/LiquidCAN.pdf
diff --git a/README.md b/README.md
@@ -24,4 +24,28 @@ When creating issues or pull requests with protocol changes, generate a PDF diff
 
 ```bash
 latexdiff-vc --git --flatten --pdf -r <old-commit> LiquidCAN.tex 
-```
+```
+
+## Development
+
+### Running CI Checks
+
+The repository includes a CI script (`ci-rust.sh`) that runs all quality checks on the Rust implementation. This script is used both locally and in GitHub Actions 
+
+**Run all checks:**
+```bash
+./ci-rust.sh
+# or explicitly
+./ci-rust.sh all
+```
+
+**Run individual checks:**
+```bash
+./ci-rust.sh build         # Build the project
+./ci-rust.sh test          # Run tests
+./ci-rust.sh test-macros   # Run macro tests
+./ci-rust.sh fmt           # Check code formatting
+./ci-rust.sh clippy        # Run clippy linter
+./ci-rust.sh clippy-macros # Run clippy on macros
+```
+You can fix formatting or linter issues by adding the -fix suffix to the command. e.g: `./ci-rust.sh clippy-fix`
diff --git a/ci-rust.sh b/ci-rust.sh
@@ -0,0 +1,136 @@
+#!/usr/bin/env bash
+
+# CI script for Rust LiquidCAN project
+# This script runs all the checks that are performed in the GitHub Actions workflow
+
+set -e  # Exit on error
+
+# Colors for output
+RED='\033[0;31m'
+GREEN='\033[0;32m'
+YELLOW='\033[1;33m'
+NC='\033[0m' # No Color
+
+# Function to print step headers
+print_step() {
+    echo -e "\n${YELLOW}==== $1 ====${NC}\n"
+}
+
+print_success() {
+    echo -e "${GREEN}✓ $1${NC}"
+}
+
+print_error() {
+    echo -e "${RED}✗ $1${NC}"
+}
+
+# Get the script directory
+SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+LIQUIDCAN_RUST_DIR="$SCRIPT_DIR/liquidcan_rust"
+MACROS_DIR="$LIQUIDCAN_RUST_DIR/liquidcan_rust_macros"
+
+# Export environment variable
+export CARGO_TERM_COLOR=always
+
+# Function to run a specific step or all steps
+run_step() {
+    local step=$1
+    
+    case $step in
+        build)
+            print_step "Build"
+            cd "$LIQUIDCAN_RUST_DIR"
+            cargo build --verbose || { print_error "Build failed"; return 1; }
+            print_success "Build completed"
+            ;;
+        
+        test)
+            print_step "Run tests"
+            cd "$LIQUIDCAN_RUST_DIR"
+            cargo test --verbose || { print_error "Tests failed"; return 1; }
+            print_success "Tests passed"
+            ;;
+        
+        test-macros)
+            print_step "Run macro tests"
+            cd "$MACROS_DIR"
+            cargo test --verbose || { print_error "Macro tests failed"; return 1; }
+            print_success "Macro tests passed"
+            ;;
+        
+        fmt)
+            print_step "Check formatting"
+            cd "$LIQUIDCAN_RUST_DIR"
+            cargo fmt --all -- --check || { print_error "Formatting check failed"; return 1; }
+            print_success "Formatting check passed"
+            ;;
+        fmt-fix)
+            print_step "Fix formatting"
+            cd "$LIQUIDCAN_RUST_DIR"
+            cargo fmt --all || { print_error "Formatting fix failed"; return 1; }
+            print_success "Formatting fixed"
+            ;;
+        
+        clippy)
+            print_step "Run clippy"
+            cd "$LIQUIDCAN_RUST_DIR"
+            cargo clippy --all-targets --all-features -- -D warnings || { print_error "Clippy check failed"; return 1; }
+            print_success "Clippy check passed"
+            ;;
+        clippy-fix)
+            print_step "Run clippy with fix"
+            cd "$LIQUIDCAN_RUST_DIR"
+            cargo clippy --all-targets --all-features --fix -- -D warnings || { print_error "Clippy fix failed"; return 1; }
+            print_success "Clippy fix passed"
+            ;;
+        clippy-macros)
+            print_step "Run clippy on macros"
+            cd "$MACROS_DIR"
+            cargo clippy --all-targets --all-features -- -D warnings || { print_error "Clippy check on macros failed"; return 1; }
+            print_success "Clippy check on macros passed"
+            ;;
+        clippy-macros-fix)
+            print_step "Run clippy on macros with fix"
+            cd "$MACROS_DIR"
+            cargo clippy --all-targets --all-features --fix -- -D warnings || { print_error "Clippy fix on macros failed"; return 1; }
+            print_success "Clippy fix on macros passed"
+            ;;
+        
+        
+        all)
+            run_step build || return 1
+            run_step test || return 1
+            run_step test-macros || return 1
+            run_step fmt || return 1
+            run_step clippy || return 1
+            run_step clippy-macros || return 1
+            ;;
+        
+        *)
+            echo "Usage: $0 [build|test|test-macros|fmt|clippy|clippy-macros|all]"
+            echo ""
+            echo "Steps:"
+            echo "  build         - Build the project"
+            echo "  test          - Run tests"
+            echo "  test-macros   - Run macro tests"
+            echo "  fmt           - Check formatting"
+            echo "  clippy        - Run clippy linter"
+            echo "  clippy-macros - Run clippy on macros"
+            echo "  all           - Run all steps (default)"
+            exit 1
+            ;;
+    esac
+}
+
+# Main execution
+STEP=${1:-all}
+echo "Running Rust CI checks..."
+echo "Step: $STEP"
+
+if run_step "$STEP"; then
+    echo -e "\n${GREEN}All checks passed successfully!${NC}"
+    exit 0
+else
+    echo -e "\n${RED}Checks failed!${NC}"
+    exit 1
+fi
diff --git a/liquidcan_rust/src/can_message.rs b/liquidcan_rust/src/can_message.rs
@@ -74,9 +74,9 @@ pub enum CanMessage {
     ParameterSetLockReq {
         payload: payloads::ParameterSetLockPayload,
     } = 52,
-    #[pad(61)]
+    #[pad(60)]
     ParameterSetLockConfirmation {
-        payload: payloads::ParameterSetLockPayload,
+        payload: payloads::ParameterSetLockConfirmationPayload,
     } = 53,
 
     // Field Access
@@ -92,7 +92,7 @@ pub enum CanMessage {
     FieldIDLookupReq {
         payload: payloads::FieldIDLookupReqPayload,
     } = 62,
-    #[pad(61)]
+    #[pad(60)]
     FieldIDLookupRes {
         payload: payloads::FieldIDLookupResPayload,
     } = 63,
diff --git a/liquidcan_rust/src/message_conversion.rs b/liquidcan_rust/src/message_conversion.rs
@@ -33,6 +33,7 @@ mod tests {
     use crate::CanMessageFrame;
     use crate::can_message::CanMessage;
     use crate::payloads;
+    use crate::payloads::FieldStatus;
     use zerocopy::FromZeros;
 
     fn test_round_trip(msg: CanMessage) {
@@ -172,9 +173,10 @@ mod tests {
 
     #[test]
     fn test_parameter_set_lock_confirmation() {
-        let payload = payloads::ParameterSetLockPayload {
+        let payload = payloads::ParameterSetLockConfirmationPayload {
             parameter_id: 13,
             parameter_lock: payloads::ParameterLockStatus::Unlocked,
+            field_status: FieldStatus::Ok,
         };
         let msg = CanMessage::ParameterSetLockConfirmation { payload };
         test_round_trip(msg);
@@ -191,7 +193,8 @@ mod tests {
     fn test_field_get_res() {
         let payload = payloads::FieldGetResPayload {
             field_id: 21,
-            value: [0xCC; 62],
+            field_status: FieldStatus::Ok,
+            value: [0xCC; 61],
         };
         let msg = CanMessage::FieldGetRes { payload };
         test_round_trip(msg);
@@ -210,6 +213,7 @@ mod tests {
     fn test_field_id_lookup_res() {
         let payload = payloads::FieldIDLookupResPayload {
             field_id: 22,
+            field_status: FieldStatus::Ok,
             field_type: payloads::CanDataType::Float32,
         };
         let msg = CanMessage::FieldIDLookupRes { payload };
diff --git a/liquidcan_rust/src/payloads.rs b/liquidcan_rust/src/payloads.rs
@@ -90,18 +90,25 @@ pub struct ParameterSetConfirmationPayload {
     pub status: ParameterSetStatus, // Status code
     pub value: [u8; 61],            // Confirmed value after set operation
 }
+#[derive(Specifier, Debug, Copy, Clone, PartialEq, Eq, Immutable, TryFromBytes, IntoBytes)]
+#[repr(u8)]
+pub enum FieldStatus {
+    Ok = 0,
+    NotFound = 1,
+}
 
 #[derive(Debug, Clone, FromBytes, IntoBytes, Immutable, PartialEq)]
 #[repr(C, packed)]
 pub struct FieldGetReqPayload {
     pub field_id: u8, // Field identifier
 }
 
-#[derive(Debug, Clone, FromBytes, IntoBytes, Immutable, PartialEq)]
+#[derive(Debug, Clone, TryFromBytes, IntoBytes, Immutable, PartialEq)]
 #[repr(C, packed)]
 pub struct FieldGetResPayload {
-    pub field_id: u8,    // Field identifier
-    pub value: [u8; 62], // Field value
+    pub field_id: u8,              // Field identifier
+    pub field_status: FieldStatus, // Status of the get operation
+    pub value: [u8; 61],           // Field value
 }
 
 #[derive(Debug, Clone, FromBytes, IntoBytes, Immutable, PartialEq)]
@@ -114,8 +121,9 @@ pub struct FieldIDLookupReqPayload {
 #[derive(Debug, Clone, TryFromBytes, IntoBytes, Immutable, PartialEq)]
 #[repr(C, packed)]
 pub struct FieldIDLookupResPayload {
-    pub field_id: u8,            // Field ID
-    pub field_type: CanDataType, // Field Datatype
+    pub field_id: u8,              // Field ID
+    pub field_status: FieldStatus, // Status of the lookup operation
+    pub field_type: CanDataType,   // Field Datatype
 }
 
 // Important: only derives TryFromBytes because bool doesn't derive FromBytes
@@ -125,6 +133,14 @@ pub struct ParameterSetLockPayload {
     pub parameter_id: u8,                    // Parameter identifier to lock
     pub parameter_lock: ParameterLockStatus, // Lock status (0=unlocked, 1=locked)
 }
+// Important: only derives TryFromBytes because bool doesn't derive FromBytes
+#[derive(Debug, Clone, TryFromBytes, IntoBytes, Immutable, PartialEq)]
+#[repr(C, packed)]
+pub struct ParameterSetLockConfirmationPayload {
+    pub parameter_id: u8,                    // Parameter identifier to lock
+    pub parameter_lock: ParameterLockStatus, // Lock status (0=unlocked, 1=locked)
+    pub field_status: FieldStatus,           // Status of the parameter
+}
 
 static_assertions::const_assert_eq!(size_of::<NodeInfoResPayload>(), 63);
 static_assertions::const_assert_eq!(size_of::<StatusPayload>(), 63);
@@ -137,5 +153,10 @@ static_assertions::const_assert_eq!(size_of::<ParameterSetConfirmationPayload>()
 static_assertions::const_assert_eq!(size_of::<FieldGetReqPayload>(), 1);
 static_assertions::const_assert_eq!(size_of::<FieldGetResPayload>(), 63);
 static_assertions::const_assert_eq!(size_of::<FieldIDLookupReqPayload>(), 61);
-static_assertions::const_assert_eq!(size_of::<FieldIDLookupResPayload>(), 2);
+static_assertions::const_assert_eq!(size_of::<FieldIDLookupResPayload>(), 3);
+static_assertions::const_assert_eq!(size_of::<ParameterSetLockConfirmationPayload>(), 3);
 static_assertions::const_assert_eq!(size_of::<ParameterSetLockPayload>(), 2);
+static_assertions::const_assert_eq!(size_of::<FieldStatus>(), 1);
+static_assertions::const_assert_eq!(size_of::<CanDataType>(), 1);
+static_assertions::const_assert_eq!(size_of::<ParameterSetStatus>(), 1);
+static_assertions::const_assert_eq!(size_of::<ParameterLockStatus>(), 1);
diff --git a/sections/05_field_management.tex b/sections/05_field_management.tex
@@ -81,18 +81,19 @@ \subsubsection{Parameter Updates}\label{subsubsec:parameter-updates}
 
 \subsubsection{Parameter Setting}\label{subsubsec:parameter-setting}
 Other bus members can send a \texttt{parameter\_set\_req} message, which includes the field ID of the parameter and the new value.
-Once the node receives the request, it attempts to set the internal parameter value and responds with a \texttt{parameter\_set\_confirmation} message containing a status code, the parameter ID, and the actual value. The status field indicates whether the operation was successful or provides an error code (see ParameterSetStatus enum in \ref{subsec:ParameterSetStatus}). The value field should contain the actual value read back from the parameter, not simply the value that was received in the request.
+Once the node receives the request, it attempts to set the internal parameter value and responds with a \texttt{parameter\_set\_confirmation} message containing a status code, a field status, the parameter ID, and the actual value. The status field indicates whether the operation was successful or provides an error code (see ParameterSetStatus enum in \ref{subsec:ParameterSetStatus}). The \texttt{field\_status} indicates whether the parameter exists. If the parameter was not found (field\_status = NotFound) the status field should return InvalidParameterID. Otherwise, the value field should contain the actual value read back from the parameter and not simply the value that was received in the request.
+
 
 When a parameter is internally modified through some automated system, the updated value must be sent as a \texttt{parameter\_set\_confirmation} message to the server with a \texttt{NodeToNodeModification} status code.
 \subsubsection{Parameter Locking}\label{subsubsec:parameter-locking}
 
 A parameter can optionally be locked through a \texttt{parameter\_set\_lock\_req} message .
 After a parameter has been locked, it cannot be modified by an external node.
-A parameter can only be unlocked by the locking node or the server.To lock a parameter, a node sends a \texttt{parameter\_set\_lock\_req} with the fieldID and the locking status (0=unlocked, 1=locked). The recieving node responds with a \texttt{parameter\_set\_lock\_confirmation}, confirming the sent fieldID and the locking status. The receiving node also sends a \texttt{parameter\_set\_lock\_confirmation} to the server to update it on the locking status of the parameter.
+A parameter can only be unlocked by the locking node or the server.To lock a parameter, a node sends a \texttt{parameter\_set\_lock\_req} with the fieldID and the locking status (0=unlocked, 1=locked). The recieving node responds with a \texttt{parameter\_set\_lock\_confirmation}, confirming the sent fieldID, the locking status, and a field status (see FieldStatus enum in \ref{subsec:FieldStatus}). If the parameter is not found the node should respond with a not found status(field\_status = NotFound) and the erroneous field id. The receiving node also sends a \texttt{parameter\_set\_lock\_confirmation} to the server to update it on the locking status of the parameter. When a parameter was not found, it does not communicate this with the server. 
 \paragraph{}
 \subsubsection{Requesting Field Data}\label{subsubsec:requesting-field-data}
 A field can be accessed through a \texttt{field\_get\_req} message, which contains the field ID.
-Nodes respond with a \texttt{field\_get\_res} message, containing the field ID and the value of the field.
+Nodes respond with a \texttt{field\_get\_res} message, containing the field ID, a field status (see FieldStatus enum in \ref{subsec:FieldStatus}), and the value of the field. If the field is not found (field\_status = NotFound), the field\_id should still contain the requested field ID from the request message, and the value field content is undefined.
 
 \subsubsection{Field name Lookup}\label{subsubsec:field-name-lookup}
-The field name lookup covers the case where nodes need to access fields from other nodes. Since they don't recieve the \texttt{field\_registration} messages, they don't know the fieldIDs of the named fields they want to access. \texttt{field\_id\_lookup\_req} messages contain the remote field name. The Node responds with a \texttt{field\_id\_lookup\_res} message, containing the fielID and the datatype of the field. A fieldID of 0 indicates that the field was not found.
+The field name lookup covers the case where nodes need to access fields from other nodes. Since they don't recieve the \texttt{field\_registration} messages, they don't know the fieldIDs of the named fields they want to access. \texttt{field\_id\_lookup\_req} messages contain the remote field name. The Node responds with a \texttt{field\_id\_lookup\_res} message, containing the fieldID, the datatype of the field, and a field status (see FieldStatus enum in \ref{subsec:FieldStatus}). If the field is not found (field\_status = NotFound), the field\_id should still contain the requested field ID from the request message.
diff --git a/sections/09_data_structures.tex b/sections/09_data_structures.tex

Original file line number	Diff line number	Diff line change
`@@ -343,3 +343,4 @@ TSWLatexianTemp*`
`343`	`343`
`344`	`344`	`# MSVC Windows builds of rustc generate these, which store debugging information`
`345`	`345`	`*.pdb`
	`346`	`+**/.idea`