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

ADC example scaling wrong #343

Closed
jordens opened this issue Mar 28, 2022 · 2 comments · Fixed by #344
Closed

ADC example scaling wrong #343

jordens opened this issue Mar 28, 2022 · 2 comments · Fixed by #344

Comments

@jordens
Copy link
Member

jordens commented Mar 28, 2022

The ADC examples scale the ADC codes with VREF/max_sample.

info!(
"ADC reading: {}, voltage for nucleo: {}",
data,
data as f32 * (3.3 / adc1.max_sample() as f32)
);

max_sample = ((1 << bits) - 1) << lshift by itself is correct according to its docstring.

stm32h7xx-hal/src/adc.rs

Lines 790 to 793 in a46841c

/// Returns the largest possible sample value for the current settings
pub fn max_sample(&self) -> u32 {
((1 << self.get_resolution().number_of_bits() as u32) - 1) << self.get_lshift().value() as u32
}

But the conversion is not. VREF corresponds to 1 << bits + lshift here as in all ADCs that I know of.
c.f. stm32-rs/stm32f4xx-hal#362 (comment) and other places.

The best approach is probably to change max_sample() and its docstring.

@richardeoin
Copy link
Member

I enjoyed the explaination on the f4 thread, thanks! Indeed this is clearly an error in the example, and a footgun in the API.

My preferred way of fixing this would be to add a new method, perhaps pub fn slope(&self) -> u32 that returns 1 << bits + lshift. After that, max_sample can be marked as depreciated.

@jordens
Copy link
Member Author

jordens commented Mar 28, 2022

Yes. Much better solution!

richardeoin added a commit to richardeoin/stm32h7xx-hal that referenced this issue Mar 29, 2022
bors bot added a commit that referenced this issue Apr 2, 2022
325: Add #[must_use] annotations r=richardeoin a=richardeoin

Use of this attribute was [expanded in the standard library](https://blog.rust-lang.org/2022/01/13/Rust-1.58.0.html#more-must_use-in-the-standard-library) in Rust 1.58, and there is now a [clippy lint](https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use[](https://github.com/richardeoin)).

Silence the lint for REC methods, since dropping the REC is intended.
Also silence the lint for GPIO modifiers (set_speed) since these are also often dropped.

341: Update sdio-host r=richardeoin a=richardeoin

* Update [sdio-host](https://crates.io/crates/sdio-host) to the latest version
* Add eMMC support from sdio-host
* Build examples for all families

Tested again on a STM32H747I-DISCO development board with a SanDisk Extreme 32 GB SDHC UHS-I card

344: Add slope method to ADC and depreciate max_sample r=richardeoin a=richardeoin

Closes #343

Co-authored-by: Richard Meadows <962920+richardeoin@users.noreply.github.com>
@bors bors bot closed this as completed in 71851dc Apr 2, 2022
@bors bors bot closed this as completed in #344 Apr 2, 2022
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 a pull request may close this issue.

2 participants