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: multichannel continuous conversion #214

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yjh0502
Copy link
Contributor

@yjh0502 yjh0502 commented May 4, 2020

related: #212

Add support for multichannel continuous conversion.

Copy link
Member

@TheZoq2 TheZoq2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay

The changes look good over all, I left some small comments in the review thingy.

Also, would you mind adding a changelog entry to CHANGELOG.md?

let pins = AdcPins(adc_ch0, adc_ch2);
let adc_dma = adc1.with_scan_dma::<_, adc::Single>(pins, dma_ch1);

// TODO: should match with # of conversions specified by AdcPins
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lingoring TODO

Copy link
Contributor Author

@yjh0502 yjh0502 May 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a validation on ADC DMA buffer length

  • When a buffer is larger than ADC conversion length, DMA will fill the buffer partially.
  • When a buffer is smaller than ADC conversion length, it will panic. I'm not sure if it's a best way to handle the case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure what the best behaviour would be here. I'd almost argue that a panic is best in both cases (unless we can return a result).

self.set_channel_sample_time(2, adc::SampleTime::T_28);
}
fn set_sequence(&mut self) {
self.set_regular_sequence(&[0, 2, 0, 2]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the sequence 4 samples?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No specific reason, I started from rustdoc example :)

stm32f1xx-hal/src/adc.rs

Lines 531 to 544 in 1357e5a

/// Set channel sequence and sample times for custom pins
///
/// Example:
/// ```rust, ignore
/// pub struct AdcPins(PA0<Analog>, PA2<Analog>);
/// impl SetChannels<AdcPins> for Adc<ADC1> {
/// fn set_samples(&mut self) {
/// self.set_channel_sample_time(0, adc::SampleTime::T_28);
/// self.set_channel_sample_time(2, adc::SampleTime::T_28);
/// }
/// fn set_sequence(&mut self) {
/// self.set_regular_sequence(&[0, 2, 0, 2]);
/// }
/// }

@yjh0502
Copy link
Contributor Author

yjh0502 commented May 24, 2020

I pushed new PR, including

  • Rebase
  • Update CHANGELOG.md
  • Try to validate buffer length on ADC DMA conversion.

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 this pull request may close these issues.

2 participants