gh-raspberrypi/pico-sdk

Watch

Fork 0

alarm_pool_add_alarm_at() returns 0 #614

New issue

Closed

opened 2021-10-14 19:55:49 -05:00 by smdjeff · 8 comments

smdjeff commented

2021-10-14 19:55:49 -05:00

(Migrated from github.com)

I believe there is some confusion about whether or not alarm_id 0 is valid. I tried this and it fails...

bool success = add_repeating_timer_ms( millis, callback, userdata, &timers ); 
assert( success );

Perhaps the Timer documentation and alarm_pool_add_repeating_timer_us() return calculation is incorrect?

I believe there is some confusion about whether or not alarm_id 0 is valid. I tried this and it fails... ``` bool success = add_repeating_timer_ms( millis, callback, userdata, &timers ); assert( success ); ``` Perhaps the Timer documentation and alarm_pool_add_repeating_timer_us() return calculation is incorrect?

kilograham commented

2021-10-14 23:16:18 -05:00

(Migrated from github.com)

I assume you mean the return out->alarm_id > 0; in add_repeating_time_ms?
yes that looks like a bug; should be >= 0

I assume you mean the `return out->alarm_id > 0;` in `add_repeating_time_ms`? yes that looks like a bug; should be `>= 0`

👍 1

kilograham commented

2021-10-19 17:18:06 -05:00

(Migrated from github.com)

Looking closer, i imagine this can only happen if your repeating timer callback cancels the repeat immediately?

smdjeff commented

2021-10-19 17:34:27 -05:00

(Migrated from github.com)

Well, that's the confusion. Is alarm_id_t = 0 the first valid alarm_id... and -1 is the error. or is 0 also an error?

kilograham commented

2021-10-19 17:38:11 -05:00

(Migrated from github.com)

0 is not a valid alarm_id
0 is a valid return from alarm_pool_add_repeating_timer_us() if there is no alarm remaining once the function returns - if fire_if_past is true then the callback will have been called but alarm_pool_add_repeating_timer_us() can return 0 still if the callback itself cancelled the timer
i have update the docs - about to post a PR

0 is not a valid alarm_id 0 is a valid return from alarm_pool_add_repeating_timer_us() if there is no alarm remaining once the function returns - if fire_if_past is true then the callback will have been called but alarm_pool_add_repeating_timer_us() can return 0 still if the callback itself cancelled the timer i have update the docs - about to post a PR

smdjeff commented

2021-10-19 17:40:35 -05:00

(Migrated from github.com)

alarm_id_t
typedef int32_t alarm_id_t
The identifier for an alarm.

Note
this identifier is signed because -1 is used as an error condition when creating alarms
alarm ids may be reused, however for convenience the implementation makes an attempt to defer reusing as long as possible. You should certainly expect it to be hundreds of ids before one is reused, although in most cases it is more. Nonetheless care must still be taken when cancelling alarms or other functionality based on alarms when the alarm may have expired, as eventually the alarm id may be reused for another alarm.

https://raspberrypi.github.io/pico-sdk-doxygen/group__alarm.html

and several api say something like...

Returns
> 0 the alarm id
0 the target timestamp was during or before this method call (whether the callback was called depends on fire_if_past)
< -1 if there were no alarm slots available

really... i think there's a lot of inconsistency in the api return values maybe this is a hold over from the python world rather than taking a shot at if from the *nix world. . they should all return traditional error codes. with 0 = success. rather than mixing and matching ids and success = true booleans.

> alarm_id_t > typedef int32_t alarm_id_t > The identifier for an alarm. > > Note > this identifier is signed because -1 is used as an error condition when creating alarms > alarm ids may be reused, however for convenience the implementation makes an attempt to defer reusing as long as possible. You should certainly expect it to be hundreds of ids before one is reused, although in most cases it is more. Nonetheless care must still be taken when cancelling alarms or other functionality based on alarms when the alarm may have expired, as eventually the alarm id may be reused for another alarm. https://raspberrypi.github.io/pico-sdk-doxygen/group__alarm.html and several api say something like... > Returns > > 0 the alarm id > 0 the target timestamp was during or before this method call (whether the callback was called depends on fire_if_past) > < -1 if there were no alarm slots available really... i think there's a lot of inconsistency in the api return values maybe this is a hold over from the python world rather than taking a shot at if from the *nix world. . they should all return traditional error codes. with 0 = success. rather than mixing and matching ids and success = true booleans.

smdjeff commented

2021-10-19 17:47:33 -05:00

(Migrated from github.com)

alarm_pool_add_alarm_at()
        ...
       public_id = missed ? 0 : make_public_id(id_high, id);
        if (missed && fire_if_past) {
            int64_t repeat = callback(public_id, user_data);
            if (!repeat) {
                public_id = 0;
                ...
        return public_id;

I suppose that means alarm_id = 0 is a special case that means "calledback because missed, but not repeated" ?

``` alarm_pool_add_alarm_at() ... public_id = missed ? 0 : make_public_id(id_high, id); if (missed && fire_if_past) { int64_t repeat = callback(public_id, user_data); if (!repeat) { public_id = 0; ... return public_id; ``` I suppose that means alarm_id = 0 is a special case that means "calledback because missed, but not repeated" ?

kilograham commented

2021-10-19 18:02:13 -05:00

(Migrated from github.com)

from https://datasheets.raspberrypi.com/pico/raspberry-pi-pico-c-sdk.pdf

I suppose that means alarm_id = 0 is a special case that means "calledback because missed, but not repeated" ?

look at the linked PR for the new state of the world and see if that makes sense. I'd still like to know if your callback cancels the repeating timer in_your_ example

from https://datasheets.raspberrypi.com/pico/raspberry-pi-pico-c-sdk.pdf ![image](https://user-images.githubusercontent.com/575810/138002037-ea3dd5b4-00b2-4038-b979-afe584ea4fb9.png) > I suppose that means alarm_id = 0 is a special case that means "calledback because missed, but not repeated" ? look at the linked PR for the new state of the world and see if that makes sense. I'd still like to know if _your_ callback cancels the repeating timer in_your_ example

kilograham commented

2021-10-25 12:27:31 -05:00

(Migrated from github.com)