ADC analog watchdog

[Thargon]

Hi all,

I'm having some troubles to solve the following problem with the ADC watchdog.

What I want to do:
Basically, I want to be informed when the measured voltage rises above / drops below a certain threshold (in order to enable/disable a battery charger) but without having to measure it explicitly in a loop.
For this I want to use the analog watchdog of the ADC on the STM32F405.
The only information I could find so far are the lines 131 (and equivalent) in in the source code (http://chibios.sourceforge.net/docs/hal_stm32f4xx_rm/adc__lld_8c_source.html) and an old conversation in the forum (http://forum.chibios.org/phpbb/viewtopic.php?f=2&t=41&p=3969&hilit=analog+watchdog#p3969).

The problem:
Seems to me as if this feature is not yet implemented for the STM32F4, or did I miss something?
If so, I would like to add it but will propably need some help


Thanks in advance!

Thargon

[Giovanni]

Hi,

It is not present on the F4 but it is present on the F3, probably you could add it along the lines of the F3 implementation.

It is on the list of things to do anyway.

Giovanni

[Thargon]

Hi,

thanks for your reply!

As I understand, the according method in the implementation for the F3 is CH_IRQ_HANDLER() (os/hal/platforms/STM32F30x/adc_lld.c:253), which calls adc_lld_serve_interrupt() in line 215, which again calls the _adc_isr_error_code() makro (os/hal/include/adc.h:264) dependig on the content of the ISR register.
The corresponding section in the implementation of the F4 seems to be the CH_IRQ_HANDLER() method (os/hal/platforms/STM32F4xx/adc_lld_c:111), which currently only handles DMA interrupts. Since there is a TODO comment for the analog watchdog, this seems the correct place to start.

But when I took a closer look on the implementation of the F3 I was a bit confused, how a watchdog interrupt is handled there. Maybe I just try to explain what happens there in my own words and you can correct me please in case I got something wrong.
Basically CH_IRQ_HANDLER() calls adc_lld_serve_interrupt(), which calls _adc_isr_error_code() as mentioned before. Within the _adc_isr_error_code() makro the adc conversion is first stopped. If an error callback function was set in the related ADCConversionGroup, the driver state is changed to ADC_ERROR and the callback is called. Depending on the driver state after that custom callback the state is reset to ADC_READY. In any case the related ADCConversionGroup and thread (if any) are then decoupled from the driver.

Now, if I got that right, a watchdog interrupt is always handled as a serious error. Thus, the adc conversion is stopped and can not be restarted from within the custom callback function, since the ADCConversionGroup and the thread are decoupled from the driver after that callback is called.
As a result, in my case I could enable/disable the charger from within the callback, but the main application has to periodically check the driver if it was stopped and restart it. In that case I could as well periodically read the current adc value and handle the charging from the main application.

Basically, I don't understand, why the watchdog is always handled as an error and there is no way to override that in the custom callback. Wouldn't it make sense to relax that behavior so that a watchdog inerrupt is only handled as error, if no callback function was set?

Thargon


PS: When I think about it, I might make a very basic mistake. When the analog watchdog is enabled, will the interrupt be called with everey measurement or only once when the threshold value is passed? So far I though the latter would be the case, but right now the former makes more sense to me. In any case I think it should be possible just to restart the adc with a different configuration as required.

[Giovanni]

Depending on the sample rate you could get a very high interrupts rate if the driver is not stopped.

A possible approach could be to detect the watchdog once then disable it without sending the driver in error state.

Giovanni

[Thargon]

I thought of a similar solution:

1 - stop adc convertion
2 - set the driver state to ADC_ERROR
3 - if an error callback was set: call it
4 - if the driver state is still ADC_ERROR:
4.1 - set the related ADCConvertionGroup to NULL
4.2 - stop and 'detach' the related thread

This way, a watchdog interrupt is handled as an error by default or if the callback does not set the driver state to something else than ADC_ERROR.
But this also enables the custom callback to reconfigure and restart the ADC without any additional logic needed.

I hope with this solution I can first wait until the voltage rises above some threshold, then the interrupt will execute my callback. Within this function I can reconfigure the ADC so that it will generate an interrupt as soon as the voltage drops below some threshold. As soon as it does, I can again toggle the configuration.

Tomorrow I will try and see how far I get

Thank you for the help!

Thargon


PS: When I'm done and the code works fine, how should/may I contribute it to the project?

[Tabulous]

Looking at the trunk this never got include into the F4xx driver ?

[Giovanni]

Not yet, it would require reworking all ADC drivers for consistency, not just the F4 one.

Such a change will necessarily go after next release.

Giovanni

[Thargon]

Hi,

I just realized that I never replied after I had a look ad the ADC driver "tomorrow"
Attached to this post is a patch for the ADCv2 driver that can be applied to the most recent version of ChibiOS. Maybe some line numbers do not actually fit the latest revision since I wrote it ages ago ("tomorrow" xD ), but my GIT can resolve it still.

I know, it is just ADCv2 and I would gladly implement it for v1 and v3 as well, but I don't have according hardware to test it, so it doesn't make any sense imo. It is a small patch however, so people should be able to port it.

@Giovanni: Maybe you should open a forum where you can ask the community for support, so people know what you need

[Giovanni]

I think the "small change requests" forum can be used also for small contributions but I could open a specific one if you think it is a good idea.

About contributions, usually integration is slow or not possible at all for a series of reasons, for example and not limited to:
- Lack of bandwidth for review, I don't work on ChibiOS full time.
- Proposed change only covers a specific port but breaks or does not supports others.
- The proposed change just changes a bit into a driver then updating all demos, makefiles and mcuconfs is left to the integrator (very common lol)
- The change does not fit or conflicts with future plans.
- The change is too architecture-specific, for example it is done thinking to the STM32 only. As an ST employee it should not bother me but ChibiOS tries to be honestly multi-platform.
- It does not fit in the architecture or should be implemented in another layer.
- The "small change" is in reality a set of changes in a single patch that should be reviewed and merged independently.
- The change is not small but has impacts on other subsystems, imagine a change in RT that then impacts OSAL and creates problems in HAL or an important API change.
- The change is very complex and review is hard and maintenance even harder, I must be able to support what is included in ChibiOS, this is why we introduced the community repository, for things I could not handle. Code remains but contributors tend to disappear...
- Lots of others.

Anyway, the best approach is to be very "atomic", small, easily reviewed changes are most likely to be included. Another case is to contribute a whole subsystem, for example a file system, there the problem would be maintenance of the new SW and also potential commercial handling or partnership, a different approach should be used.

Giovanni

[Thargon]

I thought the "Small change requests" forum rather as a place to ask YOU for support. But I guess that is the place where my patch actually belongs to. Shall I open a new thread there or do you prefer to move this one?

As to the list of difficulties for integration: I absolutely get your point
- It's a shame that ST forces you to do other things
- Integration of a patch to all ports and demos is something I can hardly achieve. I have neither the time nor all the hardware. However, I can try to integrate it to F4 and F1 ports and demos as a start.
- I can not know every detail of your future plans to ChibiOS, so I will continue to propose what I think is best
- I always modify the modules or layers I think are (or should be) responsible for a given task, but there is always room for discussion and I might be wrong

I think this is just the way it goes with nice but small projects like this. I hope ChibiOS will become more popular in the future, especially since you put so much effort in portability and re-usability (i.e. OSAL). Btw, did I ever thank you? Well, now I did