Intention of yield() in endTransmission and requestFrom methods?

S

sethkaz

Member
I'm doing a project where I'm using a Teensy 4.0 with an INA238, and using platformio and the Arduino framework.

I ran into an issue that stemmed from the use of yield() at the end of the endTransmission and requestFrom methods. Specifically, I'm using this library on a platformio project that also uses the freertos-teensy library, which redefines the yield function to include a vTaskDelay(1) call.

This unfortunately stops the I2C transactions for 1 or 2 ms, and gives opportunity to have bus collisions.

I band-aided this by commenting out the two yield() calls, but that feels like it's defeating an intended operation. What is the intent of the two yield() calls and is there another way to accomplish this? Or at worst, what are the consequences of commenting out those two lines?
 
see p#3 - following not applicable to RTOS usage.
yield() calls process background Serial and EventResponder items. It has been optimized and really shouldn't be anything like 1 or 2 ms as it can be called millions of times per second when there are none of those things active.

Putting this in the INO will replace it so that doesn't happen at all:
void yield(void)
{
}
 
Last edited:
As FreeRTOS has already overridden yield(), that’s not going to work, just cause a linker error…

I’m not a big fan of all the yield calls buried in the various libraries. I believe the reasoning is that they ensure some (rather specific) set of polled items to execute during or after a time-consuming function, when loop() hasn’t been able to exit. However, we’re stuck with them now…

I would deeply question the call to vTaskDelay(1) at that point. There’s no delay inherent in a yield call, and no guarantee of the duration of the FreeRTOS tick, either. That’s what I would be inclined to comment out, and go back to the author querying the reason for it.
 
h4yn0nnym0u5e said:
I’m not a big fan of all the yield calls buried in the various libraries. I believe the reasoning is that they ensure some (rather specific) set of polled items to execute during or after a time-consuming function, when loop() hasn’t been able to exit. However, we’re stuck with them now…
Click to expand...
Perhaps it’s influenced by cooperative multitasking thinking? From that perspective, what do you think is wrong with yield() in long-running functions?
 
I think you’re probably right, but it’s clearly causing issues when used with pre-emptive multitasking.

The specific issue I had was with yield calls deep inside the SdFat library interacting with my EventResponder response functions … which called SD functions. It got very messy. The ones in the Wire library will cause issues if the response (or newly-scheduled task) uses the Wire library. And so on.

Of course, in a proper multi-tasking environment every piece of hardware should only ever be accessed by a single task, so there is no issue there. It’s just quite hard work for the programmer when they have to juggle a UI task that’s reading settings from SD, an audio task starting and stopping streaming playback from SD, and a buffering task that’s keeping the streaming buffers updated.

Vanilla Teensyduino is of course not a multi-tasking environment, and because the existence and consequences of buried yield calls are undocumented, it can be a bit of a nightmare figuring out why your sketch is misbehaving. I speak from personal experience.
 
jmarsh said:
Can you just set freertos::g_yield_task to false so it doesn't use vTaskDelay() ?
Click to expand...
I suspect not.

I'm guessing slightly here, but it appears that because FreeRTOS doesn't have a loop(), the yield and EventResponder::runFromInterrupt behaviour are emulated by creating two tasks. If you smash the record of the task handle, the task will still continue to run, and could be in the middle of a call to freertos::yield() when the sketch code pre-empts it and (indirectly) re-enters freertos::yield(). This is probably a Bad Idea.

sethkaz said:
This unfortunately stops the I2C transactions for 1 or 2 ms, and gives opportunity to have bus collisions.
Click to expand...
Looking back, I'm a tiny bit concerned by this statement. In a pre-emptive multi-tasking system, you surely have to be able to cope with your I²C task being inactive for a fairly unknown length of time. Also, what's going to cause bus collisions? Only the INA238 has been mentioned, and according to TI it "operates only as a secondary device".
 
Arduino's yield() was intended to be a hook for a cooperative scheduler. Teensy uses it for EventResponder.

The FreeRTOS override of yield() (shown below) only calls vTaskDelay(1) if the scheduler has not yet started. Once the scheduler has started, it calls freertos::yield() in file teensy_4.cpp, which does nothing unless you have enabled the EventResponder stuff. Are you using EventResponder? If so, that is what your code is doing while waiting for the I2C return condition.

Whether you are using EventResponder or not, @Angelo asks a good question. No other task should be doing an I2C operation as long as this one is running.

Code: 
FLASHMEM void yield() {
    if (::xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED && freertos::g_yield_task) {
        if (::xPortIsInsideInterrupt() == pdTRUE) {
            BaseType_t higher_woken { pdFALSE };
            ::xTaskNotifyFromISR(freertos::g_yield_task, 0, eNoAction, &higher_woken);
            portYIELD_FROM_ISR(higher_woken);
            portDATA_SYNC_BARRIER(); // mitigate arm errata #838869
        } else {
            ::xTaskNotify(freertos::g_yield_task, 0, eNoAction);
            ::vTaskDelay(1);
        }
    } else {
        freertos::yield();
    }
}
 
Thanks for the responses folks.

I can confirm that the
Code: 
void yield(void){}
will yield a linker error as expected.
h4yn0nnym0u5e said:
what's going to cause bus collisions?
Click to expand...
The opportunity for bus collisions is that we have several tasks reading from the INA238 and the code I inherited was *ahem* light on the use of semaphores/mutexes. I solved the collision issue by wrapping every I2C call with a semaphore, but it's still a nuisance to have much longer periods where the I2C is occupied because of the 1-2ms delays.

If I'm gathering your responses correctly, there is a fundamental issue that I can't get around, right?

My solution to deploy this for my client is to provide instructions on how to comment out the two yield calls. Otherwise, I'm going to be forking/submodule-ing a bunch of dependencies, which honestly feels like more work than trying to get all the dependencies to play well together without using the default locations. (I might be able to only fork Wire, but I feel like my luck isn't that good)
 
Note that I posted (#10) at the same time you did (#11), and explained that the vTaskDelay(1) is not the issue.
sethkaz said:
I'm only noticing this behavior after the scheduler is started. Otherwise, it runs as expected (without any excessive delays).
Click to expand...
In that case it's definitely not the vTaskDelay(1) that is the issue.
sethkaz said:
Honestly, I'm not sure. I'm using the default config that comes with FreeRTOS. I'll have to take a look.
Click to expand...
I'm pretty sure EventResponder is disabled by default. If your code does not enable it, then any I2C collisions are more likely a task design issue.
 
joepasquariello said:
any I2C collisions are more likely a task design issue.
Click to expand...
Correct. The I2C collisions have been addressed by using semaphores.

The issue I'm having is that I'm trying to get a good amount of data out of the sensor (well within the bus speed limitations), and the delay is totally stifling it. I'm not sure what's causing the delay, but would any other code also result in a 1ms delay? As in, it's pretty consistently 1ms, which is why I'm leading to believe it's the vTaskDelay(1).
 
h4yn0nnym0u5e said:
Err ... it definitely is!

As written, the code will execute the vTaskDelay(1) if the scheduler is started, and the yield task is running, and yield wasn't called from within an interrupt.
Click to expand...
You're right. The "!= NOT_STARTED" got me.
h4yn0nnym0u5e said:
Try changing that line to vTaskDelay(0), which should simply request a re-schedule without causing a delay.
Click to expand...
Or change it to taskYield(), which does the same thing as taskDelay(0) and is more efficient, according to notes on the FreeRTOS site.
 
h4yn0nnym0u5e said:
Try changing that line to vTaskDelay(0), which should simply request a re-schedule without causing a delay.
Click to expand...
Thanks, this looks like it solves the problem. And I only need to fork one repo to fix it!!
joepasquariello said:
Or change it to taskYield(), which does the same thing as taskDelay(0) and is more efficient, according to notes on the FreeRTOS site.
Click to expand...
Did you mean taskYIELD()? The compiler couldn't find taskYield(), and it suggested taskYIELD(). However, that is just defined to portYIELD(), which gives me a bit of pause as it's different enough.

Thanks for the help folks!
 
joepasquariello said:
Yes, taskYIELD(). Here is a link to a question on the FreeRTOS forum that addresses use of taskYIELD() versus vTaskDelay(0)

https://www.freertos.org/FreeRTOS_S...eertos_vTaskDelay_0_vs_taskYIELD_4401035.html
Click to expand...
Thanks for the explanation.

I tried out taskYIELD(), but it's yielding (pardon the pun) compiler errors. As the vTaskDelay(0) solved the delay problem, I'm going to leave it there.

In case you're curious, the compiler error is:
Code: 
In file included from .pio\libdeps\teensy40\freertos-teensy\src/portable.h:53,
                 from .pio\libdeps\teensy40\freertos-teensy\src/FreeRTOS.h:108,
                 from .pio\libdeps\teensy40\freertos-teensy\src/arduino_freertos.h:36,
Compiling .pio\build\teensy40\FrameworkArduino\AudioStream.cpp.o
                 from .pio\libdeps\teensy40\freertos-teensy\src\portable\teensy.h:28,
                 from .pio\libdeps\teensy40\freertos-teensy\src\portable\teensy_common.cpp:36:
.pio\libdeps\teensy40\freertos-teensy\src\portable\teensy_common.cpp: In function 'void yield()':
Compiling .pio\build\teensy40\FrameworkArduino\CrashReport.cpp.o
.pio\libdeps\teensy40\freertos-teensy\src/portable/portmacro.h:93:5: error: expected id-expression before '{' token
   93 |     {                                                   \
      |     ^
.pio\libdeps\teensy40\freertos-teensy\src/task.h:216:46: note: in expansion of macro 'portYIELD'
  216 | #define taskYIELD()                          portYIELD()
      |                                              ^~~~~Compiling .pio\build\teensy40\FrameworkArduino\DMAChannel.cpp.o
~~~~
.pio\libdeps\teensy40\freertos-teensy\src\portable\teensy_common.cpp:307:15: note: in expansion of macro 'taskYIELD'
Compiling .pio\build\teensy40\FrameworkArduino\EventResponder.cpp.o
  307 |             ::taskYIELD();
      |               ^~~~~~~~~
*** [.pio\build\teensy40\lib295\freertos-teensy\portable\teensy_common.cpp.o] Error 1
 
My guess would be that the idle thread actually does something that's considered important (probably the original Teensy yield code...) and using vTaskDelay(1) is a hack to force the kernel to switch to it. I say "hack" because yield() shouldn't guarantee a task switch, only if a higher priority thread is ready.
 
The reason for the issue with taskYIELD() is that it's actually a macro, not a function, hence you shouldn't put the scope resolution operator :: in front of it. Leaving it off fixes the compile error.

I looked at the discussion linked in post #18, and noted that it's 14 years in the past! I then looked at the relevant code, and it is indeed a bit more involved, but not massively so if the requested delay is 0 ticks. Inspecting the git blame, no part of the vTaskDelay() code is older than 3 years.
jmarsh said:
My guess would be that the idle thread actually does something that's considered important (probably the original Teensy yield code...) and using vTaskDelay(1) is a hack to force the kernel to switch to it. I say "hack" because yield() shouldn't guarantee a task switch, only if a higher priority thread is ready.
Click to expand...
It's actually a specific yield thread - FreeRTOS does have an idle thread which the user can modify (e.g. to put the processor into a low-power mode), which fortunately hasn't been hijacked for this port.

The yield thread calls a copy of the Teensyduino yield code, as you guessed. A vTaskDelay or taskYIELD certainly hints to the kernel that a task switch could be helpful, though of course it will only occur if a task of the same priority is waiting for a time slice. As the yield task is at the lowest possible priority, it almost certainly won't get scheduled unless all other tasks get suspended. This part of the code is possibly incorrectly designed, as the yield task waits for a notification for 10ms periods, then runs the yield code. As we definitely want the yield code to run at least occasionally, it really ought to be set to a high priority.
 
You must log in or register to reply here.
Back
Top