ARMmbed / minar

mbed OS scheduler
Other
25 stars 21 forks source link

Setting tolerance to zero milliseconds causes strange behavior in scheduling #41

Open tibmanus opened 8 years ago

tibmanus commented 8 years ago

Steps to reproduce: Schedule a periodic task. When the tolerance is different from zero, everything works as expected. When tolerance is set to zero, the task (every now and then) will experience random deviations from the scheduled time.

Code (blinky example):

#include "mbed-drivers/mbed.h"
#include "mbed-hal/lp_ticker_api.h"
using minar::Scheduler;

DigitalOut led(LED1);

static void blinky(void) {
    led = !led; 
    printf("%lu \r\n", lp_ticker_read());
}   

void app_start(int, char**){
    Scheduler::postCallback(blinky).tolerance(minar::milliseconds(0)).period(minar::milliseconds(1000));
}  

I was using an FRDM-K64F board

ciarmcom commented 8 years ago

ARM Internal Ref: IOTSFW-2422

0xc0170 commented 8 years ago

Can you share the output from the console, what are those deviations you are experiencing?

It might be worth to create a test for lp ticker against us ticker. I have something somewhere in my stash. If you can provide the details about how long ot ge those, to find out if the ticker is buggy or somewhere else is the problem

betzw commented 8 years ago

I am currently debugging exactly the same issue on Nucleo-F401RE upon all in order to figure out if it is platform dependent, but this post here seems to strengthen my believe that it is a general issue (maybe a memory leak). Will let you know in case I am figuring out something!

betzw commented 8 years ago

Maybe I have seen something: Pls. take a look at https://github.com/ARMmbed/minar/blob/master/source/minar.cpp#L447-L467:

static bool minar::timeIsInPeriod(minar::tick_t start, minar::tick_t time, minar::tick_t end){
    // Taking care to handle wrapping: (M = now + Minumum_Sleep)
    //   Case (A.1)
    //                       S    T   E
    //      0 ---------------|----|---|-- 0xf
    //
    //   Case (A.2): this case also allows S==T==E
    //         E                 S    T
    //      0 -|-----------------|----|-- 0xf
    //
    //   Case (B)
    //         T   E                 S
    //      0 -|---|-----------------|--- 0xf
    //
    if((time >= start && ( time < end ||    // (A.1)
                          start >= end)) || // (A.2)
        (time < start && end < start && end > time)){  // (B)
        return true;
    }
    return false;
}

and consider case (A.2). In this case the function timeIsInPeriod() will return true also in all cases where (start == end) && (time >= start) (and not only when start==time==end as the comment says) which is not OK at all in my opinion!

betzw commented 8 years ago

Modifying things to:

static bool minar::timeIsInPeriod(minar::tick_t start, minar::tick_t time, minar::tick_t end){
    // Taking care to handle wrapping:
    //   Case (A.1): this case also allows S==T==E
    //                       S    T   E
    //      0 ---------------|----|---|-- 0xf
    //
    //   Case (A.2)
    //         E                 S    T
    //      0 -|-----------------|----|-- 0xf
    //
    //   Case (B)
    //         T   E                 S
    //      0 -|---|-----------------|--- 0xf
    //
    if((time >= start && ( time <= end  || // (A.1)
                          start > end)) || // (A.2)
        (time < start && end < start && end > time)){  // (B)
        return true;
    }
    return false;
}

Seems to have a positive effect (at least for this simple example).

tibmanus commented 8 years ago

I realized my example code was very stupid, and tried to reproduce it with something better, but I had no luck. Nice catch @betzw!

betzw commented 8 years ago

See PR #43