Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 169 additions & 0 deletions .github/scripts/check-pg-versions.sh
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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
#!/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")
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
Comment on lines +14 to +163
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

exit 1
else
echo ""
echo "${GREEN}✅ No PG version issues detected${NC}"
exit 0
fi
127 changes: 127 additions & 0 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
@@ -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)
Loading