search

Severson-Group / AMDC-Firmware

Embedded system code (C and Verilog) which runs the AMDC Hardware
http://docs.amdc.dev/firmware
BSD 3-Clause "New" or "Revised" License
30 stars 5 forks source link

Scheduler indicate problem task #266

Closed elsevers closed 2 years ago

elsevers commented 2 years ago

The scheduler produces an error when a task exceeds the allotted time quantum and displays an error message on the terminal. This PR modifies that error message to include the problem task's name.

npetersen2 commented 2 years ago

Thanks @elsevers, a few comments:

  1. Based on the 3 commits on this PR before your commit, I think you probably made this edit based on v1.0.x (i.e. the default branch) and then changed to merge to develop, is this correct? So, this PR is doing your fix + getting develop up-to-date with v1.0.x. If so, I think you should redo your change but base it on develop so that the 3 extra commits are not in your PR
  2. You cannot really call the running_task the "problem task" ...it just happened to be running when the time slice was over run. Consider two tasks A and B: A takes 98% of the time slice and B takes 5%. The code will print B is the issue, but in reality, it is more complicated since A was probably the real issue. For this reason, I think your change will end up confusing users, but perhaps it is ok. The real way to debug overruns is to profile all tasks and look at run-time timing stats, but this is more complex. I think the actual real solution is to go to a preemptive scheduler in a real RTOS... :)
elsevers commented 2 years ago

Thanks @elsevers, a few comments:

1. Based on the 3 commits on this PR before your commit, I think you probably made this edit based on `v1.0.x` (i.e. the default branch) and then changed to merge to `develop`, is this correct? So, this PR is doing your fix + getting `develop` up-to-date with `v1.0.x`. If so, I think you should redo your change but base it on `develop` so that the 3 extra commits are not in your PR

2. You cannot really call the `running_task` the "problem task" ...it just happened to be running when the time slice was over run. Consider two tasks `A` and `B`: `A` takes 98% of the time slice and `B` takes 5%. The code will print `B` is the issue, but in reality, it is more complicated since `A` was probably the real issue. For this reason, I think your change will end up confusing users, but perhaps it is ok. The real way to debug overruns is to profile all tasks and look at run-time timing stats, but this is more complex. I think the actual real solution is to go to a preemptive scheduler in a real RTOS... :)
  1. Oops, I had mistakenly assumed that since there were no file diffs between v1.0.x and develop that these were pointing to the same commit, but they are not. I have resolved this issue by re-basing this branch onto develop
  2. Thanks for explaining. Let's try not to confuse the users. I changed the prompt to be ERROR: CURRENT TASK IS [...]. Thoughts?