feat: use quote to generate code#50
feat: use quote to generate code#50JiangengDong wants to merge 4 commits intosequenceplanner:masterfrom
Conversation
|
Looks like a good start! I think the errors you get on galactic are related to the ros constants either being renamed or maybe moved between the ros versions. I vaguely remember hacking around it, I can take a closer look when I find some time. |
|
I looked at the issue with the constants and I see I had already written a comment about it in the code which I'm sure you saw. I think the cleanest way to handle it is to do conditional compilation on the ROS_DISTRO environment variable, and just have different cases for humble and foxy, galactic (I think these should behave the same). Then we don't need to duplicate the constants.
Yes that sounds like a good idea. We could do that later as a separate pr.
My initial feeling is that this will be more trouble than its worth, but feel free to take a stab at it, it could be fun 😄 |
|
Hi, I pushed some code that sets configuration options depending on the ROS_DISTRO. If you rebase on top of ccdfc1e you can use |
316a833 to
501243b
Compare
|
Pushed a fix. I tested this under foxy, galactic and humble, and all tests passed. |
|
Hi! I tried your branch now but I noticed some issues because I have the Also now I think we can just do conditional compilation in |
501243b to
5056593
Compare
|
Just a heads up that I force pushed some changes to your branch. I rebased on top of the new CI, fixed the issues with test_msgs, and had to add a temporary hack around the type name for static arrays for constants. But I figure we deal with that last bit once we rework the parsing of the constants. |
|
Hi @JiangengDong, I've done a parallel work on #58 and you may take a look. |
5056593 to
f9675c0
Compare
|
Hi @m-dahl sorry for my slow progress, and thank you for fixing it. I just rebased the branch to resolve the conflict with master. @jerry73204 I think your PR has more features than mine. One suggestion about the codegen part is that the snippet for checking the array size appears repeatedly in @m-dahl @jerry73204 what's your idea about these two PRs? I don't mind closing mine because the other one looks more feature-complete (need more cleanup, though). |
|
@JiangengDong @jerry73204 Thank you both for the improvements. It's unfortunate that there was duplicate work but that is just how it is. I am travelling at the moment and do not have much time to look at the PRs in detail the coming week. But as you say @JiangengDong it looks like @jerry73204's PR includes both using syn for parsing and also using quote in the build.rs files. But you hade some other nice cleanups included in this PR that I think we want. My suggestion would be to merge @jerry73204's pr to master as it is, and then we can open new PRs that adds some of @JiangengDong's more general code cleanups. |
|
This sounds good to me. I am going to close this PR and open a new one after #58 is merged. @jerry73204 let me know if you need any help on your PR. |
|
Cool @JiangengDong . I just merged #58. |
Rewrite the codegen with
quote!, so it will be easier to add new features like the static array.Progress:
r2r_msg_gencargo checkpassescargo testpassesr2r's build script to also usequoteOther thoughts
r2r_msg_genis used as a build-dependency ofr2ronly. After this PR,proc-macro2andquoteare runtime dependencies ofr2r, which is not a good idea.synto parserust-bindgen's output, instead of accessingrosidl_typesupport_introspection_c__MessageMembers