I am looking for guidelines that would make it harder for me to write bugs similar to the one I recently discovered in my own code.
Here is the mistake I made:
serial receiving character on ISR I, storing chars in a buffer. When the received char is EOL, notify a receiving task R, with eIncrement.
serial receiving task R waits, then when notified clears the notifiedValue, handles the n number of frames in the buffer.
handling these received frames may imply running long functions. These functions have waits and notifies. I had one of these functions F do that:
register itself for future notifications from task T, passing xTaskGetCurrentTaskHandle()
start task T
when task T is done, it notifies F back (setting bits in the notifiedValue, since there are actually multiple instances of task T)
The issue is that function F runs in the context of task R, so when it registers itself for future notifications from T, it exposes the notifiedValue of R. This means that F waking up may overwrite a value changed by ISR I, and ISR I might increment the notifiedValue while F is waiting… I need to keep the notifiedValues separated.
I see the problem, but I don’t really know how to setup rules for myself to prevent this in the future. I have some ideas:
don’t run long sub-functions in the context of a task.
send notifications instead, or start new tasks. This means that these other tasks need a mechanism to notify back when done, which is quickly going to grow old and verbose.
have a naming convention for functions that may be using waits/notifies, to make it clearer that there is danger.
keep somehow track of the next available notification index for the current task, so that the subfunction can register itself with a notification index and wait for only that index, and the task T does not overwrite the notified value of task R.
This last one would seem the mos robust and flexible to me, and frankly it wouldn’t surprise me if there was infrastructure for that already. I looked at the definition of tskTCB, but found only ulNotifiedValue[] and ucNotifyState[]. How about a mechanism to “reserve” a given notification index, and to “fetch” the next available one?
From your description, it seems to me that r and t are coroutines, ie there does not appear to be a need to make them concurrent in the first place. Why does the t processor run in a separate task instead of r’s context?
There are multiple instances of task T, which run in parallell (doing stuff with motors and sensors, that take up to 30 seconds).
But even without that, if T needs to wait for notifications (e.g. from an interrupt), I need to separate the notifiedValues for T, and those for R (which also gets notifiedValues from interrupt). Otherwise they might overwrite each other’s value.
I need to look into queues. The thing is that I don’t have control over the order in which these notifications might come, can I wait for a queued notification, and see it arrive although it is not first in queue, without consuming the one that is first in queue? It would still be needed by the other task…
My first thought is that you are over-using notification values. There is a very hard limit in data-bandwidth with Notification Values, as it is just one word long, and even with the indexes, you can only wait at a given time on one index.
Rather than, as is seems you are doing, of dividing your program into “tasks” and using task to task communication primatives to define your API, I like to think of my system as divided into “API Clusters” (in C++, these tend to become classes), which other API cluster communicate through with the API functions, and the tasks become implementation details of the API Center.
This means things like requesting notifications of events from a center is a call to an API function that doesn’t need to affect the task of that center, but the call and just directly manipulate the list of notification requests, that will be scaned and called when the event occurs. (This code will need some syncronization with the center, to avoid manipulating it in two different threads of code at once). These call-backs tend to be defined to need to be short, so they may use notifications to the task, but this notification is private to that center, and changing it doesn’t affect the API that everyone else is using.