Use of yield() in Teensy Cores and Libraries

jmarsh said:
Yes, this is similar to the problem of more than one entity wanting to override the startup hook functions. Which I came up with a solution for: create a const pointer to the function and ensure the backing storage for that pointer is located in a certain program section. Then it's trivial to walk through each pointer in the section and call each function.
Click to expand...

I’d love to see some example code for this. Are you saying that there’s one “master override” for that startup hook, and then that main function loops through some pointers and calls each one, depending on where it is? How do you determine which “program section” these are in, do you just compare with hard-coded addresses or something?
 
h4yn0nnym0u5e said:
To be perfectly honest, I'm of the opinion that yield() calls should under no circumstances be buried in hardware library code. That is absolutely contrary to "where a cooperative task switch would be appropriate". The only way round that would be to compel library writers who wish to do that, to also provide an isBusy() function so that it's possible to avoid switching to a task that would clash. But then my EventResponder code would have to re-trigger itself when it found it couldn't run...
Click to expand...

The purpose of inserting a yield() in a waiting loop within a library is for that function to be made non-blocking when used with a cooperative OS. The only solution to avoid libraries clashing is to understand what each one does and use them appropriately.

If you look at the Arduino core, you will find the code below in hooks.c. I think it's pretty clear that EventResponder goes outside of the intended use, and renaming yield() to default_yield() would only make things worse.

Code: 
/**
 * Empty yield() hook.

 * This function is intended to be used by library writers to build
 * libraries or sketches that supports cooperative threads.
 *
 * Its defined as a weak symbol and it can be redefined to implement a
 * real cooperative scheduler.
 */
static void __empty() {
    // Empty
}
void yield(void) __attribute__ ((weak, alias("__empty")));

@shawn, should I post my question about yield() and QNEthernet to this thread, or somewhere else?
 
shawn said:
I’d love to see some example code for this. Are you saying that there’s one “master override” for that startup hook, and then that main function loops through some pointers and calls each one, depending on where it is? How do you determine which “program section” these are in, do you just compare with hard-coded addresses or something?
Click to expand...
It uses linker script magic to get pointers to the sections with a NULL pointer marking the end of each.
I left the rejected PR open just in case the code would be useful to someone: https://github.com/PaulStoffregen/cores/pull/734/files
 
shawn said:
That’s a good question. If it’s QNEthernet-specific, maybe an issue in the repo? We could hash improvements out there.
Click to expand...
@joepasquariello One idea is I could add a configuration parameter that makes the user call Ethernet.loop() at the end of the main program loop() and that causes EventResponder to not be used.
 
shawn said:
@joepasquariello One idea is I could add a configuration parameter that makes the user call Ethernet.loop() at the end of the main program loop() and that causes EventResponder to not be used.
Click to expand...
I think that would work. Here is the post I was working on, just FYI, so you know what I encountered.

----------------------

Shawn, I've recently added use of QNEthernet to an application where I use a cooperative OS, and I want to ask about the use of yield().

My understanding of QNEthernet is that on Teensy, it automatically attaches to EventResponder, and on non-Teensy, it overrides yield(). In both cases, the result is that call to yield() results in a call to Ethernet.loop(). In my application, the cooperative OS overrides yield() with a task switch that can't call Ethernet.loop() directly, so I must explicitly call Ethernet.loop() from within my sketch. That’s fine, but it makes me wonder whether QNEthernet should override yield() in the first place.

I started by adapting QNEthernet example FixedWidthServer. Since yield() is overridden by the CoopOS, calls to yield() within QNEthernet, such as in WaitForDHCP(), do not result in calls to Ethernet.loop() and if I call that function, my application hangs. My work-around was to start the OS, add a task that calls Ethernet.loop(), and then call WaitForDHCP(). Each time that WaitForDHCP calls yield(), a task switch occurs from the task that is runnning WaitForDHCP() to a trivial task that calls Ethernet.loop(). That works, but it seems fragile.

Rather than calling Ethernet.loop() indirectly via yield(), could each call to yield() within QNEthernet() be replaced by a call to Ethernet.loop() followed by a call to yield()?

There are only 4 or 5 places within QNEthernet where yield() is called, and they are all places where QNEthernet needs for Ethernet.loop() to be called, such as waiting for a DHCP response. QNEthernet could still attach to EventResponder, which would result in Ethernet.loop() being called more often, but would allow applications that use yield() as a cooperative task switch to work correctly. I think it would require at least one explicit call to Ethernet.loop(), but I actually think this is
 
This is exactly the reason for what I proposed above: if your yield() was able to call Teensyduino's yield() (either directly or via a dedicated idle thread), EventResponder would handle calling Ethernet.loop() as intended.
 
h4yn0nnym0u5e said:
To be perfectly honest, I'm of the opinion that yield() calls should under no circumstances be buried in hardware library code. That is absolutely contrary to "where a cooperative task switch would be appropriate". The only way round that would be to compel library writers who wish to do that, to also provide an isBusy() function so that it's possible to avoid switching to a task that would clash. But then my EventResponder code would have to re-trigger itself when it found it couldn't run...
Click to expand...
Probably my take is that a hardware library should always be written non-blocking, optionally with a narrow shim of blocking methods on top for those happy to busy-wait for hardware. You could even add a parameter to the blocking calls something like "bool yieldWhileWaiting", so that the user gets to choose between non-blocking, naive blocking or blocking with yield.
 
@joepasquariello Sure, I can call Ethernet.loop() before (or after or whatever) the yield() calls. (I think it would be interesting if there was a way I could tell if yield() was overridden.) I don’t think this will have a huge impact because use of waiting by the user in the library should be “relatively” rare, so this shouldn’t really have much of a performance impact. (That wasn’t the best or sentences…) (I’m always wary of the word “should” (and its opposite or other forms) in engineering. :))

I might also add a configuration macro that can disable this behaviour. Thinking about this more…
 
shawn said:
@joepasquariello Sure, I can call Ethernet.loop() before (or after or whatever) the yield() calls. (I think it would be interesting if there was a way I could tell if yield() was overridden.) I don’t think this will have a huge impact because use of waiting by the user in the library should be “relatively” rare, so this shouldn’t really have much of a performance impact. (That wasn’t the best or sentences…) (I’m always wary of the word “should” (and its opposite or other forms) in engineering. :))

I might also add a configuration macro that can disable this behaviour. Thinking about this more…
Click to expand...
@shawn, if you look at the original Arduino code from hooks.c from my post above, you can see that yield() was originally an alias for empty(), but that is not true in the Teensy cores. If it was, you'd be able to test "if (yield == empty)" to see if yield had been overridden or not.
 
joepasquariello said:
@shawn, if you look at the original Arduino code from hooks.c from my post above, you can see that yield() was originally an alias for empty(), but that is not true in the Teensy cores. If it was, you'd be able to test "if (yield == empty)" to see if yield had been overridden or not.
Click to expand...
That’s very system-specific. I was referring being able to tell if a function is overridden, but I guess that would only be known by the linker.
 
Maybe a useful extension to EventResponder would be attachOnDemand(), and EventResponder::runOnDemand(). A function attached in "on demand" mode would be triggered as normal (e.g. within an interrupt), but only executed when the user's sketch explicitly called runOnDemand().
 
h4yn0nnym0u5e said:
Maybe a useful extension to EventResponder would be attachOnDemand(), and EventResponder::runOnDemand(). A function attached in "on demand" mode would be triggered as normal (e.g. within an interrupt), but only executed when the user's sketch explicitly called runOnDemand().
Click to expand...

I wonder if EventResponder::attachImmediate is similar to this?
 
