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

Thoughts on adding "signals" #4

Open
MikeFair opened this issue Nov 17, 2016 · 11 comments
Open

Thoughts on adding "signals" #4

MikeFair opened this issue Nov 17, 2016 · 11 comments
Assignees
Milestone

Comments

@MikeFair
Copy link
Contributor

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

#define SIG_WORK1DONE 0x01
#define SIG_WORK2DONE 0x02
#define SIG_GREENLIGHT  0x08
#define SIG_REDLIGHT       0x04

class Worker1 : public Task {
  bool loop() {
    delay(150); // Do Work1 Stuff
    setSignal(SIG_WORK1DONE);

    delay(150); // Then do Work2 Stuff
    setSignal(SIG_WORK2DONE);
    return true;
  }
}

class Monitor1 : public Task {
  bool loop() {
    uint32_t result = WaitForAny(SIG_WORK1DONE | SIG_WORK2DONE, 5000);

    if (result == 0) {
        Serial.println("No new results obtained in the last 5 seconds; Initiating "5 second rule" response!");
        // Execute 5 second response;
        clearSignal(SIG_GREENLIGHT);
        setSignal(SIG_REDLIGHT);

    } else {
        Serial.print("New results discovered! ");
        if (result & SIG_WORK1DONE) { Serial.print(" Work1 "); }
        if (result & SIG_WORK2DONE) { Serial.print(" Work2 "); }
        Serial.println(" was completed!");
        clearSignal(SIG_REDLIGHT);
        setSignal(SIG_GREENLIGHT);
    }
    return true;
  }
}
class Reporter1 : public Task {
  bool loop() {
    if (WaitForAll(SIG_WORK1DONE | SIG_WORK2DONE, 20000) != 0) {
        // Non-Zero signal means all signals matched
        clearSignal(SIG_WORK1DONE | SIG_WORK2DONE);
        Serial.println("Both Works have been completed!");

    } else {
        // Zero signal means timeout waiting for match
        Serial.println("The works are still in process!  ");
        Serial.print("Work1 " + (result & SIG_WORK1DONE) ? String("has been completed.  ") : String("is still in process.  "); }
        Serial.print("Work2 " + (result & SIG_WORK2DONE) ? String("has been completed.  ") : String("is still in process.  "); }

    }
    return true;
  }
}

}

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

@nrwiersma
Copy link
Owner

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 Dispatcher that signals would be pushed onto (like in your example a signal is just a 32 bit int). Consumers can then look at the signal stack and "handle" any matching signals, there by removing them from the stack. Perhaps there could also be a flag to leave the signal on the stack. As an additional feature signals could expire, removing them on the next match task. Maybe this is akin to the "clear" you were talking about.

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,
Nick

@nrwiersma nrwiersma added this to the 0.2 milestone Nov 17, 2016
@nrwiersma
Copy link
Owner

nrwiersma commented Nov 18, 2016

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.

@MikeFair
Copy link
Contributor Author

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. ;)
Or, they happen in the blink of an eye, only lasting one event cycle and if a Task wasn't already waiting for it, it's gone. :)

Kidding aside, it's clear to me you're seeing the issues.
For the record, I'm not adding anything that requires dynamic memory allocations or arrays/buffers.

I can see three potentially reasonable flags to do some "automation":

  1. "reset on first match" -- set a flag, and the first Task to match it wins; especially great if there's only 1 possible consumer
  2. "reset on next pass" -- Once all the Active Tasks have had the chance to see the signal at least once; clear it. In other words, only Task(s) that aren't delayed for some other reason will see it.
  3. "reset on next cycle" -- Once all Tasks have had the chance to see the signal at least once; clear it. In other words, only the Task(s) that are already waiting for it will see it (and all of them will see it).

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...


It occurs to me though that the Task could register interest, with timeout, to the Dispatcher on a signal.

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
delay for 5000ms; if the signals are matched while waiting, then mark the matched flags and clear delay_ms to 0

Which makes it a natural extension of what delay is already doing.

@MikeFair
Copy link
Contributor Author

Two more useful "match types" just to note them:

  1. Match on Clear -- Instead of matching when signal are 1(HIGH); this matches on signals that are 0(LOW)
  2. Match on Change -- Same idea, but without knowing/caring the current value; delay until it changes.

If you're interested in the code I can push it into my fork. I didn't make a separate branch for it.

@nrwiersma
Copy link
Owner

"float off into the abyss" is for garbage collected languages only, the others don't have the abyss feature :)

That list is then tested against the entire stack of current_signals

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.

@MikeFair
Copy link
Contributor Author

MikeFair commented Feb 1, 2019

I reread through this thread, and one thing I see that's different between our thoughts is what a "signal" is...
My take is that a "signal" is like a light switch; like a digital GPIO it is either HIGH or LOW.
It is not an "event", it doesn't dispatch anything, it's simply a numbered "flag".

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. :)

@nrwiersma
Copy link
Owner

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?

@MikeFair
Copy link
Contributor Author

MikeFair commented Feb 1, 2019 via email

@nrwiersma
Copy link
Owner

I think both points (yours and mine) stand. They are helpers that dont really interact with the scheduler. With the possible exception of WaitForAny. WaitForAny would either need to be on the task, or a macro, so that the correct yield and delay are used, the others are actually acting of a common uint32_t to flip the bits.

WaitForAny is a little unclear as to how you would test for truthiness.

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.

@MikeFair
Copy link
Contributor Author

MikeFair commented Feb 1, 2019

WaitForAny is a little unclear as to how you would test for truthiness.

WaitForAny and WaitForAll were kind of implementation specific to the fact I used a bitField.
The user set an uint32_t to indicate the set of 'signals' they were interested in.

The result they got back was a uint32_t with the set of signals that were actually matched.
So if the task was waiting on any of 0xFFFFFFFF and 0x01010101 were set, the task would receive 0x01010101 as the result to WaitForAny. WaitForAll was similar, but obviously blocked until everything matched so the idea of matching a subset of flags wasn't really an issue.

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).
I'll obviously post here or submit a PR if I think there's code worth you integrating.

Thanks!
Mike

@nrwiersma
Copy link
Owner

Thanks. Appreciate your input.

Nick

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

2 participants