Skip to content

refactor(radio): move per10ms() to a software timer #5966

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

Closed
wants to merge 1 commit into from

Conversation

raphaelcoeffic
Copy link
Member

This helps reducing the pressure on the timer queue.

@pfeerick pfeerick added this to the 3.0 milestone Mar 5, 2025
@pfeerick pfeerick added the house keeping 🧹 Cleanup of code and house keeping label Mar 5, 2025
Base automatically changed from 3djc/async-workaround to main March 5, 2025 01:08
@raphaelcoeffic raphaelcoeffic marked this pull request as ready for review March 10, 2025 12:19
@philmoz
Copy link
Collaborator

philmoz commented Mar 27, 2025

How accurate / consistent will this be?

Could this adversely affect the 'g_rtcTime' variable that is updated every second in the per10ms() function.

If the interval between calls is not exactly 10ms then g_rtcTime value will not accurately reflect the time.

@raphaelcoeffic
Copy link
Member Author

How accurate / consistent will this be?

No different from what we're doing right. The current implementation schedules the per10ms() functions to be called from within the timer task when the HW timer hits.

This PR just replaces the async scheduling with a fixed timer call, which reduces the pressure on the timer task's request queue. As far as I could see, this is really no different from the previous implementation, except that it the timer task's queue is not flooded in case the timer task is not scheduled as often as required.

@philmoz
Copy link
Collaborator

philmoz commented Apr 13, 2025

Thanks.
I asked because I worked on a commercial project where the original dev used a timer callback to increment a count to measure elapsed time. It was highly inaccurate because the timer callback interval was not guaranteed. The kicker was this was on code running on a smart watch. Needless to say I threw all this out and let the watch tell me the time.

@3djc
Copy link
Collaborator

3djc commented Apr 13, 2025

It is just used to fire things "from time to time", not at all for timing related activities, so inaccurate timing is no issue there

This helps reducing the pressure on the timer queue.
@raphaelcoeffic
Copy link
Member Author

@philmoz @gagarinlg @pfeerick @3djc we'd need to decide whether or not to include this in 2.11. If that is not the case (and it's really not necessary as of now), I'd squash it with the rest of #5840 and keep only that PR open.

@3djc
Copy link
Collaborator

3djc commented Apr 25, 2025

3.0 is the way to go imho

@gagarinlg
Copy link
Member

There are a couple of users having issues with 2.11 and switches not working or UI not working. Could this solve those issues?

@3djc
Copy link
Collaborator

3djc commented Apr 25, 2025

Per10ms don't handle those, so very unlikely

@raphaelcoeffic
Copy link
Member Author

Closed in favour of #5840

@raphaelcoeffic raphaelcoeffic deleted the timer-per10ms branch May 2, 2025 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
house keeping 🧹 Cleanup of code and house keeping
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants