From dcdf83d53cea2f33a63930b60388579aa4363eb2 Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Wed, 21 Jan 2026 00:00:05 -0600 Subject: [PATCH 1/3] Add Ray Morris (Sensei) to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 7c2f94fcdb1..010c78e7fed 100644 --- a/AUTHORS +++ b/AUTHORS @@ -81,6 +81,7 @@ Pierre Hugo Richard Birkby Richard Lehey Richard Marko +Ray Morris (Sensei) Rimas Avizienis Sam Cook Sami Korhonen From 5eae507e40f1fab9252ed7eb14f02b6e4a7b5baf Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Thu, 22 Jan 2026 11:35:15 -0600 Subject: [PATCH 2/3] Add GitHub Action to detect PG version increment issues Create automated check that runs on PRs to detect when parameter group structs are modified without incrementing the version number. This prevents settings corruption issues like those seen in PR #11236. Detection logic: - Scans changed C/H files for PG_REGISTER entries - Detects struct typedef modifications in git diff - Verifies version parameter was incremented - Posts helpful comment if version not incremented Files added: - .github/scripts/check-pg-versions.sh - Detection script - .github/workflows/pg-version-check.yml - Workflow definition - .github/workflows/README.md - Workflow documentation The check is non-blocking (comment only) to avoid false positive build failures, but provides clear guidance on when versions must be incremented. --- .github/scripts/check-pg-versions.sh | 163 +++++++++++++++++++++++++ .github/workflows/README.md | 127 +++++++++++++++++++ .github/workflows/pg-version-check.yml | 133 ++++++++++++++++++++ 3 files changed, 423 insertions(+) create mode 100755 .github/scripts/check-pg-versions.sh create mode 100644 .github/workflows/README.md create mode 100644 .github/workflows/pg-version-check.yml diff --git a/.github/scripts/check-pg-versions.sh b/.github/scripts/check-pg-versions.sh new file mode 100755 index 00000000000..45f4955e6f4 --- /dev/null +++ b/.github/scripts/check-pg-versions.sh @@ -0,0 +1,163 @@ +#!/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" + local struct_changes=$(echo "$diff_output" | grep -A50 "$struct_pattern" | grep -E "^[-+]" | grep -v "^[-+]typedef struct" | grep -v "^[-+]}" | grep -v "^[-+]//" | grep -v "^[-+] \*" || true) + + 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 +for file in $CHANGED_FILES; do + check_file_for_pg_changes "$file" +done + +# 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/README.md b/.github/workflows/README.md new file mode 100644 index 00000000000..967080a66fb --- /dev/null +++ b/.github/workflows/README.md @@ -0,0 +1,127 @@ +# GitHub Actions Workflows + +This directory contains automated CI/CD workflows for the INAV project. + +## Active Workflows + +### Build and Test + +#### `ci.yml` - Build Firmware +**Triggers:** Pull requests, pushes to maintenance branches +**Purpose:** Compiles INAV firmware for all targets to verify builds succeed +**Matrix:** 15 parallel build jobs for faster CI + +#### `nightly-build.yml` - Nightly Builds +**Triggers:** Scheduled nightly +**Purpose:** Creates nightly development builds for testing + +### Documentation + +#### `docs.yml` - Documentation Build +**Triggers:** Pull requests affecting documentation +**Purpose:** Validates documentation builds correctly + +### Code Quality + +#### `pg-version-check.yml` - Parameter Group Version Check +**Triggers:** Pull requests to maintenance-9.x and maintenance-10.x +**Purpose:** Detects parameter group struct modifications and verifies version increments +**Why:** Prevents settings corruption when struct layout changes without version bump + +**How it works:** +1. Scans changed .c/.h files for `PG_REGISTER` entries +2. Detects if associated struct typedefs were modified +3. Checks if the PG version parameter was incremented +4. Posts helpful comment if version not incremented + +**Reference:** See `docs/development/parameter_groups/` for PG system documentation + +**Script:** `.github/scripts/check-pg-versions.sh` + +**When to increment PG versions:** +- ✅ Adding/removing fields from struct +- ✅ Changing field types or sizes +- ✅ Reordering fields +- ✅ Adding/removing packing attributes +- ❌ Only changing `PG_RESET_TEMPLATE` default values +- ❌ Only changing comments + +### Pull Request Helpers + +#### `pr-branch-suggestion.yml` - Branch Targeting Suggestion +**Triggers:** PRs targeting master branch +**Purpose:** Suggests using maintenance-9.x or maintenance-10.x instead + +#### `non-code-change.yaml` - Non-Code Change Detection +**Triggers:** Pull requests +**Purpose:** Detects PRs with only documentation/formatting changes + +## Configuration Files + +- `../.github/stale.yml` - Stale issue/PR management +- `../.github/no-response.yml` - Auto-close issues without response +- `../.github/issue_label_bot.yaml` - Automatic issue labeling + +## Adding New Workflows + +When adding workflows: + +1. **Use descriptive names** - Make purpose clear from filename +2. **Document in this README** - Add entry above with purpose and triggers +3. **Set appropriate permissions** - Principle of least privilege +4. **Test in fork first** - Verify before submitting to main repo +5. **Handle errors gracefully** - Don't block CI unnecessarily + +### Common Patterns + +**Checkout with history:** +```yaml +- uses: actions/checkout@v4 + with: + fetch-depth: 0 +``` + +**Post PR comments:** +```yaml +- uses: actions/github-script@v7 + with: + script: | + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: 'Comment text' + }); +``` + +**Run bash scripts:** +```yaml +- run: bash .github/scripts/script-name.sh + env: + GITHUB_BASE_REF: ${{ github.base_ref }} +``` + +## Permissions + +Workflows use GitHub's fine-grained permissions: + +- `contents: read` - Read repository code +- `pull-requests: write` - Post/update PR comments +- `actions: read` - Read workflow run data + +## Local Testing + +Scripts in `.github/scripts/` can be run locally: + +```bash +cd inav +export GITHUB_BASE_REF=maintenance-9.x +export GITHUB_HEAD_REF=feature-branch +bash .github/scripts/check-pg-versions.sh +``` + +## References + +- [GitHub Actions Documentation](https://docs.github.com/en/actions) +- [Workflow Syntax](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions) +- [GitHub Script Action](https://github.com/actions/github-script) diff --git a/.github/workflows/pg-version-check.yml b/.github/workflows/pg-version-check.yml new file mode 100644 index 00000000000..fe80f4b739d --- /dev/null +++ b/.github/workflows/pg-version-check.yml @@ -0,0 +1,133 @@ +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 + bash .github/scripts/check-pg-versions.sh + echo "exit_code=$?" >> $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: | + const fs = require('fs'); + + // Find the issues file (created by the script) + const { execSync } = require('child_process'); + let issuesContent = ''; + + try { + // Re-run script to get issues output + const output = execSync('bash .github/scripts/check-pg-versions.sh 2>&1 || true', { encoding: 'utf-8' }); + + // 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 + +The following parameter groups may need version increments: + +${issuesContent} + +**Why this matters:** +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. + +**When to increment the version:** +- ✅ Adding/removing fields +- ✅ Changing field types or sizes +- ✅ Reordering fields +- ✅ Adding/removing packing attributes +- ❌ Only changing default values in \`PG_RESET_TEMPLATE\` +- ❌ Only changing comments + +**Reference:** +- [Parameter Group Documentation](../docs/development/parameter_groups/) +- Example: [PR #11236](https://github.com/iNavFlight/inav/pull/11236) (field removal requiring version increment) + +--- +*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}`); + } From 41cfcfba753268c9253294dc4949243a61a90bda Mon Sep 17 00:00:00 2001 From: Ray Morris Date: Thu, 22 Jan 2026 11:49:00 -0600 Subject: [PATCH 3/3] Address Qodo code review suggestions Improve PG version check robustness and efficiency per Qodo feedback: 1. Struct detection: Use sed/grep pipeline to better filter comments and whitespace, reducing false positives. Isolates struct body boundaries more reliably before checking for modifications. 2. File iteration: Use while/read loop instead of for loop to correctly handle filenames that contain spaces. 3. Workflow efficiency: Capture script output once and pass between steps using GITHUB_OUTPUT multiline strings, eliminating redundant execution. Changes reduce false positives and improve compatibility while maintaining the same detection logic. --- .github/scripts/check-pg-versions.sh | 12 +++++++++--- .github/workflows/pg-version-check.yml | 19 ++++++++++--------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/.github/scripts/check-pg-versions.sh b/.github/scripts/check-pg-versions.sh index 45f4955e6f4..f5ec2895dd1 100755 --- a/.github/scripts/check-pg-versions.sh +++ b/.github/scripts/check-pg-versions.sh @@ -95,7 +95,13 @@ check_file_for_pg_changes() { # Check if this struct's typedef was modified local struct_pattern="typedef struct ${struct_type%_t}_s" - local struct_changes=$(echo "$diff_output" | grep -A50 "$struct_pattern" | grep -E "^[-+]" | grep -v "^[-+]typedef struct" | grep -v "^[-+]}" | grep -v "^[-+]//" | grep -v "^[-+] \*" || true) + # 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") + 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" @@ -145,9 +151,9 @@ EOF } # Check each changed file -for file in $CHANGED_FILES; do +while IFS= read -r file; do check_file_for_pg_changes "$file" -done +done <<< "$CHANGED_FILES" # Check if any issues were found if [ -s $ISSUES_FILE ]; then diff --git a/.github/workflows/pg-version-check.yml b/.github/workflows/pg-version-check.yml index fe80f4b739d..4d00e9e2719 100644 --- a/.github/workflows/pg-version-check.yml +++ b/.github/workflows/pg-version-check.yml @@ -31,8 +31,14 @@ jobs: id: pg_check run: | set +e # Don't fail the workflow, just capture exit code - bash .github/scripts/check-pg-versions.sh - echo "exit_code=$?" >> $GITHUB_OUTPUT + # 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 }} @@ -42,16 +48,11 @@ jobs: uses: actions/github-script@v7 with: script: | - const fs = require('fs'); - - // Find the issues file (created by the script) - const { execSync } = require('child_process'); + // Use the captured output from the previous step + const output = `${{ steps.pg_check.outputs.output }}`; let issuesContent = ''; try { - // Re-run script to get issues output - const output = execSync('bash .github/scripts/check-pg-versions.sh 2>&1 || true', { encoding: 'utf-8' }); - // Extract issues from output (everything after the warning line) const lines = output.split('\n'); let capturing = false;