This is undocumented and not intuitive. I would expect, like most functions, for a delay of 0 to return immediately or to be an effective context switch. I feel like one of those things would be optional. Instead, an assert is triggered. Why?
A key difference is that the other operations are all ‘Check for some condition, and if true do something, else wait for up to x ticks for that to be true’. For that sort of action, having a zero time option can make sense.
A function call to delay for no time doesn’t really make sense most of the time. If you really want that ability, you can make a trivial wrapping function that checks the time, and if zero does a yield, else it does the delay.
Richard Barry’s response on GitHub was that it’s a semantics issue:
I think this is a semantic thing. xTaskDelayUntil() is normally used to create periodic execution, and you cannot create a task that executes every zero ticks. Whereas passing zero into vTaskDelay() results in equivalent behaviour to calling taskYIELD().
That’s the point- the semantics are counterintuitive too. While it might be used a particular way by some, it goes against more widespread API’s. The typical paradigm is not to let the library manage the repeated call but to do it yourself.
For instance, in C++, if you want to schedule a wake up, the API is
template< class Clock, class Duration > void sleep_until( const std::chrono::time_point<Clock,Duration>& sleep_time );
(https://en.cppreference.com/w/cpp/thread/sleep_until) It’s specified that this call may not throw, as long as you’re using STL types for arguments.
Boost has the same semantics as C++.
In PHP, it’s the same semantics as C++ with a warning if the time is in the past.
I can’t find other mainstream languages or libraries that use sleep_until or similar functions. For obvious reasons, there is no way in a multi-threaded/-tasked system to guarantee that your code will awaken at exactly the time you specify. Your thread may be preempted or not awoken promptly as the scheduler decides something else has priority. The sleep system shouldn’t force the scheduler to give you more priority than you have. Besides, if too many threads have the same wake time, the scheduler can’t promise they all will wake up on time- that’s impossible.
It’s more correct to sleep for at least some amount of time and then ask the system how long you actually slept for. Then, you can use that information to make a nearly correct sleep call. Most API’s have a way of getting this information efficiently. Whether it’s accessing the current tick or a lower-level intrinsic, that method is usually supported.
It makes sense to not even support the current paradigm. The philosophy of C and C++ is to be as efficient as possible. That means the library shouldn’t do any more than strictly necessary. If I need to keep track of a periodic task, I can still do it myself. But if I don’t want to, the library shouldn’t add overhead.
The current code has added overhead that a developer may not want: several assert checks (there are other asserts I take issue with but I’m ignoring for now), an unnecessary update to the address I’m passing in, and more arguments passed which impacts the code generation and optimization. All of which could be replaced by a more idiomatic API that has less overhead and less complexity.
vTaskDelay is NOT intended for something like the usage of a sleep_until. Almost every time I do a vTaskDelay, the meaning I want is that I just did something, and I want to wait a time ‘x’ before I do my next thing.
A ‘Sleep Until’ operation might better be mapped into an operation using vTaskDelayUntil, or a function that does the computation, and then that function can check about the time being 0.
In fact, it REALLY wants to be careful how it describes time, as it could be you get the result of wanting to delay -1 ticks, which because it is unsigned is basically forever (with 32 bits).
Sleep Until REALLY wants to use a time system that doesn’t wrap (at least within the operation of the program, and tick counts can (maybe with 32 bit ticks, you can ignore the wrap around, but then you need to make the difference signed)
That’s why the typical paradigm is to return immediately rather than allowing a context switch if the time delay requested is zero. Do you have an example where that doesn’t make sense?
Of course I can wrap the function to conform to C++ STL’s semantics, but that’s not the point. Why not fix the function if it’s suboptimal?
Further to my original reply regarding the normal use of vTaskDelayUntil() being to create periodic processing, and asking for a period of zero not making sense in that use case: Sometimes users will unintentionally, and therefore unknowingly, ask for a period of 0 because they have a slow tick rate and because integer maths rounding down. Or maybe they changed their tick frequency so what was not calculated as zero before is at the new tick rate. In those cases the assert() is helpful in immediately alerting them.
But that’s treating your user like we’re programming in Python. This is a C interface- you make bad mistakes, undefined things happen, not errors. That’s how C is so fast. Don’t we want RTOS to be fast, not idiot-proof?
I’m confused why you’re bringing up vTaskDelay. The code I’m asking about is vTaskDelayUntil?
I agree that most sleep_until doesn’t allow signed times. In fact, I’ve seen most systems deprecate their signed and unsigned sleep_until calls for opaque or C++ types that cannot have this issue.
That is one opinion. Many others argue each parameter should be checked programmatically on each function call, an alternative opinion. Our philosophy is to try and do both - if there is an error that is always going to exist then check it with an assert so it is caught at development and test time, after which the assert can be removed.
I guess I assumed you were going to use vTaskDelay for what you described.
If you look at the design parameters of vTaskDelayUntil, it is presumed that the previous woken value is set ONCE at the beginning of operation, and the delay parameter is how often you want to run. A value of zero makes no sense here, you will by definition be behind.
The C language may have a general idea that the language won’t check things that cost even a bit to check, but it DOES provide the ASSERT macro to allow you to add checks for debug, that disappear. FreeRTOS works the same way, you can use configASSERT to have tests in debug, that you could remove in production/release, but the costs are often small enough that you can leave them in (just as you can leave your ASSERTS in)
Many of the problems that configASSERT catch are things that can be hard to figure out in a failing system, but can be quick to check, so the asserts are there.
But RTOS asserts don’t get compiled out in release, at least as far as I can tell? That overhead of doing a comparison is still there, which is how I found it.
At any rate, that’s not my primary issue: going against established paradigms in undocumented and counterintuitive ways. English semantics of “wait zero seconds” is equivalent to “don’t wait at all.” (What doesn’t make sense is “wait negative seconds,” but that’s not my issue.)
The semantics of configASSERT() as as per the standard C assert - if you leave it undefined it will be compiled out.
I don’t want to get sidetracked by this configASSERT point. I’ll assume that my MCU tool chain made some bad choices about setting defines and/or not compiling a Release build like I expected.
What about my main issues: undocumented, not idiomatic, and counterintuitive?
I’m just going to have to disagree on the basis that different people want different things so it doesn’t matter if the assert is there or not - you don’t want it others do. If you search the forums long enough you will find cases where the assert has helped people.
So what about undocumented? If you’re going to go against more mainstream libraries and be non-idiomatic about it, the least you could do is warn us. That would’ve saved me literally hours of debugging.
If I created a PR with a new API function that behaved idiomatically, would it be considered?
Right - we can add this to the documentation for sure, but the idea of having the assert is to prevent you from having hours of debugging, not to cause that. The implementation of assert should do something that alerts you to the condition that failed the assert - we have a couple of basic examples: https://www.freertos.org/a00110.html#configASSERT
As for a PR - yes we consider all PRs but consideration does not guarantee acceptance. If there is a new function then it would have to provide something significant and different to the existing function (when assert is not defined), and updates to existing functions can’t break backward compatibility.
Right, the assert is a separate issue.
and updates to existing functions can’t break backward compatibility.
I assume it’d be more accurate to say that breaking changes would be subject to stricter scrutiny and would likely have to be targeted to milestones, e.g. major version changes?
Yes. Although breaking changes normally (always) have a backward compatibility option.
If I created a PR with a new API function that behaved idiomatically, would it be considered?
What do you plan to call it? May I suggest “vTaskDelay( const TickType_t xTicksToDelay )” instead because that seems to be exactly what you are describing?