Skip to content
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

Support .dsdl extension #12

Merged
merged 3 commits into from
Jun 22, 2022
Merged

Support .dsdl extension #12

merged 3 commits into from
Jun 22, 2022

Conversation

chemicstry
Copy link
Contributor

Dsdl now uses .dsdl extension instead of .uavcan: OpenCyphal/pydsdl#78

For compatibility purposes it is desirable to support both .uavcan and .dsdl and I don't see any reasons not to.

Also updated public_regulated_data_types to test generation. Should I update all other tests too and leave just one .uavcan as compatibility test?

Some related questions:

  • Why is canadensis_data_types pregenerated instead of using the canadensis_macro?
  • Any reason for not using git submodules for public_regulated_data_types and similar?

@samcrow
Copy link
Owner

samcrow commented Jun 21, 2022

That sounds good. Thanks for contributing.

Why is canadensis_data_types pregenerated instead of using the canadensis_macro?

The main reason is that the macro is experimental and I'm not sure if it actually provides a better user experience.

With the macro, everyone has to compile the DSDL parser and everything, run it on all the DSDL files, and generate the code at build time. IDEs are also generally less helpful with macro-generated code.

Any reason for not using git submodules for public_regulated_data_types and similar?

I was not thinking about that earlier, but you're right that it makes sense in this situation.


The tests are failing because some of the examples have zero-sized containers. The new version of heapless does not allow that. I'll fix that later.

@samcrow
Copy link
Owner

samcrow commented Jun 22, 2022

Just to check, should I merge this now or are you still planning to make the other change you mentioned?

@chemicstry
Copy link
Contributor Author

I wasn't sure if I should make them. If you'd like I can rename the rest of the DSDLs in the upcoming days, but otherwise this is ready to merge.

@samcrow samcrow merged commit fa39b7f into samcrow:master Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants