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

xtimer_usleep not working on AVR and msp430 boards #6392

Closed
kYc0o opened this issue Jan 17, 2017 · 14 comments
Closed

xtimer_usleep not working on AVR and msp430 boards #6392

kYc0o opened this issue Jan 17, 2017 · 14 comments
Assignees
Labels
Area: timers Area: timer subsystems Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@kYc0o
Copy link
Contributor

kYc0o commented Jan 17, 2017

While testing some drivers I found that the function xtimer_usleep() has no impact on AVR8 based boards.

I'm trying to look for a solution but as I'm not an xtimer expert it can take long time...

Any hints @kaspar030 @gebart ?

@kYc0o kYc0o added Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: timers Area: timer subsystems labels Jan 17, 2017
@kYc0o kYc0o added this to the Release 2017.01 milestone Jan 17, 2017
@kYc0o kYc0o changed the title xtimer_usleep not working on AVR boards xtimer_usleep not working on AVR and msp430 boards Jan 17, 2017
@kYc0o kYc0o added the Platform: MSP Platform: This PR/issue effects MSP-based platforms label Jan 17, 2017
@kYc0o
Copy link
Contributor Author

kYc0o commented Jan 17, 2017

Just realised that it doesn't work either on msp430...

@jnohlgard
Copy link
Member

I don't think I will have any time to look at this within the near future, sorry.

@OlegHahm
Copy link
Member

Have you checked what the underlying _xtimer_tsleep() does on these platforms? Does _xtimer_spin() work properly here? Are the spin barriers configured correctly?

@kYc0o
Copy link
Contributor Author

kYc0o commented Jan 17, 2017

I'll check. It also fails on cc2538...

@kYc0o
Copy link
Contributor Author

kYc0o commented Jan 17, 2017

Ok I made some tests and found a strange behaviour on avr-gcc compiler (I think).

While doing:

#define ONE_SEC_SLEEP     (1 * 1000U * 1000U)

int main (void)
{
    while (1) {
        printf("Sleeping for 1 sec...\n");
        xtimer_usleep(ONE_SEC_SLEEP);
    }

    return 0;
}

The boards arduino-mega2560 and arduino-uno displays the text without sleeping any second. BUT if I change the define by:

#define ONE_SEC_SLEEP     (1 * 1000000U)

it works... Any hints?

For boards like waspmote-pro and remote-reva which use different frequencies for the xtimer the test fails anyways.

@OlegHahm
Copy link
Member

Doesn't the U tells the compiler to use an unsigned int? On 8 and 16 bit platforms this has probably only a 16-bit width which cannot hold 1000000. Try UL instead.

@kYc0o
Copy link
Contributor Author

kYc0o commented Jan 17, 2017

Try UL instead.

This was the problem.

There is some some code on RIOT which doesn't take into account this limitation (xbee driver for instance) so such code doesn't work on 16 and 8 bit platforms. What can we do?

@OlegHahm
Copy link
Member

There is some some code on RIOT which doesn't take into account this limitation (xbee driver for instance) so such code doesn't work on 16 and 8 bit platforms. What can we do?

Open a PR to fix it.

@kYc0o
Copy link
Contributor Author

kYc0o commented Jan 17, 2017

Open a PR to fix it.

It's maybe hard to find ALL the places when such a thing was coded... For now I'll fix the XBee driver but for the rest I'll try to find some time to look further.

@OlegHahm
Copy link
Member

Well, obviously we can only fix what we are aware of. Not sure what answer you expected. Code reviews and testing should prevent these kind of bugs, but unless we have a CI that runs tests on various platforms, I don't see what else we can do.

@kYc0o
Copy link
Contributor Author

kYc0o commented Jan 20, 2017

Well the reason of this issue was because some integers were not correctly sized for these platforms, so we can close it.

However, the problem of a broken XTIMER persists in boards which are not using compatible clock speeds. Namely, for now, waspmote-pro and cc2538.

@kYc0o kYc0o closed this as completed Jan 20, 2017
@PeterKietzmann
Copy link
Member

If the problem persists, why are you closing this issue. Is it noted somewhere else? CC2538 is not avr or msp430.

@OlegHahm
Copy link
Member

There is no problem with xtimer_usleep() as far as I understood it. The problem was rather that xtimer_usleep(1000U * 1000U) evaluates to xtimer_usleep(1000000 % 2^16) on 8 and 16 bit platforms.

@kYc0o
Copy link
Contributor Author

kYc0o commented Jan 20, 2017

Because the issue described here and in the title was fixed in #6406 for the XBee example and a test in #6394 was added.

Two problems were noticed from this one: Check for size compatibility on constants used widely for 8 and 16 bit platforms and XTIMER broken for the two boards mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: timers Area: timer subsystems Platform: AVR Platform: This PR/issue effects AVR-based platforms Platform: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

No branches or pull requests

5 participants