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

STM32F3: ADC external trigger EXTSEL values #518

Open
darrylring opened this issue Aug 14, 2015 · 5 comments
Open

STM32F3: ADC external trigger EXTSEL values #518

darrylring opened this issue Aug 14, 2015 · 5 comments

Comments

@darrylring
Copy link

The ADC external trigger EXTSEL values should indicate the source of the trigger.

From include/stm32/f3/adc.h:

#define ADC_CFGR_EXTSEL_EVENT_0     (0x0 << 6)
#define ADC_CFGR_EXTSEL_EVENT_1     (0x1 << 6)
...

From include/stm32/f4/adc.h:

#define ADC_CR2_EXTSEL_TIM1_CC1 (0x0 << 24)
#define ADC_CR2_EXTSEL_TIM1_CC2 (0x1 << 24)
...

I believe for the F3 the external trigger source can be different depending on whether it is ADC1/3 or ADC2/4 being configured, so maybe they should be like ADC_CFGR_EXTSEL_ADC13_TIM1_CC1?

I'm happy to create a pull request with these changes. I imagine these would need to be alongside the existing macros so as not to break existing code.

karlp added a commit to karlp/libopencm3 that referenced this issue Aug 15, 2015
Note: this is "compatible" with prior f3 code, but is completely different
behaviour to f4 for instance, so would be surprising and bad to make the change
as is.  it's just an idea.  This would however completley drop the idea of
tryign to make defines that cover all the complicated f3 cases.

Relates to github issue libopencm3#518
@karlp
Copy link
Member

karlp commented Aug 15, 2015

Given how complicated tables 89-92 is, I'm not sure these would be useful to anyone. I'm somewhat included to look at even removing these definitions of the EVENT_X defines, and make the API just take the index, (0,1,2, without the shifting) in the call.

so.. something liek....

https://github.com/libopencm3/libopencm3/compare/master...karlp:draft/idea-f3-adc-extsel?expand=1

@darrylring
Copy link
Author

I can certainly say that I would find it useful! Being able to use ADC_CR2_EXTSEL_TIM1_CC2 instead of ADC_CFGR_EXTSEL_EVENT_0 and looking at the data sheet would be helpful and make the code more clear.

I guess I could use a macro in my own code. It just seems to me that it's worth exploring given that the other STM32 series have clearer macros for this purpose.

@karlp
Copy link
Member

karlp commented Aug 16, 2015

By all means shows us what you're proposing, but bear in mind how complicated the f3 adcs can be. I'm not sure sure how you could come up with some defines that would legible in even 120 character terminals :) It's a fair point that f3 has "boring" defines though, no debate, so by all means show us a better way. Making the API easier to use with numbers from the data sheet as I was proposing I thought might make it easier, given that you would always have to be consulting the reference manual to work out the triggering modes anyway.

@darrylring
Copy link
Author

I have a working ADC and a working timer but I can't get the ADC to trigger off the timer after two solid days of work. You're probably right.

After (if) I get this to work, I will revisit this, because I do think it can be made to work. As far as I can see it's not that much different than the F4.

@karlp
Copy link
Member

karlp commented Aug 17, 2015

Try coming by IRC in #libopencm3 on irc.freenode.org and show us what your code is like so far, we can probably help you work out what's not working yet.

karlp added a commit to karlp/libopencm3 that referenced this issue Oct 14, 2015
Note: this is "compatible" with prior f3 code, but is completely different
behaviour to f4 for instance, so would be surprising and bad to make the change
as is.  it's just an idea.  This would however completley drop the idea of
tryign to make defines that cover all the complicated f3 cases.

Relates to github issue libopencm3#518
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

No branches or pull requests

2 participants