⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@sensei-hacker
Copy link
Member

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

User description

Summary

Add automated CI check that detects when parameter group structs are modified without incrementing the version number. This prevents settings corruption issues like the one highlighted in PR #11236.

Changes

  • Add .github/scripts/check-pg-versions.sh - Detection script that analyzes git diffs
  • Add .github/workflows/pg-version-check.yml - GitHub Actions workflow
  • Add .github/workflows/README.md - Documentation for all workflows

How It Works

  1. Triggers on PRs to maintenance branches that modify C/H files
  2. Scans changed files for PG_REGISTER entries
  3. Detects struct typedef modifications in the git diff
  4. Checks if the PG version parameter was incremented
  5. Posts a helpful comment if version not incremented

Detection Logic

The script:

  • Extracts struct names from PG_REGISTER lines
  • Looks for changes to the corresponding typedef struct definitions
  • Compares old and new version numbers from the diff
  • Reports potential issues with file:line references

Comment Behavior

  • Non-blocking: Posts comments but doesn't fail the build
  • Update-friendly: Updates existing comment instead of creating duplicates
  • Helpful: Explains why version increments matter and when they're needed
  • Conservative: May have false positives (comments when not needed) but won't miss real issues

Example Scenarios

Scenario 1: Field removed without version increment ✅ Detects

// Old: version 4
typedef struct blackboxConfig_s {
    uint8_t device;
    uint8_t invertedCardDetection;  // Removing
    uint32_t includeFlags;
} blackboxConfig_t;
PG_REGISTER(..., 4);

// New: still version 4 - WRONG!
typedef struct blackboxConfig_s {
    uint8_t device;
    uint32_t includeFlags;
} blackboxConfig_t;
PG_REGISTER(..., 4);  // Should be 5!

Result: ⚠️ Comment posted explaining the issue

Scenario 2: Field removed with proper version increment ✅ No comment

// Same struct change but version incremented 4→5
PG_REGISTER(..., 5);

Result: ✅ No comment (correct behavior detected)

Scenario 3: Only PG_RESET_TEMPLATE defaults changed ✅ No comment

// Struct unchanged, only default values modified
PG_RESET_TEMPLATE(myConfig_t, myConfig,
    .field = 20  // Changed from 10
);

Result: ✅ No comment (struct layout unchanged)

Why This Matters

When PG struct layout changes without version increment:

  1. pgLoad() sees matching versions (4==4) and copies old EEPROM data
  2. Field offsets have shifted, so wrong bytes are read into wrong fields
  3. Result: Silent settings corruption, unexpected behavior, potential crashes

With version increment (4→5):

  1. pgLoad() detects version mismatch
  2. Skips the memcpy, keeps defaults instead
  3. Result: Settings reset to defaults (safe, obvious to user)

References

Related Work

This automation follows the PG system documentation created in PR #11271.

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 iNavFlight#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.
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 22, 2026

PR Compliance Guide 🔍

All compliance sections have been disabled in the configurations.

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.
@sensei-hacker sensei-hacker merged commit f4dc128 into iNavFlight:maintenance-9.x Jan 22, 2026
20 checks passed
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.

1 participant