search

stm32-rs / stm32f7xx-hal

A Rust embedded-hal HAL for all MCUs in the STM32 F7 family
Apache License 2.0
115 stars 65 forks source link

adc? #116

Open pdgilbert opened 3 years ago

pdgilbert commented 3 years ago

Where is adc? It is in device:: in release 0.2.0. On git device:: has disappeared but adc does not seem to be in any of the usual places.

hannobraun commented 3 years ago

Was device a re-export of the PAC (the stm32f7 crate)? If so, this is now available as pac. The HAL itself doesn't seem to have ADC support.

pdgilbert commented 3 years ago

It seems the stm32f7 crate has modules stm32f7::stm32f7x3 and stm32f7::stm32f7x9 and the first has adc_common and adc2 while the second has adc_common and adc1. It looks likestm32f7xx_hal has pac::adc2 but not pac::adc1. I don't really know enough about this to say if that makes any sense, but perhaps there is a re-export omission?. (Also, I don't know much about device:: other than having found adc1 there.)

hannobraun commented 3 years ago

The stm32f7 crate supports the whole STM32F7 series, which consists of several product lines, like STM32F7x3, STM32F7x9, and so forth. Those modules in stm32f7 correspond to those product lines, and things (like adc1) might be missing from those modules because they are not available on the hardware, or because the SVD file (a vendor-provided file that describes the hardware peripherals) has issues. It seems implausible that ADC1 is missing but ADC2 is available, so I'm suspecting a problem with the SVD file.

To make things slightly more complicated, the modules of the stm32f7 crate are only available, if support for that product line is enabled via a compile-time feature flag. So depending on where you look (the documentation on docs.rs for example), not all of the available modules might be visible.

stmf32f7xx-hal also has compile-time feature flags (see Cargo.toml). When you enable the right feature for your target hardware, stm32f7xx-hal automatically enables the corresponding feature in stm32f7, and re-exports the corresponding product line module as stm32f7xx_hal::pac (see lib.rs).

So saying something like "pac:::adc1 is missing" is only true for specific configurations.

In any case, since this HAL only re-exports the contents of the PAC, there's not much we can do here to solve your problem. If I were in your situation, I'd ask myself the following questions:

  1. Are you enabling the correct feature for your target hardware?
  2. Does the PAC, when built with that feature, contain all the hardware peripherals that are available on the target hardware? I'd check that using locally built documentation, to make sure you're looking at the correct configuration (for example, by running something like cargo doc --features stm32f722 --open from the root of this repository).

If peripherals that should be available on the target hardware are indeed missing, that's a problem in the PAC, which is defined in the stm32-rs repository. I suggest opening an issue (or pull request) there.

pdgilbert commented 3 years ago

(Sorry, I wrote this weeks ago, but forgot to click and it got lost on a browser tab.)

Thank you for the extended explanation! As you may have guessed, I do not have actual stm32f7 hardware and I am just trying to compile some examples. So I can easily use adc2 or another device. I probably need to understand stm32f7 better before I get into whether the SVD is missing something. It seems strange to me that adc2 is there but not adc1, but I am also struggling to get the adc type correct and other things. So I probably need some additional feature settings or something. This is complicated a bit by my using setup() functions for which I want to get impl traits working.

Looking at cargo doc --features stm32f722 --open it does appear that adc2 is there but not adc1.

I have examples building using adc on other device hals, and other examples building on stm32f7xx_hal (summary at https://pdgilbert.github.io/eg_stm_hal/). Many have also been tested on hardware that I do have.

pdgilbert commented 2 years ago

I've looked at this more now but will not claim that I really understand it. The stm32f7x2.svd file has support for adc so I do not think that is the problem. To a novice like me it looks more likely the problem is that in https://github.com/stm32-rs/stm32f7xx-hal/blob/0a0d06d5f63c3a1471362370fdfb8c367207d308/src/lib.rs#L154-L155 the adc support is only activated for very few devices while it should be available on many more.

Regarding reading the internal temperature there is also a problem that there is no (easy) way to access the internal channel used for the MCU temperature sensor, even on the MCUs that have adc support enabled. In stm32f4xx_hal and some others this is done with a variable Temperature (see https://github.com/stm32-rs/stm32f4xx-hal/blob/1faf35a27c754f00b22d510754a2fb598763c029/src/adc.rs#L1063-L1065). Temperature may have previously been in the device module in stm32f7xx_hal but it is not in the current adc module.

hannobraun commented 2 years ago

The stm32f7x2.svd file has support for adc so I do not think that is the problem.

Which version of the SVD file are you looking at? The PAC (that this HAL is based on) seems to be based on the files in this directory. As of this writing, the F7 files were updated 2 years ago, so is it possible you were looking at a newer version?

The file is also patched and transformed during the PAC building process, so a problem might be introduced there. I am not familiar with this though, so no idea if it's a problem.

the adc support is only activated for very few devices while it should be available on many more.

That is true, but please note that it's a completely separate problem from what was discussed here earlier. Originally, we were discussing that some ADC functionality was missing from the PAC. Back then, no ADC support existed in this HAL at all. The API was added in July, months after the previous discussion on this issue happened.

It seems the contributor who added the ADC API enabled it for the hardware they developed it for, which is a sensible thing to do. If you can confirm that the API works for other hardware too, extending the number of devices it's enabled for makes sense, and would be a welcome contribution.

Regarding reading the internal temperature there is also a problem that there is no (easy) way to access the internal channel used for the MCU temperature sensor, even on the MCUs that have adc support enabled.

Making more hardware features available in the API would definitely be a welcome addition.

Temperature may have previously been in the device module in stm32f7xx_hal but it is not in the current adc module.

The device module was just a re-export of the PAC (i.e. the stm32f7 crate), and has been renamed to pac. Theoretically, all functionality that is present in the SVD should be exposed there.

pdgilbert commented 2 years ago

The stm32f7x2.svd I was looking at was from https://github.com/tinygo-org/stm32-svd. It does look to be newer and using a simple grep ADC | wc it looks like there has been some work done on adc. Just to be clear, this is the first SVD file I have ever looked at, and I am a novice to embedded and still new to rust. So it is perfectly fair to assume I don't know what I'm talking about.

I realize I have introduced something slightly different from the earlier discussion but I thought adc? was probably broad enough that I did not need to start an new issue. My remark that Temperature may have previously been in the device was based only on an early version of an example I am working on. I don't think the earlier version actually ever worked, so it probably was not there either.

Making more hardware features available in the API would definitely be a welcome addition

Apart from the problem that this is a bit beyond my current abilities there is the chicken and egg problem. I might consider getting hardware if I can get examples to build. My current project is to get several different examples building with several different device hals. (CI at https://github.com/pdgilbert/rust-integration-testing/actions) The temperature example, which is the only one using internal adc, fails on stm32f722 but I can get building using stm32f769. I have to fake the internal temperature because the channel is not available.

hannobraun commented 2 years ago

The stm32f7x2.svd I was looking at was from https://github.com/tinygo-org/stm32-svd. It does look to be newer and using a simple grep ADC | wc it looks like there has been some work done on adc. Just to be clear, this is the first SVD file I have ever looked at, and I am a novice to embedded and still new to rust. So it is perfectly fair to assume I don't know what I'm talking about.

SVD files are often incomplete and full of errors, so it's very plausible that someone patched it, or that ST released a new version, and that those changes haven't made their way into stm32-rs. Once someone does the work of improving the situation in stm32-rs, the improvements will trickle down to this HAL.

I realize I have introduced something slightly different from the earlier discussion but I thought adc? was probably broad enough that I did not need to start an new issue.

Yeah, totally. I wasn't objecting, just providing context in case it wasn't totally clear.

Apart from the problem that this is a bit beyond my current abilities there is the chicken and egg problem. I might consider getting hardware if I can get examples to build.

All I'm saying is, there's no grand plan to leave certain features out, or anything like that. Availability of features is driven by contributors who need them, and I just wanted to make it clear that you, or anyone else, are welcome to make improvements that adapt the HAL to your needs.

adamgreig commented 2 years ago

SVD files are often incomplete and full of errors, so it's very plausible that someone patched it, or that ST released a new version, and that those changes haven't made their way into stm32-rs. Once someone does the work of improving the situation in stm32-rs, the improvements will trickle down to this HAL.

In this specific case, the SVDs from tinygo-org/stm32-svd are actually exactly the SVDs from stm32-rs (or older, since there's been a handful of recent stm32f7 updates). So there shouldn't be anything in that SVD that's not in the PAC. For reference, you can always download the latest built SVDs from stm32-rs here too.

Currently stm32f7xx-hal uses stm32f7 v0.11.0, but the current version is 0.13 and I'm going to publish 0.14 in a few days. Looking at the Changelog I'm not sure how many ADC improvements for F7 there are since then, though.

hannobraun commented 2 years ago

Thanks for the note, Adam!

Currently stm32f7xx-hal uses stm32f7 v0.11.0, but the current version is 0.13 and I'm going to publish 0.14 in a few days. Looking at the Changelog I'm not sure how many ADC improvements for F7 there are since then, though.

Oh man, we've really gotten behind. I somehow implicitly assumed we were on the latest version. stm32l0xx-hal has been updated recently, so that probably confused my into thinking we were up-to-date here too :smile:

pdgilbert commented 2 years ago

Ok, I look forward to testing again when things are up-to-date. Thanks.

zvladimir commented 9 months ago

I have also encountered a problem with not being able to use the ADC. I'm using an STM32F723 microcontroller on a development board, and based on the code https://github.com/stm32-rs/stm32f7xx-hal/issues/116#issuecomment-929524511

it seems like I can't use the ADC for this controller, which is very strange because it is supported by the hardware. Are there any ways to get the ADC working?