-
Notifications
You must be signed in to change notification settings - Fork 34
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
Thoughts on adding "signals" #4
Comments
Hey, Really awesome idea. Perhaps you could explain more about the clearing of signals. In my mind signals are like events, they occur and then are handled, or float off into the abyss, but are not really cleared. With the above understanding I would think that there would be a That is all just off the top of my head, will need to give this a little more thought, and a look at how these things usually work, cause I really don't know. I come more from an event based background. Later, |
Ok. I see the issue with clearing the signal now, signal expiry will not work. It occurs to me though that the Task could register interest, with timeout, to the Dispatcher on a signal. That way when the dispatcher receives the signal within the timeout, it can update the info on the Task. Will play with this idea some more. Later. |
It's cool how being a "user" of event systems can be so different an experience than "making the events." :) or float off into the abyss, but are not really cleared. That's called a memory leak. ;) Kidding aside, it's clear to me you're seeing the issues. I can see three potentially reasonable flags to do some "automation":
Other than that, I see the smartest/easiest thing to do is to keep it manual and make/let the application manage setting and resetting of the signals. For example, one useful pattern is for Tasks to use a different signal_id to indicate when they've seen a signal. So Task1 sets signal_id 1, then it issues a "waitForAll" on signal_ids 2 & 3. When/if 2&3 get set, Task1 knows that the signal has been seen by the appropriate Tasks and clears signal_ids 1,2, and 3...
Yep, exactly what the "WaitForAny(signal_mask, ms_timeout)" and "WaitForAll(signal_mask, ms_timeout)" functions are doing. Since all 31 available signals are expressed in a single uint32_t signal_mask (via bit positions), the Task registers the entire list of signals it is interested in with the one call. That list is then tested against the entire stack of current_signals in one bitwise & operation (because the entire stack is also one uint32_t) during the updateDelay() function. From a coding perspective; it's better expressed as Which makes it a natural extension of what delay is already doing. |
Two more useful "match types" just to note them:
If you're interested in the code I can push it into my fork. I didn't make a separate branch for it. |
"float off into the abyss" is for garbage collected languages only, the others don't have the abyss feature :)
I was actually thinking the other way round. The tasks that are currently interested in signals are in a list, when a signal is fired, it checks the interested info and removes delay and flags as signaled. No storing of the signal. |
I reread through this thread, and one thing I see that's different between our thoughts is what a "signal" is... I am still thinking of tasks that run "on schedule" and during their time on the CPU they check the state of the signals to see if they have work to do. It's not a "callback" model where they register functions... The 31 "signals" are like a bank of switches that can be set or cleared, and a task can delay until those switches are in the right position, or timeout. As I mentioned above, aside from testing for the "HIGH" state, two other functions could be testing for the "LOW" state and maybe testing for a "CHANGED" state (i.e. the flag was either high or low when the task first started waiting, and the state "changed")... Simply tracking a series of numbered 'flags' seems more manageable than some kind of dispatcher. Your call obviously. :) |
Tracking flags doesnt actually require changes at all. A simple for loop testing your condition, and yielding when it isnt true, like:
That would basically do what you just described. It could probably be macroed to make it easier. Is that what you were thinking? |
Yes, but that example doesn't really demonstrate the coordination of signals across multiple tasks or testing for multiple signals at once (either one, some, or all of many signals a task might want to block on).
Like in the example I posted there's a couple worker tasks, a "manager" task, and a UI reporting task.
Looking at the signals set by other tasks helps to communicate and coordinate the activities between them. Setting an clearing also helps to indicate acknowledgement of when other tasks have received the producer's output.
It is a "lightweight" version of message passing between tasks.
The changes were "helper functions" like:
uint32_t result = WaitForAny(SIG_WORK1DONE | SIG_WORK2DONE, 5000);
Is simply more useful than making me write the code to do the same.
…On Fri, Feb 1, 2019 at 10:19 AM Nicholas Wiersma ***@***.***> wrote:
Tracking flags doesnt actually require changes at all. A simple for loop
testing your condition, and yielding when it isnt true, like:
for (signal != HIGH) {
yield();
}
That would basically do what you just described. It could probably be
macroed to make it easier.
Is that what you were thinking?
|
I think both points (yours and mine) stand. They are helpers that dont really interact with the scheduler. With the possible exception of
All in all, if you would like to contribute the helpers, I would happily include them in the lib. I think this kind of task coordination is likely a common use case. |
WaitForAny and WaitForAll were kind of implementation specific to the fact I used a bitField. The result they got back was a uint32_t with the set of signals that were actually matched. The functions setSignals/clearSignals/changeSignals also took the same uint32_t. Haven't looked at the code in a long while; similar story to you I'm sure. I think this issue was more to point out the use case/pattern for intertask signals (which is similar in concept to runGroups in a way). Thanks! |
Thanks. Appreciate your input. Nick |
I've gone ahead in my local branch and added "Signals" to Delay.h.
There are 31 signals (using a 32bit bitField) available to use.
It works by a Task issuing a "result = WaitForAny(signals, msTimeout);" or "result = WaitForAll(signals, msTimeout);".
When the Task resumes, if result == 0 then the timeout expired.
Otherwise, result contains all the signals that matched the request.
In the case of WaitForAll, a non-zero result means all the requested signals matched; with WaitForAny, result contains all of the requested signals at the time any one of the active signals matched.
Signals are activated with "setSignals(uint32_t signals);" and cleared with "clearSignals(uint32_t signals)".
Here's some sample code with three Tasks; 1 Worker, 1 Monitor, and 1 Reporter
It's still a bit of an evolving feature; and I'm looking for some ideas/thoughts on how/when to reset/clear the activated signals. It's clearly related to interrupts and callbacks so their might be some tie in to those systems.
For now, I'm seeing that I/we can leave clearing the signals manually cleared, clear on first match, clear once all Tasks currently requesting the signal have seen it.
I will eventually track down the Windows/Linux/Arduino APIs to see what/how they do it. I think the way it's going to go is you need each type of reset for different things and so it's best to have options to control how/when/if a signal is automatically cleared.
The way it works internally is a "signal" is represented by a bit in a 32bit Field, like an interrupt flag or a GPIO digital pin state.
Each "signal" is a binary power of 2 from 1 to 2^31 (most easily created with bitshifting 1 << signal times) and I expect to primarily be created using #define. As a consequence "signals" can be mixed or reversed using "OR" and "AND".
One of the 32-bits in the field is reserved by the system to indicate whether the test is for "Any" or "All".
Internally it's using the same delay clock that "delay" uses and so works off the same mechanism. I added a section before the timespan test to determine if the appropriate signals have been matched and clear out the delay if they have.
All that said, I expect to wait until after you've had a chance to comment on the existing changes to move things into the Scheduler before posting too much on this one.
Cheers!
Mike
The text was updated successfully, but these errors were encountered: