stm32h7xx-hal icon indicating copy to clipboard operation
stm32h7xx-hal copied to clipboard

Add fully type erased pin variant

Open jg2562 opened this issue 3 years ago • 17 comments

Adds type erased pins which allow for generic pin usage over code.

Solves #315

jg2562 avatar Feb 14 '22 02:02 jg2562

I have not yet gone through all the data sheets to verify that the 0x400 offset mentioned in the issue.

jg2562 avatar Feb 14 '22 02:02 jg2562

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 https://github.com/stm32-rs/stm32h7xx-hal/pull/317#issuecomment-1038558508, 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: @.***>

mlamoore avatar Feb 14 '22 08:02 mlamoore

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.

jg2562 avatar Feb 14 '22 09:02 jg2562

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.

jg2562 avatar Feb 14 '22 22:02 jg2562

@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.

jg2562 avatar Feb 15 '22 08:02 jg2562

Looks good! To get this merged, I'd like to see:

  • A new example for this in the examples folder, to help new users discover it and to get some CI coverage
  • (More) successful testing on hardware. I can help with that :)

richardeoin avatar Feb 18 '22 20:02 richardeoin

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.

jg2562 avatar Feb 19 '22 00:02 jg2562

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 https://github.com/stm32-rs/stm32h7xx-hal/pull/317#issuecomment-1045435172, 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: @.***>

mlamoore avatar Feb 19 '22 00:02 mlamoore

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

jg2562 avatar Feb 19 '22 00:02 jg2562

I finally got around to adding the example, let me know what you guys think!

jg2562 avatar Feb 25 '22 07:02 jg2562

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.

jg2562 avatar Feb 27 '22 22:02 jg2562

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 :)

richardeoin avatar Feb 28 '22 20:02 richardeoin

Yeh i could probably redo it, but what would you imagine the output looks like at that point?

jg2562 avatar Mar 04 '22 19:03 jg2562

I think the output would be a list of pins and their input value (high/low)

richardeoin avatar Mar 06 '22 15:03 richardeoin

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.

jg2562 avatar Mar 15 '22 12:03 jg2562

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.

richardeoin avatar Mar 21 '22 20:03 richardeoin

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.

jg2562 avatar Mar 21 '22 22:03 jg2562