Add fully type erased pin variant #317
Conversation
|
I have not yet gone through all the data sheets to verify that the 0x400 offset mentioned in the issue. |
|
That 0x400 offset won't be in the datasheet, it will be in the Technical
Reference Manual. For example, RM0433 describes the STM32H742, 743, 750,
753 and section 10.4.7 (of rev 6) shows the GPIO register map which shows
that all GPIO blocks have the same register layout. Section 2.3.2, table 8
(of RM0433 rev 6) is 'Memory and bus architecture->Memory
organization->Memory map and register boundary addresses->Register boundary
addresses' and from that table we can see that GPIOA starts at 0x58020000
and B, C, through K all start 0x400 after the previous block.
Looking at Cargo.toml, this crate covers chips described in RM0433, RM0399,
RM0455, and RM0468. I just checked those four documents, and they all have
the same layout and 0x400 spacing, except that GPIOI doesn't exist on the
RM0488 parts and its 0x400 is skipped. Looking at our code, GPIOI is
conditionally excluded for the RM0468 parts, and GPIOJ/GPIOK always have
index 9/10, so the registers will still line up properly.
This code looks good to me, I double-checked that all the correct registers
and bits are being read/written for these parts (ST hasn't changed any
meanings of bits relative to the STM32F4xx parts). I haven't tested this on
an actual microcontroller, though.
…On Sun, Feb 13, 2022 at 8:23 PM Jack ***@***.***> wrote:
I have not yet gone through all the data sheets to verify that the 0x400
offset mentioned in the issue.
—
Reply to this email directly, view it on GitHub
<#317 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAUS5FI6EK37GE6WUMTM2HLU3BRQPANCNFSM5OKDO6XA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
I should be able to check it in about a day with an actual microcontroller and digital analyzer for one of the variants. Thanks for the info! I will double check it for the proposed supported architectures in the other PR as well. |
src/gpio.rs
Outdated
| /// This is useful when you want to collect the | ||
| /// pins into an array where you need all the | ||
| /// elements to have the same type | ||
| pub fn erase(&self) -> ErasedPin<MODE> { |
There was a problem hiding this comment.
Please use 4 spaces for indentation, not tabs.
There was a problem hiding this comment.
Fixed! Apologies, i thought cargo fmt would handle that.
|
Are there any thoughts about having the pins include the fmt trait? I left it out from the stm32f4 port because of a error formatting the pin mode. We could potentially ditch the mode in the fmt or use their stripped type name function. |
|
@mlamoore I didn't realize that those 4 reference manuals that you checked covered all currently created microcontrollers of the stmh7 family, I just thought it just was the supported ones in this crate. I double-checked them as well, and it looks just like you said, so it should work just fine. Also, I was able to test on hardware today, for the most part I was getting the expected behavior, but a few pins weren't working. I don't know if it relates to this change or something else, I will be checking in on that more tomorrow. EDIT: I checked it out, it seemed unrelated to this change. However, it might be helpful if someone else could double check. |
src/gpio.rs
Outdated
| let block_ptr = (crate::pac::GPIOA::ptr() as usize + offset) | ||
| as *const crate::pac::gpioa::RegisterBlock; | ||
|
|
||
| unsafe { &*block_ptr } |
There was a problem hiding this comment.
Can be simplified to just
unsafe { &*crate::pac::GPIOA::ptr().add(offset) }I don't think the FIXME applies any more
|
Looks good! To get this merged, I'd like to see:
|
|
I have a testing program I used, where pretty much all the pins are individually written high then low in a loop and then by connecting them to pin A0 you get a report back on which pin you just probed. It's kinda of cumbersome, but it is helpful for debugging and an example of using erased pins in an array. This example does use the Debug trait I mentioned earlier, though. Otherwise, I could come up with a small example of a few differnet pins from different ports in the same array if you'd like. Also thanks! I'm definitely having some odd behaviors on a few select pins, but I think that this is not related to this PR. So it'd be very helpful to have a second opinion. |
|
For a debug trait, you could leave out the mode for now, just debug as
P(4,11) instead of P(4,11)<INPUT>.
There's another conversation right now about using the log crate, I wonder
if we could do that for this example too?
…On Fri, Feb 18, 2022, 6:35 PM Jack ***@***.***> wrote:
I have a testing program I used, where pretty much all the pins are
individually written high then low and then by connecting them to pin A0
you get a report back on which pin you just probed. It's kinda of
cumbersome, but it is helpful for debugging and an example of using erased
pins in an array. This example does use the Debug trait I mentioned
earlier, though. Otherwise, I could come up with a small example of a few
differnet pins from different ports in the same array if you'd like.
Also thanks! I'm definitely having some odd behaviors on a few select
pins, but I think that this is not related to this PR. So it'd be very
helpful to have a second opinion.
—
Reply to this email directly, view it on GitHub
<#317 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAUS5FLTXSO6SUF2QQS3ZYDU33QUXANCNFSM5OKDO6XA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Yeah, leaving out the mode would probably be easiest for now. I was planning on formatting it with the port as the ASCII representation of the port. so A0 would just become 'PA0'. In terms of the example, I'll have to learn a bit about the logging, but that does sound like an appropriate idea for the example |
|
I finally got around to adding the example, let me know what you guys think! |
There was a problem hiding this comment.
Thanks, looking good!
- It might be a bad idea to run the example on a development board, because the development board hardware drives many of the pins. For some pins, it would cause two outputs to drive each other. To avoid this the example could just make all the pins inputs and print the current state? (high or low)
- Could you run rustfmt to make the formatting consistent with the rest of the project? You can follow the Quick Start section on the rustfmt page https://github.com/rust-lang/rustfmt
Sorry about the formatting, still getting used to running formatting and clippy after changes. That is a good point, it would not reveal if a single output is pinned out multiple times. The reason I avoided having most pins as inputs is when probing you might touch the probe to ground and I wanted to avoid powering down the entire dev kit at the time. My initial use case for this was testing the documented pinouts of a dev kit that seemed to not actually match the real pinouts. So I was simply moving the A0 probe down the header and verifying that the reports given match the documentation. Interestingly as a sidenote, I did find a few outputs were not connected to the pins they claimed. That all being said, if you think we should change the use case of the script a bit to be more beginner-friendly/useful, I would be happy to update it! Also let me know if the documentation should be improved, wasn't sure if I captured your guy's documentation standards. |
|
The explaination at the top of the example looks good, thanks. I understand the use case, but I was a little concerned that on some (more expensive) development boards there are devices directly connected to the H7 like Ethernet PHYs, SD cards, CAN transceivers and so on. Those devices have outputs, if a beginner was to run this example they might drive the output of those devices with an H7 output, which is bad. Experienced developers should know better, but it might save someone a lot of pain if the example mostly used inputs instead. The example does say "experimenting" at the top though, if you don't have time to change it then we can also just include it like this :) |
|
Yeh i could probably redo it, but what would you imagine the output looks like at that point? |
|
I think the output would be a list of pins and their input value (high/low) |
|
I just updated the example, but I haven't been able to run it due to my dev kit. It should now just read in all the inputs and print them to the rtt channel. |
|
I've now merged #334, which adds type erased pins as well as many other things(!) That's good, but means looking at this PR again and seeing if any parts of it should still be included. |
|
Oh awesome! I was very excited for #334 and figured it would eliminate the need for the majority of this PR. I would say that the only thing that didn't overlap was the example, if you would still like to keep that around. Lastly, I would like to double-check #334's debug formatting to make sure it outputs what we expect as I remember that being very f4 specific. |
Adds type erased pins which allow for generic pin usage over code.
Solves #315