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

Update, clean up, and finish EDU program queue thread #338

Open
Tracked by #194
PatrickKa opened this issue Jan 5, 2025 · 1 comment
Open
Tracked by #194

Update, clean up, and finish EDU program queue thread #338

PatrickKa opened this issue Jan 5, 2025 · 1 comment

Comments

@PatrickKa
Copy link
Contributor

PatrickKa commented Jan 5, 2025

Description

It's been a while since the code for that thread was written, so we need to check if it still matches the functionality described in miro and the PDD. One thing I noticed is that we should probably jump back to the beginning of the loop after waking up from one of the two SuspendUntil(endOfTime) calls at the top. There are also tons of DEBUG_PRINT()s and blank lines that need to be cleaned up. Computing the start delay and calling SuspendFor() instead of directly using the start time with SuspendUntil() seems unnecessarily complicated as well. Finally, we need to think about what to do when the EDU program queue is overwritten while the program queue thread is still processing it (the RF and command parsing threads will have a higher priority).

@PatrickKa
Copy link
Contributor Author

PatrickKa commented Jan 5, 2025

Regarding the last point, I see three options:

  1. We use resetEduProgramQueueThread to signal whether the thread should jump to the beginning of the processing loop. This boolean variable must be a topic/CommBuffer or somehow else thread safe. It gets set to false at the start of the processing loop. Further down, we add if(resetEduProgramQueueThread) { continue; } at any point where we want to have the option to jump to the top. I think that should be before and after the first two SuspendFor() calls (the ones before ExecuteProgram()) and before the queue index is incremented.
  2. Before we suspend a second time, we check if startDelay2 < eduCommunicationDelay because that would mean that we loaded a new and different queue entry compared to the one loaded at the beginning of the processing queue. If that's the case, we jump back to the top of the loop. This option is much simpler than option 1, but it allows resetting the processing loop only at one point. This leads to the following issue. Overwriting the EDU program queue also sets the queue index to 0. If this happens after the point where we can reset the loop (e.g. while we wait for a program to finish executing) we will increment the queue index at the end of the loop and thereby skip the first entry of the new queue.
  3. Same as option 2 but when overwriting the EDU program queue we set the queue index to -1 (or 255 if it's a uint8_t). At the beginning of the loop, we also add if(queueIndex == -1) { queueIndex = 0; }. This should fix the issue of option 2. It even allows us to check for the modification of the EDU program queue at arbitrary times, similar to option 1.

After writing this down, I see that I clearly favor option 3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant