search

Ableton / link

Ableton Link
Other
1.09k stars 149 forks source link

Use timer and interrupt to notify Link task instead of portYIELD #106

Closed mathiasbredholt closed 2 years ago

mathiasbredholt commented 2 years ago

To resolve an issue with the task watchdog being triggered, I propose to replace portYIELD() with a timer that notifies the Link task to run with a period of 100 microseconds.

Optionally it would be nice to add configuration options for which timer to use.

This PR proposes a more elegant solution to poll the Context faster than the FreeRTOS tick rate. The existing solution with using portYIELD() forces the scheduler to run after each poll, which effectively preempts all tasks running at a lower priority than the Link task (eg. the IDLE tasks which run at priority 0, lower than the Link task at priority 2). This can trigger the watchdog as the IDLE task is not allowed to run (depending on the configuration).

By using a timer + interrupt that notifies the Link task to run, we obtain a solution that allows to poll the Context at a fixed rate that is independent of the FreeRTOS tick rate, while allowing other tasks with lower priority to run as well.

I've done tests with different update intervals and 100 microseconds seem to offer good performance, while giving time to the scheduler for running other tasks with lower priority.

mathiasbredholt commented 2 years ago

I pushed a fix for the formatting errors :)

fgo-ableton commented 2 years ago

Hey, I can't reproduce any issues using esp idf 4.3.1. In this issue the pasted logs use ESP-IDF v4.3.1-dirty - so that is probably a random mix of v4 and v5 bits. So I guess this is actually in preparation for esp idv v5, right? It would be cool if you could clarify that in the PR description. Anyways, I just tested it and it seems to work.

mathiasbredholt commented 2 years ago

Hi @fgo-ableton, thank you for your response, this PR is for improving applications with ESP-IDF v4.3.1, while #107 is for compatibility purposes with existing and future versions of ESP-IDF.

I've updated the description to explain the benefits of this PR.