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

[Fixes #17] Update SVDs from master and regenerate with svd2rust 0.24.1 #21

Conversation

michalfita
Copy link
Collaborator

I've bumped up rust2svd locally and regenerated all PACs, this time from the master branch of https://github.com/posborne/cmsis-svd/ repository, as cmsis-svd/cmsis-svd#154 have been merged recently.

@michalfita michalfita added the release Release preparation label Jul 24, 2022
@michalfita michalfita requested a review from tmplt July 24, 2022 16:10
@tmplt
Copy link
Member

tmplt commented Jul 24, 2022

I've been thinking how we are handling PACs at the moment. Is it possible to remove all generated code and dynamically svd2rust them when needed via some build.rs? Would every PAC need a build.rs?

Getting rid of these 50k files would be for the better.

I don't think this will be the last PAC bump. I have some SVD discrepancy bugs filed at Microchip.

@michalfita
Copy link
Collaborator Author

michalfita commented Jul 24, 2022

This idea is worth consideration. It would help if svd2rust can be used as library crate (dev-dependency). Same applies to form.

Need to do some research. Yes, each crate would need it's own build.rs, but probably we can either minimize differences between them or generate them automatically.

One of problems to tackle is how to run svd2rust only once per version, as it generates the full crate and do not check whether recreation is required.

@michalfita
Copy link
Collaborator Author

OK, I think we have some answers:
rust-embedded/svd2rust#392

@michalfita
Copy link
Collaborator Author

Problem 2: how to deliver right SVD with the crate to the user without invoking more less curl to run extra download?

@tmplt
Copy link
Member

tmplt commented Jul 25, 2022

how to deliver right SVD with the crate to the user without invoking more less curl to run extra download?

I do not like the idea of downloading remote assets, unless sha265-summed, in the worst case. Either we continue the cmsis-svd approach or just vendor the SVDs in the repo. Or have I misunderstood the problem?

@michalfita
Copy link
Collaborator Author

I don't like either.

I've done some research already - the problem to solve before publishing each PAC crate is to put the right SVD in the directory, where build.rs can find it, but at the same time we need to have link to right one in the submodule so anyone wishing to use the repository clone can still use it. I had script to do the publishing somewhere, but I can't find it, so I'll have to write new one. (If rust-lang/cargo#6817 did what it says on the tin, it wouldn't be issue then.)

Now, what I started is MVP for one PAC, where build.rs can run svd2rust as library to generate the code from SVD on the fly, form it (can be used as library either, however that's undocumented). I may end up with PR to svd2rust as the generation process produces build.rs that conflicts with mine and expects device.x to ready to include!() at the time build.rs is executed. It would require function to create submodule to use in build.rs and pass device.x to it. So far things looks pretty straightforward, but it's a lot of code to make to address all intermediate needs of the process.

In the mean time I'd merge this one, so we progress toward next release and tackle the security issue found by dependabot (PR #22 more about this), because getting there with user side generation will take some time. I need to figure out some CI to validate these crates.

@tmplt
Copy link
Member

tmplt commented Jul 25, 2022

Sounds good.

As for the regeneration, the crate versions have not been bumped. But instead of bumping it to 0.24.1, bump it to 0.24.0 so that we can repurpose the PATCH semver field for SVD changes also, either from us or Microchip. That is: SVD patch or svd2rust PATCH bump -> bump PAC crates' PATCH. MINOR continues to track svd2rust's MINOR.

@michalfita
Copy link
Collaborator Author

🤔 Is it something what other maintainers do?

@tmplt
Copy link
Member

tmplt commented Jul 25, 2022

I don't understand the question.

@michalfita
Copy link
Collaborator Author

michalfita commented Jul 27, 2022

🤔 Is it something what other maintainers do?

I don't understand the question.

Question was in other words, whether other PAC maintainers keep only minor version from svd2rust and start from 0 on patch version?

I've done some updates readying these for next release.

@tmplt
Copy link
Member

tmplt commented Jul 27, 2022

Question was in other words, whether other PAC maintainers keep only minor version from svd2rust and start from 0 on patch version?

I've seen PACs that mirror the full svd2rust version, but none that co-opt the PATCH field like this.

@tmplt
Copy link
Member

tmplt commented Sep 9, 2022

This branch is now deprecated and can be closed. Refer to #24.

tmplt added a commit to GrepitAB/atsams70-rust that referenced this pull request Sep 9, 2022
This commit adds a CI (and the Dockerfile it is based upon) that:
1. builds the HAL for the atsame70n21b and atsamv71q21b targets;
2. builds the board examples;
3. checks the formatting of all code except pac/; and
4. lints the code base with clippy, expect pac/.

Closes atsams-rs#21.
tmplt added a commit to GrepitAB/atsams70-rust that referenced this pull request Sep 9, 2022
meta: add CI configuration

Closes atsams-rs#21

See merge request embedded-rust/atsamx7x-hal!14
tmplt added a commit to GrepitAB/atsams70-rust that referenced this pull request Sep 9, 2022
atsamv71_xult: use Monotonic timers instead of asm::delay

Closes atsams-rs#21

See merge request embedded-rust/atsamx7x-hal!12
@michalfita
Copy link
Collaborator Author

@tmplt I'm closing this as I don't have time to work on this now. Let's focus on getting your stuff merged and I'll do my work on PACs from there.

@michalfita michalfita closed this Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release preparation
Development

Successfully merging this pull request may close these issues.

2 participants