shawn said:
@joepasquariello The latest two commits in the QNEthernet library repo should address your needs. (96db12b1d and 9c572384f: https://github.com/ssilverman/QNEthernet/commits/9c572384f2998dc80883d55c02f8b23480868cfc/)

But I have a question: Why not use the non-blocking versions of these functions (or equivalent code)? (i.e. EthernetClient::connect() and stop(), EthernetClass::waitForLocalIP() and waitForLink(), and DNSClient::getHostByName())
Click to expand...
Thanks, Shawn. I will take a look at your changes tomorrow. I'm sure I'll eventually understand more about QNEthernet, but right now my understanding is very superficial. My goal was to add TCP connectivity to our application, and I did that by starting from one of your examples. I do remember looking at the events, but I didn't understand how they worked, so I have not yet incorporated that into any of my projects.
 
joepasquariello said:
Thanks, Shawn. I will take a look at your changes tomorrow. I'm sure I'll eventually understand more about QNEthernet, but right now my understanding is very superficial. My goal was to add TCP connectivity to our application, and I did that by starting from one of your examples. I do remember looking at the events, but I didn't understand how they worked, so I have not yet incorporated that into any of my projects.
Click to expand...
My view is that if you’re advanced enough to do your own cooperative task switching, you don’t actually need to use those functions in QNEthernet that use yield(). I don’t mind the change because, in theory, someone could have overridden yield(). But more advanced users should use all non-blocking functions — those that don’t internally use yield() or do any sort of waiting.

You might have noticed those // NOTE: Depends on Ethernet loop being called from yield() comments above the yield() calls. :)
Changed to: // NOTE: Call Ethernet loop in case an overridden yield() doesn't

While I don't encourage use of those blocking calls, you correctly pointed out that the examples still use them, so they should still work for (ideally) all use cases.
 
Last edited:
shawn said:
My view is that if you’re advanced enough to do your own cooperative task switching, you don’t actually need to use those functions in QNEthernet that use yield(). I don’t mind the change because, in theory, someone could have overridden yield(). But more advanced users should use all non-blocking functions — those that don’t internally use yield() or do any sort of waiting.

You might have noticed those // NOTE: Depends on Ethernet loop being called from yield() comments above the yield() calls. :)
Changed to: // NOTE: Call Ethernet loop in case an overridden yield() doesn't

While I don't encourage use of those blocking calls, you correctly pointed out that the examples still use them, so they should still work.
Click to expand...
I think what happened is a tried an example, and since it did what I wanted, I never looked beyond it. When I download your modified code, I'll try the non-blocking functions. Thanks very much!
 
Since EventResponder is a c++ class, you can always just derive from it and override triggerEvent() with the behaviour you want.
 
jmarsh said:
Since EventResponder is a c++ class, you can always just derive from it and override triggerEvent() with the behaviour you want.
Click to expand...
Hmmm ... that's a good thought. It's not really written such that derived classes can make much use of its methods, but it's certainly a concept worth toying with. At least it would provide a workaround for my use case - if people want SD access other than playback/record, then they'd have to resort to the OnDemand() events, but that's still pretty simple for them. Oddly enough, people do want both at once; when you make a synth, looper or mini DAW, you want to be able to load and save settings while running. Most unreasonable...

The thing with triggerEvent() is it doesn't "know" how and when execution is going to occur: that's down to using the appropriate attach() method.
 
joepasquariello said:
I think what happened is a tried an example, and since it did what I wanted, I never looked beyond it. When I download your modified code, I'll try the non-blocking functions. Thanks very much!
Click to expand...
Can you clarify: which example did what you wanted? Did it call one of the waiting functions? If so and it worked, what exactly didn’t work for you before?

My impression was that something actually didn’t work for you, as opposed to theoretically. But I’m now reading this as a purely theoretical issue. (Please correct me if I’m wrong.)

What were the specific changes to QNEthernet that you made?

Note: I’m going to update the behaviour so that Ethernet.loop() is called before yield() only if a configuration macro is set. This will restore the original behaviour by default. I’m writing up documentation now. My preference is not to call Ethernet.loop() twice in a row for the majority of cases.
 
Last edited:
shawn said:
Can you clarify: which example did what you wanted? Did it call one of the waiting functions? If so and it worked, what exactly didn’t work for you before?

My impression was that something actually didn’t work for you, as opposed to theoretically. But I’m now reading this as a purely theoretical issue. (Please correct me if I’m wrong.)

What were the specific changes to QNEthernet that you made?

Note: I’m going to update the behaviour so that Ethernet.loop() is called before yield() only if a configuration macro is set. This will restore the original behaviour by default. I’m writing up documentation now. My preference is not to call Ethernet.loop() twice in a row for the majority of cases.
Click to expand...
I started from the FixedWidthServer example. It wasn't a theoretical issue. When I build an application that uses the cooperative OS, the task-switching version of yield() gets linked. The function that waits for DHCP calls yield(), but in my program yield() does not call Ethernet.loop(), so the function would never return.
 
joepasquariello said:
I started from the FixedWidthServer example. It wasn't a theoretical issue. When I build an application that uses the cooperative OS, the task-switching version of yield() gets linked. The function that waits for DHCP calls yield(), but in my program yield() does not call Ethernet.loop(), so the function would never return.
Click to expand...
I see. I was confused when you said, "since it did what I wanted." Did you mean when you didn't use the cooperative OS features?
 
shawn said:
I see. I was confused when you said, "since it did what I wanted." Did you mean when you didn't use the cooperative OS features?
Click to expand...
Yes, that's right. I first modified the example to do what I wanted my simple TCP server to do, and then tried to merge that functionality into the multi-tasking application, and that's when I ran into the issue related to yield().
 
This is all great, and kudos as ever to @shawn for being a responsive library maintainer who works with "his" users to provide an incredibly high level of support for all abilities.

For users at the level of @joepasquariello, who are prepared to dive deeply into the Teensyduino ecosystem, override yield(), and deal with the consequences, things are probably fine. Clearly if you're creating/using a co-operative OS you know to keep to the "one task, one piece of hardware" rule, so you're OK. I guess you could get into issues if you use QNEthernet with SPI-based Ethernet hardware plus, say, a SPI display, because then you'd have to know SPI uses yield() (which the naïve user doesn't because it's not documented). And then your task has to run both the display and Ethernet, which is messy.

If you're a library writer wanting to take advantage of facilities built into cores, then limitations need to be documented much better than they currently are. EventResponder is nearly 6 years old, yet still documented as "an experimental API". The documentation for attach() states "Calls from yield() allow use of Arduino libraries, String, Serial, etc." - as I've discovered, this is simply not true in the general case: you can't safely use serial, Wire, SPI, SD, ADC, USB ...

As an experimental API EventResponder should of course be open to carefully considered and ideally non-breaking changes. I've tried that, once, and basically it's a brick wall. In a way I hope that QNEthernet never gets rolled into Teensyduino, because the released version will almost certainly be out of date compared to current users' needs.

I'm still not sure what would be a minimal, useful, non-breaking set of changes to vanilla Teensyduino (putting slightly aside super-users like @joepasquariello, while still trying to make sure their existing codebase is unlikely to break - I would never advocate that!). Some of the following, maybe?:
  • embrace @jmarsh's idea of a separately named default_yield(), with yield() weak and aliased to it, but ovverideable
  • default_yield() has an optional parameter, showing which library code called it and is thus currently unsafe to re-enter: if the parameter is set, calls...
  • ...default_yield_from(where) has an required parameter, showing which library code called it and is thus currently unsafe to re-enter
  • default_yield_from(where) is effectively the existing yield() code, plus maintenance of the flag(s) of which libraries have called it; existing yield() prevents re-entering its body, with the exception of the EventResponder response code
  • all library calls to yield() modified to use the default_yield_from(where) directly
  • add a function to check whether you're executing from a yield() context, and which libraries' flag(s) are set
  • EventResponder modified so that instantiations can flag libraries they use, and trigger processing will not execute pending events if the default_yield_from(where) shows there will be a clash
I don't expect any of this to be done quickly, even if it's agreed to be a Good Thing, so I'm going to adopt @jmarsh's suggestion of a derived-ish class, and re-invent the wheel. In the spirit of not breaking my existing API, having to explicitly call the AudioEventResponder::runEvents() function will not be the default, though it'd probably be the preferred route to robust code.

And I will document what I do.
 
You must log in or register to reply here.
Back
Top