treewide: Apply changes for new svd2rust API #545
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Via Rahix/avr-device#157, we have now upgraded to an even newer version of svd2rust, making more changes necessary. So far, I have discovered:
Footnotes |
Rahix
added a commit
to Rahix/avr-device
that referenced
this pull request
Apr 30, 2025
After starting work on porting avr-hal to the new version of avr-device, I started hitting the effects of the changed identifiers in the generated API. In Rahix/avr-hal#545, commit Rahix/avr-hal@ef69e1b, I at least got to the point of getting the Arduino Uno examples to compile again. I have to say, I absolutely do not like the code churn we introduce with this. While it is probably manageable for avr-hal with team effort, that's not all: - All downstream users will have to work through the same churn of (sometimes non-trivial) identifier changes. - All current PRs in avr-hal will need to be rebased by their authors to the new style identifiers. - Legacy examples found on the web all become outdated. - Rust compiler error messages on fixing the names are quite hit-or-miss. The update strategy is not clear if you don't understand what has happened. I took a look around the ecosystem and others have also decided to stick to what svd2rust calls the "legacy" identifier theme. I am much in favor of doing the same. There is no significant advantage to doing this change now, so let's not waste huge amounts of effort on it.
|
Not at all a friend of the code churn produced by the identifier changes. See commit ef69e1b and Rahix/avr-device#197. |
Rahix
added a commit
to Rahix/avr-device
that referenced
this pull request
Apr 30, 2025
After starting work on porting avr-hal to the new version of avr-device, I started hitting the effects of the changed identifiers in the generated API. In Rahix/avr-hal#545, commit Rahix/avr-hal@ef69e1b, I at least got to the point of getting the Arduino Uno examples to compile again. I have to say, I absolutely do not like the code churn we introduce with this. While it is probably manageable for avr-hal with team effort, that's not all: - All downstream users will have to work through the same churn of (sometimes non-trivial) identifier changes. - All current PRs in avr-hal will need to be rebased by their authors to the new style identifiers. - Legacy examples found on the web all become outdated. - Rust compiler error messages on fixing the names are quite hit-or-miss. The update strategy is not clear if you don't understand what has happened. I took a look around the ecosystem and others have also decided to stick to what svd2rust calls the "legacy" identifier theme. I am much in favor of doing the same. There is no significant advantage to doing this change now, so let's not waste huge amounts of effort on it.
This allows us to see which targets are currently failing the build.
As avr-device is upgrading to use svd2rust version 0.33.1, there are
some significant changes in the generated API. We have to adapt the HAL
code to use the new API whereever relevant.
This commit was mostly generated using the following command, which adds
the parentheses behind each register access to change it from
struct-field access to method call.
cargo build --message-format json 2>/dev/null \
| jq '.message.children[].spans[] | {file: .file_name, line: .line_start, col: (.text[0].highlight_start - 1), insert: .suggested_replacement}' 2>/dev/null \
| jq -r '"sed -ri '"'"'" + (.line | tostring) + "s/^(.{" + (.col | tostring) + "})/\\1" + .insert + "/'"'"' $(cd ../..; realpath " + .file + ")"' \
| sort | uniq | bash
Shell magic for the win :)
Beyond this, .bits() had to be converted to .set() where safe accesses
are performed.
avr-device no longer has a `critical-section-impl` feature. Instead it now has a `critical-section` feature that is enabled by default [1]. As such, we can drop the re-exposed feature here. [1]: Rahix/avr-device#195
This reverts commit ef69e1b.
|
Rahix/avr-device#197 was agreed upon, so I reverted the identifier changes here. |
Previous versions of `avr-device` allowed accidentally mis-specifying the MCU in the interrupt definition. Now that this is caught, fix one such mistake here.
|
Closing in favor of #656 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As avr-device is upgrading to use svd2rust version 0.33.1 (see Rahix/avr-device#155), there are some significant changes in the generated API. We have to adapt the HAL code to use the new API whereever relevant. This PR is a preview of what will change.
The following major changes are necessary:
.set()method instead of.bits().Fortunately, these changes can be trivially automated from rustc error messages using the following script: