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

Support for ADCs #20

Closed
moshevds opened this issue Oct 10, 2019 · 11 comments · Fixed by #47
Closed

Support for ADCs #20

moshevds opened this issue Oct 10, 2019 · 11 comments · Fixed by #47
Labels
enhancement New feature or request

Comments

@moshevds
Copy link

Hi,

I want to use the analog-to-digital converters on STM32F3. Since I need to implement this anyway, I think this HAL crate is the best place to add support.

  • Can I just go ahead and submit pull requests when I have any?
  • Otherwise, if someone is already working on this, is there any way I can help?
  • I am assuming we need to implement Channel and OneShot from embedded_hal::adc. Is this correct?
  • If not, are there any known problems with those traits that make them unsuitable for the SAR and ΣΔ converters available on STM32F3 chips?

My primary goal is using the ΣΔ ADCs on STM32F373, but I think implementing SAR and other STM32F3 chips may be reasonable because of a large amount of overlap. I would also love to hear any other thoughts about this.

Greetings,
Môshe van der Sterre

@dfrankland
Copy link
Member

I don't think anyone else is working on this, so feel free to contribute! I don't know enough to help out though.

@strom-und-spiele
Copy link
Collaborator

I'm also interested in using (and therefore implementing) this.
@moshevds did you start anything you can share?

@strom-und-spiele
Copy link
Collaborator

So I've been looking into this and had wonder how to solve the following issue:
The availability of ADC channels varies for different sub-variants of stm32f3xx controllers.
This means that e.g. channel ADC1_IN11 is connected to PB0 on a f303x6 chip while there is no such channel on a f303xB.
However the PB0 on the f303xB variant is connected to channel ADC3_IN12. Which in turn is not available on the f303x6 variant as they only support ADC1 and 2.
(And of course it's on of the f303 chips I'm interested in)

Side note: I'm new to Rust in general and this crate in particular so maybe I'm just not getting it. Feel free to say so directly and enlighten me ;)

@moshevds
Copy link
Author

@strom-und-spiele I've only been experimenting, and have nothing definitive to share yet.

You are right that the chip capabilities vary, so we need a configurable implementation. I do think a shared implementation for all stm32f3xx adc configs is the way to go. After all, the C HAL provided by ST is also configurable across all STM32F3xx chips (stm32f3xx_hal_adc.c). Configuration is not a part of the embedded_hal::adc Traits, but we can learn from other configurable features in this crate (and/or other embedded crates).

However, I think the first step is implementing the traits for a hard-coded configuration, and generalizing from that. I would be happy to discuss this further. I'm on freenode/#rust-embedded if you want to reach me. My nickname on freenode is MvdS.

@moshevds
Copy link
Author

@strom-und-spiele I have taken a look at the SAR ADC on the f3discovery board, you can see my very simple readout here: https://gist.github.com/moshevds/e39e2897058b4f00ab35477986c7d980
If you change the voltage on pin PA1, you should see the readout change.

If you want to experiment with implementing the OneShot trait, it could serve as a starting point. There are 3 things that don't work as I intended them:

  • Waiting until the ADC is ready. I guess the read is probably optimized out. I added a hprintln! to give the ADC some time, and that seems to work.
  • adc1.sqr1 doesn't have functions for individual bits. I am not sure why. So I had to use the unsafe block.
  • The result is different from the C version that I tried to emulate. So one of the register changes is probably incorrect.

@strom-und-spiele
Copy link
Collaborator

@strom-und-spiele
Copy link
Collaborator

https://github.com/strom-und-spiele/stm32f3xx-hal/pull/1
example implementation for adc. currently only working for the 303xB/C/D/E

@Sh3Rm4n
Copy link
Member

Sh3Rm4n commented Dec 27, 2019

Feel free to open a PR, which is only working for the 303, for now. We can
always go up from there later.

Btw. @strom-und-spiele your link is not working anymore.

@strom-und-spiele
Copy link
Collaborator

I'll look into it and open a PR with whatever I manage to finish by the end of the day. Thanks for stating it clearly that partial work is acceptable.

@strom-und-spiele
Copy link
Collaborator

According to the reference manual, STM32F303xB/C/D/E, STM32F303x6/8, STM32F328x8,
STM32F358xC and STM32F398xE all implement the ADC module similary (with differences between 303x6/8 and 303xB/C/D/E being already addressed in PR #47 )
Now implementing it for STM32F328x8, STM32F358xC and STM32F398xE should boil down to adding the pins with the apropriate feature gates.
However I didnt' do it as I don't have the hardware to test it.

Coming to think of it, it also seems to be an apropriate take to implement it "as written" and assume it works until someone who has the hardware prooves otherwise.
Only if there is an implementation that can be used and tested, the code can improve.

I can also relate to the demand of quality, i.e. actually running your code on hardware.

Is there a best practice to this? And if not, what are your personal opinions?

@Sh3Rm4n
Copy link
Member

Sh3Rm4n commented Apr 18, 2020

Now implementing it for STM32F328x8, STM32F358xC and STM32F398xE should boil down to adding the pins with the apropriate feature gates.

That's good to hear. I'm fine if this PR only addressing the stm32f303 for now. I'm not aware of any best practices, but if differences does only come down to choosing the right pins, I would go and add them, even if this means that we develop in the blind, as we do not have the hardware.

We can always improve from this point onwards, if someone comes along with the right hardware and points out the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants