New API xTaskPeriodicDelay (#1349)

[ntd]

New function to be used for periodic tasks to ensure a constant execution frequency. It is intended to supersede xTaskDelayUntil to overcome its shortcomings, that is:

• avoid run away of pxPreviousWakeTime ([BUG] xTaskDelayUntil updates pxPreviousWakeTime incorrectly in case of deadline miss #1339)
• catch up any skipped period immediately (and update pxPreviousWakeTime accordingly), notify the caller of the number of periods skipped (by returning them) and wait until the next period (it could be less than xTimeIncrement if we are close to the next period)
• notify the caller when not enough ticks have been elapsed (by returning 0) and handle the situation gracefully (by properly waiting until the next wake time)

Description

Test Steps

I tested that function on my ESP-IDF setup, directly running the resulting binary on an ESP32-S3 evaluation board. I would be more than happy to provide a unit test but I simply don't know how to do it.

This is the code I used:

static void loop(void *data)
{
        TickType_t rv;
        TickType_t last = xTaskGetTickCount();
        for (int i = 0; i < 10; ++i) {
                rv = xTaskPeriodicDelay(&last, 5);
                printf("Iteration %d: last=%lu, tick=%lu, rv=%lu\n",
                       i, last, xTaskGetTickCount(), rv);
        }
        vTaskDelete(NULL);
}

void app_main(void)
{
        TaskHandle_t handle;
        xTaskCreate(loop, "LOOP", 2048, NULL, 1, &handle);
        vTaskSuspend(handle);
        vTaskResume(handle);
        vTaskDelay(3);
        vTaskSuspend(handle);
        vTaskResume(handle);
        vTaskDelay(6);
        vTaskSuspend(handle);
        vTaskDelay(12);
        vTaskResume(handle);
}

#if 0

/* This is an approximation of what ESP-IDF is doing behind the scene */
static void main_task(void *data)
{
        printf("main_task: Calling app_main()\n");
        app_main();
        printf("main_task: Returned from app_main()\n");
        vTaskDelete(NULL);
}

void main(void)
{
        TaskHandle_t handle;
        xTaskCreate(main_task, "MAIN", 2048, NULL, 1, &handle);
        vTaskStartScheduler();
}
#endif

and here are the results:

I (301) main_task: Calling app_main()
Iteration 0: last=1, tick=1, rv=0
Iteration 1: last=1, tick=4, rv=0
Iteration 2: last=1, tick=6, rv=0
I (406) main_task: Returned from app_main()
Iteration 3: last=6, tick=22, rv=1
Iteration 4: last=21, tick=26, rv=3
Iteration 5: last=26, tick=31, rv=1
Iteration 6: last=31, tick=36, rv=1
Iteration 7: last=36, tick=41, rv=1
Iteration 8: last=41, tick=46, rv=1
Iteration 9: last=46, tick=51, rv=1

Checklist:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

#1349

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[archigup]

Is this not solved by the timer task?

[ntd]

Is this not solved by the timer task?

Not sure what to answer here. This PR just implements xTaskPeriodicDelay as a "saner" alternative to xTaskDelayUntil. You can check #1349 for the whole story.

[kstribrnAmzn]

@archigup this could be implemented with a signalling mechanism + a software timer task however that is more setup than simply calling this function. For that reason and because it avoids the runaway problem of xTaskDelayUntil I'm actually a fan of this function.

[kstribrnAmzn]

Suggested change

	TickType_t xTaskPeriodicDelay( TickType_t * const pxPreviousWakeTime,
	UBaseType_t xTaskPeriodicDelay( TickType_t * const pxPreviousWakeTime,

Let's use the unsigned based type since this is a simple count

[ntd]

My problem is this code:

/* This plays well with overflow */
const TickType_t xTicksElapsed = xTickCount - *pxPreviousWakeTime;

needs everything to be of the same type to work properly, so xTicksElapsed must be TickType_t. Then xIncrements (that is, what xTaskPeriodicDelay returns) is calculated as:

xIncrements = xTicksElapsed / xTimeIncrement;

and if I declare it as UBaseType_t I get the following error on my host:

error: conversion from ‘TickType_t’ {aka ‘long unsigned int’} to ‘UBaseType_t’ {aka ‘unsigned char’} may change value [[-Werror=conversion](https://gcc.gnu.org/onlinedocs/gcc-16.1.0/gcc/Warning-Options.html#index-Wconversion)]

I can cast it to remove the error, but in that case I think the result can easily overflow if I suspend a task for too much time.

[kstribrnAmzn]

Probably need a check for overflow here. Don't want xIncrements * xTimeIncrement to be exactly one more than the xTicksIncrements maximum possible value.

[ntd]

xTicksIncrements cannot overflow. If you substitute xIncrements you get:

xTicksIncrements = xTicksElapsed / xTimeIncrement * xTimeIncrement;

so xTicksIncrements would never exceed xTicksElapsed.

[kstribrnAmzn]

I'm not particularly worried about the failing UTs as those will need to be added.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[ntd]

@kstribrnAmzn Many thanks for the review. I force-pushed before realizing you rebased, so I force-pushed again... sorry for the mess.

Some issues you raised are still open though. In the specific, I don't know how to solve the UBaseType_t return value in the proper way.