Skip to content

Add ESP32 support #445

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 25, 2020
Merged

Add ESP32 support #445

merged 4 commits into from
May 25, 2020

Conversation

arjanmels
Copy link
Contributor

Adding support for ESP32 as used in new pull requests on https://github.com/esp-rs.

ESP32 has 2 level interrupt handling. First level in the cores. Second level in the periphery/software.

@arjanmels arjanmels requested a review from a team as a code owner May 20, 2020 12:00
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @burrbull (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels May 20, 2020
@arjanmels arjanmels mentioned this pull request May 20, 2020
Merged
@burrbull
Copy link
Member

can you add any tests for esp32?

@arjanmels
Copy link
Contributor Author

Ok, will look into it, but will be next week.

therealprof
therealprof approved these changes May 25, 2020
View reviewed changes
Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looks good to me, too.

@therealprof
Copy link
Contributor

bors r+

@arjanmels
Copy link
Contributor Author

@burrbull @therealprof Thanks for the lightning fast review!

@therealprof
Copy link
Contributor

bors ping

@bors
Copy link
Contributor

bors bot commented May 25, 2020

pong

@therealprof
Copy link
Contributor

bors r+

@therealprof
Copy link
Contributor

bors retry

@bors
Copy link
Contributor

bors bot commented May 25, 2020

Build succeeded:

@bors bors bot merged commit 40db112 into rust-embedded:master May 25, 2020
@burrbull
Copy link
Member

burrbull commented Jun 2, 2020

with new svd-parser:
[ERROR svd2rust] In device Espressif

Caused by:
0: In peripheral AES
1: Peripheral have registers tag, but it is empty

registers tag can't be empty according to https://www.keil.com/pack/doc/CMSIS/SVD/html/elem_registers.html

@arjanmels
Copy link
Contributor Author

@burrbull, @MabezDev Ok, I will look into fixing teh example svd, hopefully later today, but cannot promise.

@arjanmels
Copy link
Contributor Author

@burrbull I updated the svd file, so now should be ok. (I tested it with branch svd_new, but it also passed before the updates, so I might have done something wrong.)

YuhanLiin pushed a commit to YuhanLiin/svd2rust that referenced this pull request Apr 17, 2021
@bors @teskje
Merge rust-embedded#436
a0618f9 
436: Use standard wording for the MSRV note r=therealprof a=ra-kete

Part of [wg/rust-embedded#445](rust-embedded/wg#445).

svd2rust already has its MSRV documented and tested, but @adamgreig suggested updating it to use the standard wording. Since this is a special case in that the MSRV applies to the generated code, not svd2rust itself, the standard wording doesn't fully apply, so I changed it in a (hopefully) sensible way.

An open question is: Should we also document the MSRV of svd2rust itself? Looking at how the CI works, currently it's implicitly guaranteed to be the same version als the MSRV for the generated code, so we could just extend the note in the README to also include svd2rust itself.

Co-authored-by: Jan Teske <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@burrbull burrbull burrbull approved these changes

@therealprof therealprof therealprof approved these changes

Assignees

@burrbull burrbull

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arjanmels @rust-highfive @burrbull @therealprof