search

modm-io / modm

modm: a C++23 library generator for AVR and ARM Cortex-M devices
https://modm.io
Mozilla Public License 2.0
748 stars 132 forks source link

fix scan mode for STM32F3 #977

Closed victorandrehc closed 1 year ago

victorandrehc commented 1 year ago

This PR removes enableScanMode and disableScanMode methods and incorporate them into addChannel.

victorandrehc commented 1 year ago

@salkinium I created this PR improving the scan mode that was merged on monday, but it seems like the CI died on me. It made 13/25 tests and never executed the last 12. How can I make it execute the last 12?

salkinium commented 1 year ago

The 25 other CI jobs are guarded behind the ci:hal tag, since they run quite long and "steal" CI time from other PRs. It's not the best mechanism (you cannot set the tag, only maintainers). You can however execute the HAL tests locally, see the readme in the tests folder.

salkinium commented 1 year ago

I don't really know anything about the ADCs, so I'm delegating this to @rleh ;-P

victorandrehc commented 1 year ago

This Pr basically sets the scan flag on register CR1 and remove the enable and disable scan mode methods, because I figured that if one is adding a channel they are probably meaning scan mode, so would be better to just enable scan mode there silently.

rleh commented 1 year ago

I'll look into it, give me some (2-3) days...

victorandrehc commented 1 year ago

Hi @chris-durand, I will change(eliminate) the reference of the F3 then, I thought it was referring to F303 controllers. About the scan mode, my objective is exactly to support the DMA. I have a local branch doing that already and it would be my next PR. My strategy was to merge it on smaller easier to review pull requests. If you prefer we can close this PR and I can do it all together on a big PR.
Since you use scan mode just for dma, maybe it makes sense to keep the enable and disable scan mode methods.

chris-durand commented 1 year ago

Since you use scan mode just for dma, maybe it makes sense to keep the enable and disable scan mode methods.

I would just leave it as is and recommend to close the PR without merging. The scan mode does no harm for now. We can finally sort it when you implement ADC DMA in your next PR.

victorandrehc commented 1 year ago

The DMA PR is not right yet. But I will create a draft so the progress can be followed.