-
Notifications
You must be signed in to change notification settings - Fork 1.7k
8226 deprecated, enum parse fix, enums json #11185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: maintenance-9.x
Are you sure you want to change the base?
8226 deprecated, enum parse fix, enums json #11185
Conversation
Merge Maintenance 9.x to master
Branch Targeting SuggestionYou've targeted the
If This is an automated suggestion to help route contributions to the appropriate branch. |
PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
High-level Suggestion
Avoid committing the large, auto-generated inav_enums.json file. Instead, publish it as a build artifact or release asset to prevent bloating the repository. [High-level, importance: 7]
Solution Walkthrough:
Before:
# .github/workflows/docs.yml (or similar)
- name: Generate enum documentation
run: python docs/development/msp/gen_enum_md.py
# git repository contains:
# - docs/development/msp/gen_enum_md.py
# - docs/development/msp/inav_enums.json (large, generated)
After:
# .github/workflows/docs.yml (or similar)
- name: Generate enum documentation
run: python docs/development/msp/gen_enum_md.py
- name: Upload artifact
uses: actions/upload-artifact@v3
with:
name: inav-enums-json
path: docs/development/msp/inav_enums.json
# .gitignore
docs/development/msp/inav_enums.json
# git repository no longer contains inav_enums.json
| # ---------- Parsing regexes ---------- | ||
|
|
||
| RE_ENUM_START = re.compile(r'^\s*typedef\s+enum\s*\{') | ||
| RE_ENUM_START = re.compile(r'^\s*typedef\s+enum(?:\s+[A-Za-z_]\w*)?\s*\{') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Update the RE_ENUM_START regex to optionally match __attribute__((packed)) to correctly parse packed enums. [possible issue, importance: 6]
| RE_ENUM_START = re.compile(r'^\s*typedef\s+enum(?:\s+[A-Za-z_]\w*)?\s*\{') | |
| RE_ENUM_START = re.compile(r'^\s*typedef\s+enum(?:\s+[A-Za-z_]\w*)?(?:\s+__attribute__\(\([\w\s,\(\)]+\)\))?\s*\{') |
| if '_source' in jsonfile[e.name]: | ||
| jsonfile[e.name]['_source'] = jsonfile[e.name]['_source'].replace('../../../src', 'inav/src') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Replace the hardcoded path string replacement with a flexible regex using re.sub to normalize source paths with varying depths. [general, importance: 7]
| if '_source' in jsonfile[e.name]: | |
| jsonfile[e.name]['_source'] = jsonfile[e.name]['_source'].replace('../../../src', 'inav/src') | |
| if '_source' in jsonfile[e.name]: | |
| jsonfile[e.name]['_source'] = re.sub(r'^(?:\.\./)+src', 'inav/src', jsonfile[e.name]['_source']) |
| |---|---:|---| | ||
| | `MODE_RX` | 1 << 0 | | | ||
| | `MODE_TX` | 1 << 1 | | | ||
| | `MODE_RXTX` | MODE_RX | MODE_TX | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Fix the markdown table for MODE_RXTX by merging the MODE_RX and MODE_TX cells into a single "Value" cell, displaying MODE_RX | MODE_TX. [general, importance: 6]
| | `MODE_RXTX` | MODE_RX | MODE_TX | | | |
| | `MODE_RXTX` | MODE_RX \| MODE_TX | | |
|
The deprecation message could mention the replacement MSP2_INAV_LOGIC_CONDITIONS_CONFIGURED pattern. The Qodo bot sometimes suggests dumb things; sometimes points to a good potential improvement. |
Not sure what he means by "publish it as a build artifact or release asset", it may be gone soon anyway as i figure out a way to proprely generate C headers and a python library as part of a united SDK; i used to delete it after creation. The regex still is part of that improvement, i noticed some enums were missing so it will be getting some improvements anyway; i'm no good at regex, that is no language of man. It could be a very good idea to keep a changelog of MSP; i did take note of this page recently which hasn't been updated in years https://github.com/iNavFlight/inav/wiki/INAV-MSP-frames-changelog |
User description
Small fix to enum parsing that caused some to be missed, added a "not implemented" flag to MSP2_INAV_LOGIC_CONDITIONS, kept the inav_enums.json file for use in SDK
PR Type
Enhancement, Bug fix
Description
Fixed enum parsing regex to handle named enums correctly
Added 30+ previously missed enums to documentation
Marked MSP2_INAV_LOGIC_CONDITIONS as deprecated due to buffer overflow
Generated stable inav_enums.json file with normalized source paths
Diagram Walkthrough
File Walkthrough
2 files
Fixed enum parsing and JSON generationMarked MSP2_INAV_LOGIC_CONDITIONS as not implemented2 files
Added 30+ missing enums to referenceGenerated stable enum definitions JSON file2 files
Updated MSP2_INAV_LOGIC_CONDITIONS deprecationClarified not_implemented field documentation2 files
Updated checksum for message changesIncremented JSON file revision to 41 files