diff --git a/.github/scripts/check-pg-versions.sh b/.github/scripts/check-pg-versions.sh deleted file mode 100755 index 9a15be8913a..00000000000 --- a/.github/scripts/check-pg-versions.sh +++ /dev/null @@ -1,186 +0,0 @@ -#!/bin/bash -# -# Check if parameter group struct modifications include version increments -# This prevents settings corruption when struct layout changes without version bump -# -# Exit codes: -# 0 - No issues found -# 1 - Potential issues detected (will post comment) -# 2 - Script error - -set -euo pipefail - -# Output file for issues found -ISSUES_FILE=$(mktemp) -trap "rm -f $ISSUES_FILE" EXIT - -# Color output for local testing -if [ -t 1 ]; then - RED='\033[0;31m' - GREEN='\033[0;32m' - YELLOW='\033[1;33m' - NC='\033[0m' # No Color -else - RED='' - GREEN='' - YELLOW='' - NC='' -fi - -echo "🔍 Checking for Parameter Group version updates..." - -# Get base and head commits -BASE_REF=${GITHUB_BASE_REF:-} -HEAD_REF=${GITHUB_HEAD_REF:-} - -if [ -z "$BASE_REF" ] || [ -z "$HEAD_REF" ]; then - echo "âš ī¸ Warning: Not running in GitHub Actions PR context" - echo "Using git diff against HEAD~1 for local testing" - BASE_COMMIT="HEAD~1" - HEAD_COMMIT="HEAD" -else - BASE_COMMIT="origin/$BASE_REF" - HEAD_COMMIT="HEAD" -fi - -# Get list of changed files -CHANGED_FILES=$(git diff --name-only $BASE_COMMIT..$HEAD_COMMIT | grep -E '\.(c|h)$' || true) - -if [ -z "$CHANGED_FILES" ]; then - echo "✅ No C/H files changed" - exit 0 -fi - -echo "📁 Changed files:" -echo "$CHANGED_FILES" | sed 's/^/ /' - -# Function to extract PG info from a file -check_file_for_pg_changes() { - local file=$1 - local diff_output=$(git diff $BASE_COMMIT..$HEAD_COMMIT -- "$file") - - # Check if file contains PG_REGISTER (in either old or new version) - if ! echo "$diff_output" | grep -q "PG_REGISTER"; then - return 0 - fi - - echo " 🔎 Checking $file (contains PG_REGISTER)" - - # Extract all PG_REGISTER lines from the diff (both old and new) - local pg_registers=$(echo "$diff_output" | grep -E "^[-+].*PG_REGISTER" || true) - - if [ -z "$pg_registers" ]; then - # PG_REGISTER exists but wasn't changed - # Still need to check if the struct changed - pg_registers=$(git show $HEAD_COMMIT:"$file" | grep "PG_REGISTER" || true) - fi - - # Process each PG registration - while IFS= read -r pg_line; do - [ -z "$pg_line" ] && continue - - # Extract struct name and version - # Pattern: PG_REGISTER.*\((\w+),\s*(\w+),\s*PG_\w+,\s*(\d+)\) - if [[ $pg_line =~ PG_REGISTER[^(]*\(([^,]+),([^,]+),([^,]+),([^)]+)\) ]]; then - local struct_type="${BASH_REMATCH[1]}" - local pg_name="${BASH_REMATCH[2]}" - local pg_id="${BASH_REMATCH[3]}" - local version="${BASH_REMATCH[4]}" - - # Clean up whitespace - struct_type=$(echo "$struct_type" | xargs) - version=$(echo "$version" | xargs) - - echo " 📋 Found: $struct_type (version $version)" - - # Check if this struct's typedef was modified - local struct_pattern="typedef struct ${struct_type%_t}_s" - # Isolate struct body from diff, remove comments and empty lines, then check for remaining changes - local struct_body_diff=$(echo "$diff_output" | sed -n "/${struct_pattern}/,/\}.*${struct_type};/p") - - # If struct not found in current file, check corresponding .h or .c file - if [ -z "$struct_body_diff" ]; then - local companion_file="" - if [[ "$file" == *.c ]]; then - companion_file="${file%.c}.h" - elif [[ "$file" == *.h ]]; then - companion_file="${file%.h}.c" - fi - - if [ -n "$companion_file" ] && git diff --name-only $BASE_COMMIT..$HEAD_COMMIT | grep -q "^${companion_file}$"; then - echo " 🔍 Struct not in $file, checking $companion_file" - local companion_diff=$(git diff $BASE_COMMIT..$HEAD_COMMIT -- "$companion_file") - struct_body_diff=$(echo "$companion_diff" | sed -n "/${struct_pattern}/,/\}.*${struct_type};/p") - fi - fi - - local struct_changes=$(echo "$struct_body_diff" | grep -E "^[-+]" \ - | grep -v -E "^[-+]\s*(typedef struct|}|//|\*)" \ - | sed -E 's://.*$::' \ - | sed -E 's:/\*.*\*/::' \ - | tr -d '[:space:]') - - if [ -n "$struct_changes" ]; then - echo " âš ī¸ Struct definition modified" - - # Check if version was incremented in this diff - local old_version=$(echo "$diff_output" | grep "^-.*PG_REGISTER.*$struct_type" | grep -oP ',\s*\K\d+(?=\s*\))' || echo "") - local new_version=$(echo "$diff_output" | grep "^+.*PG_REGISTER.*$struct_type" | grep -oP ',\s*\K\d+(?=\s*\))' || echo "") - - if [ -n "$old_version" ] && [ -n "$new_version" ]; then - if [ "$new_version" -le "$old_version" ]; then - echo " ❌ Version NOT incremented ($old_version → $new_version)" - - # Find line number of PG_REGISTER - local line_num=$(git show $HEAD_COMMIT:"$file" | grep -n "PG_REGISTER.*$struct_type" | cut -d: -f1 | head -1) - - # Add to issues list - cat >> $ISSUES_FILE << EOF -### \`$struct_type\` ($file:$line_num) -- **Struct modified:** Field changes detected -- **Version status:** ❌ Not incremented (version $version) -- **Recommendation:** Review changes and increment version if needed - -EOF - else - echo " ✅ Version incremented ($old_version → $new_version)" - fi - elif [ -z "$old_version" ] || [ -z "$new_version" ]; then - # Couldn't determine version change, but struct was modified - echo " âš ī¸ Could not determine if version was incremented" - - local line_num=$(git show $HEAD_COMMIT:"$file" | grep -n "PG_REGISTER.*$struct_type" | cut -d: -f1 | head -1) - - cat >> $ISSUES_FILE << EOF -### \`$struct_type\` ($file:$line_num) -- **Struct modified:** Field changes detected -- **Version status:** âš ī¸ Unable to verify version increment -- **Current version:** $version -- **Recommendation:** Verify version was incremented if struct layout changed - -EOF - fi - else - echo " ✅ Struct unchanged" - fi - fi - done <<< "$pg_registers" -} - -# Check each changed file -while IFS= read -r file; do - check_file_for_pg_changes "$file" -done <<< "$CHANGED_FILES" - -# Check if any issues were found -if [ -s $ISSUES_FILE ]; then - echo "" - echo "${YELLOW}âš ī¸ Potential PG version issues detected${NC}" - echo "Output saved to: $ISSUES_FILE" - cat $ISSUES_FILE - exit 1 -else - echo "" - echo "${GREEN}✅ No PG version issues detected${NC}" - exit 0 -fi diff --git a/.github/workflows/pg-version-check.yml b/.github/workflows/pg-version-check.yml deleted file mode 100644 index d9d8c289930..00000000000 --- a/.github/workflows/pg-version-check.yml +++ /dev/null @@ -1,128 +0,0 @@ -name: Parameter Group Version Check - -on: - pull_request: - types: [opened, synchronize, reopened] - branches: - - maintenance-9.x - - maintenance-10.x - paths: - - 'src/**/*.c' - - 'src/**/*.h' - -jobs: - check-pg-versions: - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write - - steps: - - name: Checkout PR code - uses: actions/checkout@v4 - with: - fetch-depth: 0 # Need full history to compare with base branch - - - name: Fetch base branch - run: | - git fetch origin ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }} - - - name: Run PG version check script - id: pg_check - run: | - set +e # Don't fail the workflow, just capture exit code - # Run script and capture output. Exit code 1 is expected for issues. - # The output is captured and encoded to be passed between steps. - output=$(bash .github/scripts/check-pg-versions.sh 2>&1) - exit_code=$? - echo "exit_code=${exit_code}" >> $GITHUB_OUTPUT - echo "output<> $GITHUB_OUTPUT - echo "$output" >> $GITHUB_OUTPUT - echo "EOF" >> $GITHUB_OUTPUT - env: - GITHUB_BASE_REF: ${{ github.base_ref }} - GITHUB_HEAD_REF: ${{ github.head_ref }} - - - name: Post comment if issues found - if: steps.pg_check.outputs.exit_code == '1' - uses: actions/github-script@v7 - with: - script: | - // Use the captured output from the previous step - const output = '${{ steps.pg_check.outputs.output }}'; - let issuesContent = ''; - - try { - // Extract issues from output (everything after the warning line) - const lines = output.split('\n'); - let capturing = false; - let issues = []; - - for (const line of lines) { - if (line.includes('###')) { - capturing = true; - } - if (capturing) { - issues.push(line); - } - } - - issuesContent = issues.join('\n'); - } catch (err) { - console.log('Error capturing issues:', err); - issuesContent = '*Unable to extract detailed issues*'; - } - - const commentBody = '## âš ī¸ Parameter Group Version Check\n\n' + - 'The following parameter groups may need version increments:\n\n' + - issuesContent + '\n\n' + - '**Why this matters:**\n' + - 'Modifying PG struct fields without incrementing the version can cause settings corruption when users flash new firmware. The `pgLoad()` function validates versions and will use defaults if there\'s a mismatch, preventing corruption.\n\n' + - '**When to increment the version:**\n' + - '- ✅ Adding/removing fields\n' + - '- ✅ Changing field types or sizes\n' + - '- ✅ Reordering fields\n' + - '- ✅ Adding/removing packing attributes\n' + - '- ❌ Only changing default values in `PG_RESET_TEMPLATE`\n' + - '- ❌ Only changing comments\n\n' + - '**Reference:**\n' + - '- [Parameter Group Documentation](../docs/development/parameter_groups/)\n' + - '- Example: [PR #11236](https://github.com/iNavFlight/inav/pull/11236) (field removal requiring version increment)\n\n' + - '---\n' + - '*This is an automated check. False positives are possible. If you believe the version increment is not needed, please explain in a comment.*'; - - try { - // Check if we already commented - const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - }); - - const botComment = comments.find(comment => - comment.user.login === 'github-actions[bot]' && - comment.body.includes('Parameter Group Version Check') - ); - - if (botComment) { - // Update existing comment - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: botComment.id, - body: commentBody - }); - console.log('Updated existing PG version check comment'); - } else { - // Post new comment - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: commentBody - }); - console.log('Posted new PG version check comment'); - } - } catch (err) { - core.setFailed(`Failed to post comment: ${err}`); - } diff --git a/cmake/check-pg-struct-sizes.sh b/cmake/check-pg-struct-sizes.sh new file mode 100755 index 00000000000..fcbbbcac0ed --- /dev/null +++ b/cmake/check-pg-struct-sizes.sh @@ -0,0 +1,133 @@ +#!/bin/bash +# +# Check PG struct sizes against database +# Fails build if size changed but PG version wasn't incremented +# +# Usage: check-pg-struct-sizes.sh +# + +set -euo pipefail + +ELF_FILE=$1 +SCRIPT_DIR=$(dirname "$0") +DB_FILE="$SCRIPT_DIR/pg_struct_sizes.db" + +if [ ! -f "$ELF_FILE" ]; then + echo "Error: ELF file not found: $ELF_FILE" >&2 + exit 1 +fi + +if [ ! -f "$DB_FILE" ]; then + echo "âš ī¸ Warning: PG struct sizes database not found: $DB_FILE" >&2 + echo " Run: $SCRIPT_DIR/extract-pg-sizes-nm.sh $ELF_FILE > $DB_FILE" >&2 + echo " Skipping validation." >&2 + exit 0 +fi + +echo "🔍 Checking PG struct sizes against database..." + +# Extract current sizes +TEMP_CURRENT=$(mktemp) +"$SCRIPT_DIR/extract-pg-sizes-nm.sh" "$ELF_FILE" 2>/dev/null > "$TEMP_CURRENT" + +# Function to get PG version from source code +get_pg_version() { + local struct_type=$1 + + # Remove _t suffix to get config name + # e.g., blackboxConfig_t -> blackboxConfig + local config_name="${struct_type%_t}" + + # Search for PG_REGISTER in source files + # Format: PG_REGISTER_WITH_RESET_TEMPLATE(type, name, pgn, version) + # or: PG_REGISTER_WITH_RESET_FN(type, name, pgn, version) + # or: PG_REGISTER(type, name, pgn, version) + local version=$(grep -rh "PG_REGISTER.*$config_name" src/main --include="*.c" 2>/dev/null | \ + grep -oP 'PG_REGISTER[^(]*\([^,]+,[^,]+,[^,]+,\s*\K\d+' | head -1) + + echo "$version" +} + +FAILED=0 +ISSUES="" +UPDATED="" + +# Check each struct in current binary +while read -r struct_type current_size; do + [ -z "$struct_type" ] && continue + + # Look up in database + db_entry=$(grep "^$struct_type " "$DB_FILE" 2>/dev/null || echo "") + + if [ -z "$db_entry" ]; then + # New struct not in database - add it automatically + current_version=$(get_pg_version "$struct_type") + + if [ -z "$current_version" ]; then + echo " âš ī¸ Warning: Cannot find PG version for new struct $struct_type" >&2 + echo " â„šī¸ New: $struct_type (${current_size}B) - skipping (no PG_REGISTER found)" + continue + fi + + echo " ➕ New: $struct_type (${current_size}B, v$current_version)" + + # Add to database + printf "%-30s %3s %s\n" "$struct_type" "$current_size" "$current_version" >> "$DB_FILE" + UPDATED="$UPDATED\n â€ĸ $struct_type: NEW (${current_size}B, v$current_version)" + continue + fi + + db_size=$(echo "$db_entry" | awk '{print $2}') + db_version=$(echo "$db_entry" | awk '{print $3}') + + if [ "$current_size" != "$db_size" ]; then + # Size changed - check if version was incremented + current_version=$(get_pg_version "$struct_type") + + if [ -z "$current_version" ]; then + echo " âš ī¸ Warning: Cannot find PG version for $struct_type" >&2 + continue + fi + + if [ "$current_version" -le "$db_version" ]; then + # SIZE CHANGED BUT VERSION NOT INCREMENTED - FAIL BUILD + echo " ❌ $struct_type: size changed ${db_size}B → ${current_size}B but version not incremented (still v$current_version)" + ISSUES="$ISSUES\n â€ĸ $struct_type: ${db_size}B → ${current_size}B (version $current_version should be $((current_version + 1)))" + FAILED=1 + else + # Version was incremented - this is valid, update database + echo " ✅ $struct_type: size changed ${db_size}B → ${current_size}B with version increment v$db_version → v$current_version" + + # Update database entry with new size and version + sed -i "s/^$struct_type *[0-9]* *[0-9]*$/$(printf "%-30s %3s %s" "$struct_type" "$current_size" "$current_version")/" "$DB_FILE" + UPDATED="$UPDATED\n â€ĸ $struct_type: ${db_size}B → ${current_size}B (v$db_version → v$current_version)" + fi + else + # Size unchanged + echo " ✓ $struct_type (${current_size}B)" + fi +done < "$TEMP_CURRENT" + +rm -f "$TEMP_CURRENT" + +if [ $FAILED -eq 1 ]; then + echo "" + echo "❌ PG STRUCT SIZE VALIDATION FAILED" + echo "" + echo "The following structs changed size without version increments:" + echo -e "$ISSUES" + echo "" + echo "Fix: Increment PG version in PG_REGISTER for affected structs" + echo "" + exit 1 +fi + +if [ -n "$UPDATED" ]; then + echo "" + echo "📝 Database auto-updated for structs with version increments:" + echo -e "$UPDATED" + echo "" +fi + +echo "✅ All PG struct sizes validated successfully" +exit 0 diff --git a/cmake/extract-pg-sizes-nm.sh b/cmake/extract-pg-sizes-nm.sh new file mode 100755 index 00000000000..70b37ec93f3 --- /dev/null +++ b/cmake/extract-pg-sizes-nm.sh @@ -0,0 +1,49 @@ +#!/bin/bash +# +# Extract PG struct sizes using nm (no extra dependencies) +# Reads pgResetTemplate symbol sizes from symbol table +# +# Usage: extract-pg-sizes-nm.sh +# + +set -euo pipefail + +ELF_FILE=$1 + +if [ ! -f "$ELF_FILE" ]; then + echo "Error: ELF file not found: $ELF_FILE" >&2 + exit 1 +fi + +echo "Extracting PG struct sizes from $ELF_FILE using nm..." >&2 + +# Detect architecture for correct nm command +if command -v arm-none-eabi-nm &> /dev/null && [[ $(file "$ELF_FILE") == *"ARM"* ]]; then + NM_CMD="arm-none-eabi-nm" +else + NM_CMD="nm" +fi + +# Extract pgResetTemplate symbols +# Format: D pgResetTemplate_ +$NM_CMD --print-size "$ELF_FILE" 2>/dev/null | grep "pgResetTemplate_" | \ + while read addr size_hex type symbol; do + # Extract config name from symbol + # pgResetTemplate_blackboxConfig -> blackboxConfig + config_name="${symbol#pgResetTemplate_}" + + # Convert hex size to decimal + size_dec=$((16#$size_hex)) + + # Find corresponding struct type from PG_REGISTER + struct_type=$(grep -rh "PG_REGISTER.*$config_name" src/main --include="*.c" 2>/dev/null | \ + grep -oP 'PG_REGISTER[^(]*\(\K[^,]+' | head -1) + + if [ -z "$struct_type" ]; then + # Fallback: convert config name to struct type + # blackboxConfig -> blackboxConfig_t + struct_type="${config_name}_t" + fi + + printf "%-30s %s\n" "$struct_type" "$size_dec" + done | sort -u diff --git a/cmake/main.cmake b/cmake/main.cmake index d8e2dddf3ec..b7375cdb4ae 100644 --- a/cmake/main.cmake +++ b/cmake/main.cmake @@ -105,6 +105,13 @@ function(setup_firmware_target exe name) if(args_SKIP_RELEASES) set_target_properties(${exe} ${name} PROPERTIES SKIP_RELEASES ON) endif() + + # Add PG struct size validation + add_custom_command(TARGET ${exe} POST_BUILD + COMMAND ${CMAKE_COMMAND} -E chdir ${CMAKE_SOURCE_DIR} ${CMAKE_SOURCE_DIR}/cmake/check-pg-struct-sizes.sh $ + COMMENT "Validating PG struct sizes for ${name}" + VERBATIM + ) endfunction() function(exclude_from_all target) diff --git a/cmake/pg_struct_sizes.db b/cmake/pg_struct_sizes.db new file mode 100644 index 00000000000..f9dda814f67 --- /dev/null +++ b/cmake/pg_struct_sizes.db @@ -0,0 +1,49 @@ +# PG Struct Size Database +# Format: struct_type size version +# Generated from: build_sitl/bin/SITL.elf +# Date: 2026-01-23 04:53:01 UTC + +adcChannelConfig_t 4 0 +armingConfig_t 6 3 +barometerConfig_t 8 5 +batteryMetersConfig_t 36 2 +beeperConfig_t 12 2 +blackboxConfig_t 16 4 +compassConfig_t 32 6 +displayConfig_t 1 0 +djiOsdConfig_t 6 3 +ezTuneSettings_t 12 1 +failsafeConfig_t 22 3 +featureConfig_t 4 0 +generalSettings_t 1 0 +geozone_config_t 20 0 +gimbalConfig_t 10 1 +gimbalSerialConfig_t 1 0 +gpsConfig_t 32 5 +gyroConfig_t 28 12 +headTrackerConfig_t 16 1 +imuConfig_t 16 2 +logConfig_t 8 0 +modeActivationOperatorConfig_t 4 0 +motorConfig_t 10 11 +navConfig_t 156 7 +navFwAutolandConfig_t 16 0 +opticalFlowConfig_t 8 2 +osdCommonConfig_t 4 0 +osdConfig_t 168 15 +pidAutotuneConfig_t 10 2 +pidProfile_t 292 11 +pitotmeterConfig_t 8 2 +positionEstimationConfig_t 72 8 +powerLimitsConfig_t 8 1 +rangefinderConfig_t 2 3 +rcControlsConfig_t 10 4 +reversibleMotorsConfig_t 6 0 +rxConfig_t 36 13 +servoConfig_t 12 3 +statsConfig_t 20 2 +systemConfig_t 39 7 +telemetryConfig_t 64 8 +timeConfig_t 4 1 +vtxConfig_t 55 4 +vtxSettingsConfig_t 12 2