Replace SWIG XML with libclang for message payload struct parsing#1343
Replace SWIG XML with libclang for message payload struct parsing#1343juan-g-bonilla merged 5 commits intodevelopfrom
Conversation
juan-g-bonilla
added
dont merge
juan-g-bonilla
changed the title
|
@codex review and add a release not snippet |
Summary
Testing
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d57907d39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
juan-g-bonilla
force-pushed
the
feature/1342-payload-meta-json
branch
3 times, most recently
from
c7ff076 to
aa5cc36
Compare
|
Great work! This is a very nice and welcome feature. Thanks for the ping @juan-g-bonilla! The JSON metadata would definitely be useful for the bsk-ros2 bridge. I'm currently regex-parsing the *Payload.h structs to auto-generate ROS 2 message definitions, so having structured type info (array shapes, sizes, comments) available as JSON would be a nice improvement over that. Looking forward to this landing! |
We were very happy to learn about your work and see Basilisk used in a new ecosystem, so it’s a pleasure to help out if we can! I also happen to be friends with Pedro Roque at Caltech, so I’ll try to get a beer out of him if this PR ends up helping him too. |
schaubh
force-pushed
the
feature/1342-payload-meta-json
branch
from
aa5cc36 to
e796c64
Compare
schaubh
changed the title
juan-g-bonilla
force-pushed
the
feature/1342-payload-meta-json
branch
2 times, most recently
from
8892d15 to
c45f70c
Compare
|
@ReeceHumphreys , with the BSK SDK out, could you please take a look at this branch as well. I think this will impact how we make an SDK for BSK v2.11. Thanks for the feedback. In particular, you mentioned your idea to me this week about wrapping messages with protobuffers to make the SDKs more robust to changes in the message definitions. Does this work complement this, compete with this? |
schaubh
left a comment
schaubh
left a comment
There was a problem hiding this comment.
[P2] generatePayloadMetaJson.py (line 154) ignores fatal diagnostics other than unknown type name, so malformed payload headers can silently generate wrong metadata. I reproduced this locally with a minimal double a[N]; payload where N was undefined; the parser emitted a scalar double field and reported the struct as complete.
juan-g-bonilla
force-pushed
the
feature/1342-payload-meta-json
branch
from
c45f70c to
8feac7a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8feac7a36e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add generatePayloadMetaJson.py, which uses libclang to parse *Payload.h headers and emit a JSON description of each struct's fields (kind, ctype, size, offset, enum values, comments). This replaces the SWIG -xml approach and provides richer per-field data for downstream code generation. Add test_generatePayloadMetaJson.py with coverage for primitives, multi-word types, type aliases, arrays, enums, pointers, nested structs, incomplete types, and C vs C++ parsing modes. Add libclang>=15,<=18 to requirements_dev.txt.
generateSWIGModules.py now reads the JSON produced by generatePayloadMetaJson.py
instead of SWIG XML. It generates typed __init__ (keyword-only, with docstring),
__fields__, and properly typed and with documentation attributes for each payload class.
msgInterfacePy.i.in gains {extraShadowFeature} and {extraPythonContent}
placeholders so the code-generated blocks are injected at the right SWIG points.
newMessaging.ih: remove the hand-written __init__ %feature("shadow") and
__fields__ classmethod (now generated per-type); remove inspect import.
Replace the SWIG -xml step with generatePayloadMetaJson.py to produce per-message .json files under autoSource/cMsgMeta/. Pass the JSON path to generateSWIGModules.py and add generatePayloadMetaJson.py / msgInterfacePy.i.in / generateSWIGModules.py as explicit DEPENDS so the target rebuilds when any of those generator scripts change. Also auto-detect the correct -x language flag (c vs c++) based on the payload directory, so C payloads parse cleanly with libclang.
juan-g-bonilla
force-pushed
the
feature/1342-payload-meta-json
branch
from
8feac7a to
01f5ac1
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01f5ac1af2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Weighing in on more detail in Slack but the gist is this PR would change the way the SDK works a bit as we depend on the XML file for generating the SWIG messages. We can refactor it to work with this new system though. The capnp changes are pretty independent of any of this. |
schaubh
left a comment
schaubh
left a comment
There was a problem hiding this comment.
Good to go. Thanks for contributing this change.
4 participants
Description
Replaces the SWIG
-xmlstep with alibclang-based parser (generatePayloadMetaJson.py) that produces a JSON description of each*Payload.hstruct.generateSWIGModules.pyconsumes this JSON to emitRECORDER_PROPERTY_*macros as before, but now with accurate C-level type metadata (exact sizes, enum values, array shapes, alignment/padding) instead of relying on SWIG XML internals.The richer metadata also enables two improvements to the generated SWIG bindings for each payload class:
__init__with per-field type annotations and docstrings sourced directly from the header comments, replacing the generic**kwargsconstructor.@propertydescriptors and a__fields__classmethod.These improvements allow for nicer IDE support around payloads:
Commits are organized as: (1) new libclang parser + tests, (2) refactor
generateSWIGModules.pyand update the SWIG templates, (3) wire the new pipeline into CMake.libclang(viapip install libclang) is added as a dev dependency. The parser bundles minimal stub headers so no system Clang installation is required.Verification
Unit tests were added in
test_generatePayloadMetaJson.pycovering primitives, multi-word C types, type aliases, 1-D/2-D arrays, enums, pointers, nested structs, incomplete types, and C vs C++ parsing modes. A full CMake build was run to confirm all existing message targets produce identicalRECORDER_PROPERTY_*output.Documentation
No existing documentation is invalidated.
Future work
If/when merged, @E-Krantz might consider using the newly generated
jsonfiles in his very cool bsk-ros workflow.