Skip to content

Conversation

@sensei-hacker
Copy link
Member

@sensei-hacker sensei-hacker commented Jan 22, 2026

PR Type

Enhancement


Description

  • Add GitHub Action to detect Parameter Group version increment issues

    • Prevents settings corruption from struct modifications without version bumps
    • Scans changed C/H files for PG_REGISTER entries and struct modifications
    • Posts helpful PR comment if version not incremented
  • Improve PG version check robustness and efficiency

    • Better struct detection using sed/grep pipeline to filter comments
    • Handle filenames with spaces using while/read loop
    • Capture script output once and pass between workflow steps
  • Add Ray Morris (Sensei) to AUTHORS file

  • Document GitHub Actions workflows and best practices


Diagram Walkthrough

flowchart LR
  PR["Pull Request to<br/>maintenance-9.x/10.x"]
  TRIGGER["Workflow Triggered<br/>on C/H file changes"]
  SCRIPT["check-pg-versions.sh<br/>Analyzes diff"]
  DETECT["Detect struct<br/>modifications"]
  CHECK["Verify version<br/>incremented"]
  COMMENT["Post PR comment<br/>if issues found"]
  
  PR --> TRIGGER
  TRIGGER --> SCRIPT
  SCRIPT --> DETECT
  DETECT --> CHECK
  CHECK --> COMMENT
Loading

File Walkthrough

Relevant files
Enhancement
check-pg-versions.sh
Parameter Group version detection script                                 

.github/scripts/check-pg-versions.sh

  • New bash script that detects Parameter Group struct modifications in
    git diffs
  • Extracts PG_REGISTER entries and checks if associated struct typedefs
    were modified
  • Verifies version parameter was incremented when struct changes
    detected
  • Generates detailed issue report with file locations and
    recommendations
  • Supports both GitHub Actions context and local testing with proper
    error handling
+169/-0 
pg-version-check.yml
GitHub Actions workflow for PG version checks                       

.github/workflows/pg-version-check.yml

  • New GitHub Actions workflow triggered on PRs to maintenance-9.x and
    maintenance-10.x
  • Runs on changes to src/**/*.c and src/**/*.h files
  • Executes check-pg-versions.sh script and captures output
  • Posts or updates PR comment with detected issues and guidance
  • Uses fine-grained permissions (contents: read, pull-requests: write)
+134/-0 
Documentation
README.md
GitHub Actions workflows documentation                                     

.github/workflows/README.md

  • New documentation file describing all GitHub Actions workflows
  • Documents pg-version-check.yml workflow with purpose and triggers
  • Provides guidance on when to increment PG versions with examples
  • Includes common workflow patterns and local testing instructions
  • Documents permissions model and references for GitHub Actions
+127/-0 
Miscellaneous
AUTHORS
Add Ray Morris to AUTHORS                                                               

AUTHORS

  • Added Ray Morris (Sensei) to the contributors list in alphabetical
    order
+1/-0     

sensei-hacker and others added 5 commits January 21, 2026 00:00
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.
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.
Add GitHub Action to detect Parameter Group version increment issues
@sensei-hacker sensei-hacker merged commit 1092cc0 into master Jan 22, 2026
42 checks passed
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

All compliance sections have been disabled in the configurations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-level Suggestion

Replace the fragile sed/grep shell script logic for parsing C code with a more robust solution. Utilize a proper C parser, such as clang-query or pycparser, to analyze the code's Abstract Syntax Tree (AST) for changes, which will improve accuracy and reduce errors. [High-level, importance: 9]

Solution Walkthrough:

Before:

