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

uptime wraps too soon #47

Closed
pwittich opened this issue Mar 30, 2020 · 2 comments
Closed

uptime wraps too soon #47

pwittich opened this issue Mar 30, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@pwittich
Copy link
Contributor

pwittich commented Mar 30, 2020

Describe the bug
uptime seems to roll over after less than 1000 minutes (not clear about reason)

To Reproduce
Steps to reproduce the behavior:

  1. look at uptime
  2. it's never more than a few hundred minutes

It ought to be ok; TickType_t is AFAIK a signed 32 bit integer. The number is multiplied by some constants; hopefully the compiler avoids unneeded up- and down-multipliers. According to my estimate the overall scaling is (1000/100)*(1/60000). The tick rate is 100 Hz. So this should reduce to 1/6000, hopefully at compile time. Max is like 2x10^9 for a 32 bit signed integer, so the max number of minutes should be like 33k. What am I doing wrong here?

image

static BaseType_t uptime(int argc, char ** argv)
{
int s = SCRATCH_SIZE;
TickType_t now = pdTICKS_TO_MS( xTaskGetTickCount())/1000/60; // time in minutes
snprintf(m,s, "%s: MCU uptime %d minutes\r\n", argv[0], now);
return pdFALSE;
}

@pwittich pwittich added the bug Something isn't working label Mar 30, 2020
@pwittich
Copy link
Contributor Author

pwittich commented Mar 30, 2020

Looking at the assembly output it looks like the compiler does not reduce this to 1/6000.

        bl      xTaskGetTickCount
        mov     r2, r0
        mov     r3, #1000
        mul     r3, r3, r2
        .loc 1 1116 14
        ldr     r2, .L354
        umull   r2, r3, r2, r3
        lsrs    r3, r3, #19

So we have an explicit multiplication by 1000 and then a division by a constant. The value stored at L354 is 375299969, so the umul followed by the shift corresponds to a multiplication of 1/5999999.999161332198245, i.e., 1/6e6.

@pwittich
Copy link
Contributor Author

If I change the code in question to the following instead

  TickType_t now = xTaskGetTickCount()/(configTICK_RATE_HZ*60); // time in minutes

the generated assembly is now

        bl      xTaskGetTickCount
        mov     r2, r0
        .loc 1 1115 14
        ldr     r3, .L354
        umull   r2, r3, r3, r2
        lsrs    r3, r3, #7

where the value at 354 is now 91625969, which with the shift corresponds to multiplication by 1/6000.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant