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

fix the bloody longterm vtimer bug #374

Merged
merged 4 commits into from
Dec 4, 2013

Conversation

LudwigKnuepfer
Copy link
Member

You know who you are, I'm looking at you!

@LudwigKnuepfer
Copy link
Member Author

PS:
If you want to test this, I suggest reducing SECONDS_PER_TICK and MICROSECONDS_PER_TICK to 10 seconds or so... (https://github.com/RIOT-OS/RIOT/blob/master/sys/vtimer/vtimer.c#L22)

@LudwigKnuepfer
Copy link
Member Author

@kaspar030 This will need your ACK for the license again.

While you're at it: If you can still explain the purpose of https://github.com/RIOT-OS/RIOT/blob/master/sys/vtimer/vtimer.c#L70 I'll forgive you for not commenting it in the first place ;-)

    uint32_t next = hwtimer_next_absolute + longterm_tick_start;
    uint32_t now = HWTIMER_TICKS_TO_US(hwtimer_now());

    /* <YOUR COMMENT HERE> */
    if((next - HWTIMER_TICKS_TO_US(VTIMER_THRESHOLD) - now) > MICROSECONDS_PER_TICK ) {
        next = now + HWTIMER_TICKS_TO_US(VTIMER_BACKOFF);
    }

    hwtimer_id = hwtimer_set_absolute(HWTIMER_TICKS(next), vtimer_callback, NULL);

@OlegHahm
Copy link
Member

The parts I understood are fine and the rest seems at least not break anything more than before: ACK.

@LudwigKnuepfer
Copy link
Member Author

@kaspar030 ACK?

@mehlis
Copy link
Contributor

mehlis commented Dec 3, 2013

tested on native with ccn, I see no problems!

@mehlis
Copy link
Contributor

mehlis commented Dec 3, 2013

you can cherry pick https://github.com/mehlis/RIOT/commit/e0cdd99f6ec7d0fd7db9c9790605bf759f7b6c59 to fix all warnings in the vtimer (yay!)

@LudwigKnuepfer
Copy link
Member Author

@kaspar030 ping

@kaspar030
Copy link
Contributor

@LudwigOrtmann compiling now...

@LudwigKnuepfer
Copy link
Member Author

@kaspar030 you should acquire a faster computer... ;-)

@kaspar030
Copy link
Contributor

@LudwigOrtmann Actually it compiles and sems to work, but then I didn't really know how to test longtime timers, then I wanted to write an automated test, then there was other stuff to do. But as basic tests worked fine, I'll go with oleg. Can only be better, so
ACK. And good job, btw. ;)

kaspar030 added a commit that referenced this pull request Dec 4, 2013
@kaspar030 kaspar030 merged commit f1b060c into RIOT-OS:master Dec 4, 2013
@LudwigKnuepfer LudwigKnuepfer deleted the vtimer_fix_longterm branch December 5, 2013 11:22
thomaseichinger pushed a commit to thomaseichinger/RIOT that referenced this pull request Dec 9, 2013
@OlegHahm OlegHahm added the PR-award-nominee Deprecated. Will be removed soon. label Nov 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-award-nominee Deprecated. Will be removed soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants