speeduino
speeduino copied to clipboard
noisymime
Want faster and better ADC (analog digital converter) reads with ADC_sequencer(). (Massively improved main Loops/s.)
What I came up with is a "ADC_sequencer" as I call it. Basically a scheduler for analog reads. The original core for stm32 actually starts and stops and disables the analog digital converter at every read, this means powering it down, and then requires a startup stabilization again at the start of every read, so there is a delay in the analogRead(). What I have done is: 1.Make it non blocking: at every call read previous result, and start new conversion. 2.Do not disable the ADC when changing channels: this means we do not have to do the ADC startup delay every time. 3. Increase the channel sampling time: This means that when changing channels we get a much better sample of the newly selected channel. And just because we can do that because now it is still much faster than previously powering UP/DOWN the adc block all the time. 4. Same applies to the ADC of ATMEGA, it is also already coded but not yet tested and debugged, so interesting to see the Loops/S increase on Atmega. 5. Currently the code that I am testing is located here:https://github.com/TBAMax/speeduino/tree/ADC_clean But it is not based on the latest release, but some features missing(new comms, TPS resolution update for example) so there is still a quite lot of work to merge it to make a pull request. Feel free to use the code, to build on it. Or help out by putting the bounty on the issue here.
The main core of the idea implementation is in the https://github.com/TBAMax/speeduino/blob/ADC_clean/speeduino/sensors.ino ADC_sequencer() function though many other functions are attuned to it.
Currently when testing on the STM32F407VE all analog readings seem stable and loops speed is minimum 17kLoops/s (when the ignition/fuel is running) and about 28kLoops/s when engine stopped, occassionally jumping to the 50k.
Seems like you did a very complicated implementation, any reason to don't use DMA instead?
Hello! Thanks @VitorBoss for starting conversation. Ok, let me explain.
Why not to use DMA: it is very platform specific, quite complicated to set up, and other modules may want to use it also(CAN, SDcard,etc.). On Atmega we would not gain anything(because it do not have DMA), also the need for new extra buffer for ADCvalues (already too many global variables).
Another consideration was if to use interrupts. Why don't use interrupts: Possible slight impact on ignition accuracy (specially on Atmega). Added complexity of setting up interrupt priorities on platforms that support it. Also needs extra buffer, and the methods of reading from it with timestamped results(MAPdot and TPSdot need timestamps on readings), thus adds extra layer of complexity. And generally I do not like to see many nointerrupts(); interrupts(); in the code.
Also mainly the strive for more portable code: nonblocking functions for starting AD conversion, checking if it is completed, and reading the result from AD converter registers should be fairly straightforward to implement on all different chips without much hassle.
The extra buffer needed can be defined on the boards file, STM32 have a lot of DMA channels, you can define a macro to do ADC readings in a way to be compatible across the boards. I had started interrupt based ADC readings a few years ago, the code is still present in the sources but it was unreliable because it convert all ADC channels instead of the used ones. The ADC buffer don't need to be the same across all boards, STM32 can have up to 32 ADC channels
DMA on stm32 can also be programmed to convert multiple channels. But then that went tricky because how to select the channels based on the pin names(set in the init.ino). Also that also will do the whole sequence of channels at once. We do not need all the channels converted so often. Well of course can just discard the useless conversions, and probably the ADC is still fast enough for our purpose. Yes the current approach turned out to be trickier than anticipated because some functions relying on instant waiting for conversion. Especially MAP read function that included read for EMAP channel also, so I had to break that up into the separate functions. Well the motivation for doing all the difficult work for me is actually the need to improve the code readability also, to be able to understand and simplify what every part of the code is actually doing, and that comes from seeing sometimes read values fluctuate, and need to understand what is going on. Also when I can see that there is quite good chance to increase the overall program speed drastically, I am on it. But I refuse to just add some even more complicated pieces, but will rather rework some areas. I understand that it is sometimes to the point that rises the question if the original authors name is still good to keep in the header, but what can I do about that. I also understand that there is needed some DIY style element to all this, and so actually improving the code readability and other factors for example from guidelines from MISRA-C carries in itself the threat of ending the project quickly, because of the huge market that is behind this. When it gets too good and reliable there is a big chance that it attracts big cash and is bought out and closed down, of cource will go on under some other name(s) but the original community would be shut down and this would be bad, because a bunch of separate guys doing thing only on their own is not a bit same good.
Turns out for atmega2560 the speed gain loops/s is not even noticeable at the moment. Only effect is the ability to run the ADC at normal speed (adc clock 125kHz) instead of overclocked(adc clock 1MHz), and therefore more stable and more accurate readings.
I close this now to clean the issues folder. Conclusions about the topic: Since the starting of this issue the STM32 ADC libraries have evolved so much that there is no problem using the standard analogRead(). For the Atmega chips the conclusion from this is that for those the ADC converter is not the bottleneck regarding the loop speed, and the impact for speed on those from improving the analog read functions is small, and it is more profitable to focus on optimizing other parts of the code.
I close this now to clean the issues folder. Conclusions about the topic: Since the starting of this issue the STM32 ADC libraries have evolved so much that there is no problem using the standard analogRead(). For the Atmega chips the conclusion from this is that for those the ADC converter is not the bottleneck regarding the loop speed, and the impact for speed on those from improving the analog read functions is small, and it is more profitable to focus on optimizing other parts of the code.
really ? I came back on speeduino code after one year, it has been running fine in the meantime. I went a big fan of your patch as i'm using an stm32, i even tried it on a real bike.
now i was lurking about this issue and i saw a pr of sept 2023 on stm32duino about this same optimisation but it didnt end well, the pr didnt work as expected.
I'm now just syncing my customisations with master and havent tried the new version on the ecu yet. what are the loops/s now ?
@gdiciocco Hi. Interested about the issue on stm32duino, can it be affecting us with the Speeduino as well?
At some point something happened to the master branch that made it run similarily fast. But I did not investigate what happened.
As there have been work on the schedulers recently this ADC patch have somehow fallen out of scope mostly (althouhg it do not have to be this way).
The ADC sheduling in my opinion too was a good succsess in genereal. The coding style however could be improved. I was not completely happy with the readability. Some more object orientation needed there probably. Use of the classes instead of switch statements (Uncle Bob´s "Clean code" lessons on Youtube).
@TBAMax
https://github.com/stm32duino/Arduino_Core_STM32/pull/2133
Here. Having the issue solved at library level would be excellent!
Gave a try at current tree with current stm32duino lib on black f407.
Got 25k loops/s, a similar number was obtainable the last year with the adc sequencer patch only.
At that time the main tree was up to 6k loops/s.
So as you said stm32duino ppl must have done something very good to analogread in the meantime.