```bash
# .github/scripts/check-pg-versions.sh

# Get diff for a file
diff_output=$(git diff ... -- "$file")

# Extract PG_REGISTER line with regex
pg_registers=$(echo "$diff_output" | grep "PG_REGISTER")

# For each registration...
# Extract struct name with regex
[[ $pg_line =~ PG_REGISTER... ]]
struct_type="${BASH_REMATCH[1]}"

# Find struct definition in diff using another regex
struct_pattern="typedef struct ${struct_type%_t}_s"
struct_body_diff=$(echo "$diff_output" | sed -n "/${struct_pattern}/,/\}.*${struct_type};/p")

# Check for changes in struct body by stripping comments/whitespace
struct_changes=$(echo "$struct_body_diff" | grep ... | sed ... | tr ...)

if [ -n "$struct_changes" ]; then
    # Check if version was incremented, again using grep/regex
    ...
fi

#### After:

```bash
```python
# Conceptual Python script using a C parser
import pycparser
from pycparser import c_ast

# Get old and new versions of the file from git

# Parse old and new versions of the file into an AST
ast_old = pycparser.parse_file('old_file.c', use_cpp=True)
ast_new = pycparser.parse_file('new_file.c', use_cpp=True)

# Visitor to find PG_REGISTER calls and struct definitions
class PGVisitor(c_ast.NodeVisitor):
    def visit_FuncCall(self, node):
        # Find PG_REGISTER(...) calls and store struct name/version
        ...
    def visit_Typedef(self, node):
        # Find typedef struct ... and store the struct's AST node
        ...

# Compare struct definitions from old and new ASTs.
# If a struct's AST has changed but its version in PG_REGISTER has not,
# report an issue.

Comment on lines +132 to +134
} catch (err) {
core.setFailed(`Failed to post comment: ${err}`);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Replace core.setFailed(...) with throw new Error(...) in the actions/github-script step, as the core object is not available in that context and would cause a reference error. [general, importance: 8]

Suggested change
} catch (err) {
core.setFailed(`Failed to post comment: ${err}`);
}
} catch (err) {
throw new Error(`Failed to post comment: ${err}`);
}

Comment on lines +30 to +38
- 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Add a step to the workflow to explicitly fail the job if the check script returns an exit code of 2, ensuring that script errors are not ignored. [possible issue, importance: 7]

Suggested change
- 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
- name: Fail on script errors
if: steps.pg_check.outputs.exit_code == '2'
run: |
echo "::error::PG version check script failed with exit code 2"
exit 2

Comment on lines +14 to +163
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")
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Quote variable expansions (e.g., "$ISSUES_FILE", "$BASE_COMMIT..$HEAD_COMMIT") to avoid failures with whitespace/globs and to make cleanup robust. [Learned best practice, importance: 5]

Suggested change
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")
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
ISSUES_FILE="$(mktemp)"
trap 'rm -f "$ISSUES_FILE"' EXIT
...
CHANGED_FILES="$(git diff --name-only "$BASE_COMMIT..$HEAD_COMMIT" | grep -E '\.(c|h)$' || true)"
...
local diff_output
diff_output="$(git diff "$BASE_COMMIT..$HEAD_COMMIT" -- "$file")"
...
if [ -s "$ISSUES_FILE" ]; then
...
cat "$ISSUES_FILE"
exit 1
fi

Comment on lines +36 to +41
output=$(bash .github/scripts/check-pg-versions.sh 2>&1)
exit_code=$?
echo "exit_code=${exit_code}" >> $GITHUB_OUTPUT
echo "output<<EOF" >> $GITHUB_OUTPUT
echo "$output" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Use a unique delimiter and printf when writing multiline output to $GITHUB_OUTPUT so script output cannot accidentally terminate the block or break formatting. [Learned best practice, importance: 6]

Suggested change
output=$(bash .github/scripts/check-pg-versions.sh 2>&1)
exit_code=$?
echo "exit_code=${exit_code}" >> $GITHUB_OUTPUT
echo "output<<EOF" >> $GITHUB_OUTPUT
echo "$output" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
output="$(bash .github/scripts/check-pg-versions.sh 2>&1)"
exit_code=$?
delimiter="EOF_$(date +%s%N)"
{
echo "exit_code=${exit_code}"
echo "output<<${delimiter}"
printf '%s\n' "$output"
echo "${delimiter}"
} >> "$GITHUB_OUTPUT"

@github-actions
Copy link

Branch Targeting Suggestion

You've targeted the master branch with this PR. Please consider if a version branch might be more appropriate:

  • maintenance-9.x - If your change is backward-compatible and won't create compatibility issues between INAV firmware and Configurator 9.x versions. This will allow your PR to be included in the next 9.x release.

  • maintenance-10.x - If your change introduces compatibility requirements between firmware and configurator that would break 9.x compatibility. This is for PRs which will be included in INAV 10.x

If master is the correct target for this change, no action is needed.


This is an automated suggestion to help route contributions to the appropriate branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants