-
-
Notifications
You must be signed in to change notification settings - Fork 749
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
Util Timer enhancements #1444
Comments
Is it worth benchmarking the 32 bit vs 64 bit for point 1 to see what the real gains are? I'm sure this will be different for each platform too. |
Yes, definitely. My plan is to toggle a pin on IRQ entry and exit and look at it with an oscilloscope. The reason I suggest it was when I was doing things a while back I did exactly that and felt that the IRQ was taking too long to complete. |
Done some work on this - as you note the hit from 64->32 math isn't huge. There hasn't been a massive improvement. The one that hurt was recurring timers which used what ended up being a 64 bit divide, so it's nice to get rid of that. |
Currently we use a 64 bit time in the util times, which really slows things down as we're doing 64 bit maths inside the IRQ. We could move to 32 bits, perhaps switching to a 'time until scheduled' method rather than storing the raw system time.
We also use a queue which is shuffled around to reschedule, requiring a reasonably big copy each time. We could keep this constant and just have an array of indexes that we reordered. Another benefit of this would be that timers had unique IDs, so could be easily checked in the code.
Some things (like waveform and stepper) continually poll to see if the waveform is active or not. The waveform could actually just post to the event queue (using the new timer IDs) to avoid this polling.
When doing ADC reads, we wait in the IRQ for the read to complete. The HAL could be modified to allow an ADC read with callback function + user data, which would make waveform to take up a lot less time.
Currently timers are set using a global time. It means that stuff like
digitalPulse
has to usegetTime()+pulseLength
internally which makes the accuracy dependent on the RTC. We could just avoid using the RTC completely - timing would potentially be a little less accurate (because of the time taken to reschedule the event not being counted via the timer) but then most Util Timer tasks are reasonably short-lived anyway.The text was updated successfully, but these errors were encountered: