Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alarm pool can appear to stop working #1953

Closed
peterharperuk opened this issue Sep 25, 2024 · 1 comment
Closed

alarm pool can appear to stop working #1953

peterharperuk opened this issue Sep 25, 2024 · 1 comment
Assignees
Milestone

Comments

@peterharperuk
Copy link
Contributor

peterharperuk commented Sep 25, 2024

We have an issue with a program on RP2040 that we communicate with via a USB, it was occasionally unresponsive. After some investigation a repeated alarm callback that is used to call tud_task_ext was not happening. This only seems to be a problem on sdk 2.0. Further investigation suggests it's possible for the alarm pool to register an alarm in the past, so it'll only go off in ~71 minutes. The attached program seems to reproduce the problem.

The suggested fix to alarm_pool_irq_handler is to update "now" after the call to "ta_time_us_64"

diff --git a/src/common/pico_time/time.c b/src/common/pico_time/time.c
index 118547d8..2f5409a4 100644
--- a/src/common/pico_time/time.c
+++ b/src/common/pico_time/time.c
@@ -259,13 +259,13 @@ static void alarm_pool_irq_handler(void) {
                 index = next;
             }
         }
-        now = (int64_t) ta_time_us_64(timer);
         earliest_index = pool->ordered_head;
         if (earliest_index < 0) break;
         // need to wait
         alarm_pool_entry_t *earliest_entry = &pool->entries[earliest_index];
         earliest_target = earliest_entry->target;
         ta_set_timeout(timer, timer_alarm_num, earliest_target);
+        now = (int64_t) ta_time_us_64(timer);
         // check we haven't now past the target time; if not we don't want to loop again
     } while ((earliest_target - now) <= 0);
 }

alarm_test.zip

@peterharperuk peterharperuk self-assigned this Sep 25, 2024
peterharperuk added a commit to peterharperuk/pico-sdk that referenced this issue Sep 25, 2024
@kilograham kilograham added this to the 2.0.1 milestone Sep 25, 2024
kilograham pushed a commit that referenced this issue Sep 26, 2024
* Avoid setting an alarm in the past

Fixes #1953

* Call ta_time_us_64 when needed

Remove local "now" variable.
@lurch
Copy link
Contributor

lurch commented Sep 26, 2024

Fixed in develop

@lurch lurch closed this as completed Sep 26, 2024
peterharperuk added a commit to peterharperuk/pico-sdk that referenced this issue Oct 7, 2024
peterharperuk added a commit that referenced this issue Nov 5, 2024
* Test for best_effort_wfe_or_timeout sev issue

best_effort_wfe_or_timeout should not ignore an outstanding sev
See #1812

* Test for alarm being set in the past issue

See #1953
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants