Repository navigation
Fix TaskRunner::addTask registering every task twice - #56
Merged
kevin-f-ortega merged 3 commits intoOct 14, 2026
Merged
Conversation
task to m_task_table[m_index], so every task was placed twice and ran twice per round. Destroying a task left a dangling duplicate in the table. Count tasks by finding the first empty slot rather than using m_index, since m_index is the run cursor. Insert each task once. Add a TaskRunner SDD and randomized unit tests. Fixes fprime-community#30
This was referenced Oct 2, 2026
Open
Member
|
@Iva-L would you be able to help us review this? |
Iva-L
suggested changes
Oct 13, 2026
Contributor
There was a problem hiding this comment.
Sure, great catch! Just a minor change for safety. I verified it with fprime-stm32-baremetal-reference :
- Current main, flashed and run:
taskRunner.m_task_tableshows all 18 tasks duplicated. - Same board, rebuilt with this PR's fix:
m_task_tableshows exactly 18 unique entries.
Iva-L
approved these changes
Oct 13, 2026
Iva-L
left a comment
Iva-L
left a comment
Contributor
There was a problem hiding this comment.
LGTM. I don't know if @kevin-f-ortega has anything else to review?
kevin-f-ortega
requested changes
Oct 13, 2026
kevin-f-ortega
left a comment
kevin-f-ortega
left a comment
Contributor
There was a problem hiding this comment.
LGTM, just a minor update to this PR
Contributor
|
And great catch @nathancheek ! |
kevin-f-ortega
approved these changes
Oct 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See nasa/fprime#5815
#42 added a shift-based insert but kept the old lines that first write the task to
m_task_table[m_index], so every task was placed twice and ran twice per round. Destroying a task left a dangling duplicate in the table.This PR solves that by removing the extra
m_task_tableinsertion. It also switches to a different method of counting the number of tasks.m_indexis used to point to the next task to run, so this variable can't be relied upon for calculating the number of tasks if tasks have already started running.I added UTs to show that these issues are resolved. Also added an SDD to include the unit test requirements.
Claude AI was used during investigation, bugfixing, unit test writing, and documentation.