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

Enable all valid (and only valid) alternate functions for GPIO pins #21

Merged
merged 38 commits into from
Nov 14, 2019

Conversation

IamfromSpace
Copy link
Contributor

This PR adds the appropriate alternate function methods for all pins and devices, and notably does not expose any alternate functions that do not appear in their corresponding reference manuals.

Two lists are added to the existing GPIO macro, one for alternate functions that are common among all devices being configured, and the other that is for alternate functions that are specific to a subset of the devices (and includes a list of them).

@dfrankland
Copy link
Member

Wow this is amazing. Allow me to try to build a couple of my existing projects with your branch to check that there's no regressions.

@IamfromSpace
Copy link
Contributor Author

Thanks! I’m glad it seems useful. Definitely good to get it through regression since this does remove methods previously available—though none of them theoretically did anything.

I’m also doing the final proof pass today, and have spotted a couple errors that need fixing. Hoping to get that pushed up by this evening.

@IamfromSpace
Copy link
Contributor Author

Should be it for corrections. Got tripped up by a few omitted columns in a some reference manual tables. Should all be sorted out now!

For reference, I (manually, my attempts at automation failed) pulled the reference sheet AF data into json files here then wrote some quick and dirty JS to output the pin definitions. Going forward hopefully necessary updates are small enough that they can be done by hand.

@dfrankland
Copy link
Member

dfrankland commented Nov 14, 2019

I've been testing out on some of the examples I've written over on https://github.com/dfrankland/proton-c with this branch and for the most part things work. I think there was a small issue with the upgrade to 0.3.0 and how it works with stm32-usbd because some examples involving USB fail now on master of this repository. I was able to fix it however with the updates in #24. To fully ensure there's no regression, I hope you don't mind if we merge that in before this.

@dfrankland
Copy link
Member

I rebased these changes on to master and tested using the simple keyboard example here: https://github.com/dfrankland/proton-c/tree/test-new-af-gpio. Everything worked nicely and all device features built successfully.

@dfrankland dfrankland merged commit 902b56f into stm32-rs:master Nov 14, 2019
@Sh3Rm4n
Copy link
Member

Sh3Rm4n commented Nov 14, 2019

Warning are now apearing with this pr:

Running `cargo check --features=stm32f328,rt`...
    Checking stm32f3xx-hal v0.3.0 (/home/fabian/Projects/stm32f3xx-hal)
warning: method is never used: `afr`
    --> src/gpio.rs:288:21
     |
288  |                       pub(crate) fn afr(&mut self) -> &$gpioy::AFRL {
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
642  | / gpio!([
643  | |     {
644  | |         devices: [
645  | |             "stm32f301",
...    |
2162 | |     },
2163 | | ]);
     | |___- in this macro invocation
     |
     = note: `#[warn(dead_code)]` on by default

I guess there is no easy way to fix it, just wanted to note.

@dfrankland
Copy link
Member

Yeah, I noticed this too. We can create a feature cfg to include this for the relevant crates, but I figured that the warnings weren't too bad.

@IamfromSpace
Copy link
Contributor Author

Ah, thought I’d taken care of most of these cases, must have missed it in the final pass. It’s pretty tough to fix in the macro, which is why AFRH allowed for dead code previously: https://github.com/IamfromSpace/stm32f3xx-hal/blob/bb40b02eb2c99702ae5bb87a2d41abc0d97ec613/src/gpio.rs#L266

I’ll open a fix for it soon, but I’m traveling for a bit, so it’ll be tough for me to validate it covers everything.

This pull request was closed.
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.

3 participants