CODEWITHSUNDEEP
c
Danijel
Danijel

Reputation: 8600

C defines proper usage

A problem with C defines: 

#define RANGE1_ms 64
#define FS 16000.0f
#define BS 64
#define COUNTER1_LIMIT ( ( RANGE1_ms/1000.0f)* FS/BS )

This gives 16.0 for COUNTER1_LIMIT.

Debugging the code in Eclipse shows all is OK. However, when I build release version from a makefile, it produces different result. I have narrowed the problem down to this line:

if( counter1 == (uint16_t)COUNTER1_LIMIT )
{
    ...
}

where counter1 is uint16_t.

What am I doing wrong with those defines?

This solves the problem:

if( counter1 == 16 )

but that's not the way to go.

Upvotes: 1

Views: 116

Answers (4)

chux
chux

Reputation: 153602

Avoid FP math with the pre-processor.

// Avoid the following
#define FS 16000.0f
#define COUNTER1_LIMIT ( ( RANGE1_ms/1000.0f)* FS/BS )

Alternative 1: use integer math that rounds quotients to nearest. Adding half the divisor works when dividend/divisor are positive.

#define RANGE1_ms 64
#define FS 16000
#define BS 64
#define ms_PER_s 1000
#define COUNTER1_LIMIT_N (1ul * RANGE1_ms * FS)
#define COUNTER1_LIMIT_D (1ul * ms_PER_s * BS )
#define COUNTER1_LIMIT_I ((COUNTER1_LIMIT_N + COUNTER1_LIMIT_D/2)/COUNTER1_LIMIT_D)
#define COUNTER1_LIMIT_F (1.0*COUNTER1_LIMIT_N/COUNTER1_LIMIT_D)

if (counter1 == COUNTER1_LIMIT_I)

Alternative 2:

When constants like FS truly need to be FP like 16123.4f, use a rounding function rather than truncation with an integer cast like (uint16_t)

#include <math.h>
if (counter1 == lround(COUNTER1_LIMIT))

Alternative 3:

When constants like FS truly need to be FP like 16123.4f, add 0.5 then truncate with an integer cast like (uint16_t). The add 0.5 trick works when the value is positive. It fails with a number of values when adding 0.5 is not exact. Yet has the advantage: it can be computed, as in OP's case, at compile time.

if (counter1 == (uint16_t)(COUNTER1_LIMIT + 0.5))

Upvotes: 1

user31264
user31264

Reputation: 6737

RANGE1_ms/1000.0f is 64/1000.0f . The problem is, computers cannot represent 0.064 exactly. Ultimately, numbers are represented as a*2b, where a and b are integers, which is impossible for 0.064 . So, RANGE1_ms/1000.0f is approximately 0.064. When you multiply it by 16000f and divide by 64, you receive approximately 16f. It may be 15.999999, or 16.000003, or something. When you cast it to uint16_t, it becomes either 15 or 16.

One way is to cast it as (uint16_t)(COUNTER1_LIMIT+0.5). It will round it to the closest integer. Alternatively, if your COUNTER1_LIMIT is always integer (that is you know that the expression should produce integer number), you may do

#define FS_BY_1000 16
#define COUNTER1_LIMIT (RANGE1_ms * FS_BY_1000/BS)

Upvotes: 1

Lundin
Lundin

Reputation: 214040

The reason for the bug is that floating point numbers are accurate, see the millions of others posts on SO about this, for example Why Are Floating Point Numbers Inaccurate?

But in your specific case the problem is that you use floating point where it isn't needed or useful. You just need a compile-time constant. Fix the expression like this:

#define RANGE1_ms 64u
#define COUNTER1_LIMIT (16000u / 64u * RANGE1_ms / 1000u )

Upvotes: 2

Ajay Brahmakshatriya
Ajay Brahmakshatriya

Reputation: 9203

Floating point numbers are susceptible to loss of precision. The 16.0 can get converted to 15.9999 when loaded as a temporary. Casting it to uint16_t makes it 15 that doesn't compare against 16.

You need a function to round the floating point values. That you can call on COUNTER1_LIMIT.

Other option would be to promote counter1 to float and check if the absolute difference between this and COUNTER1_LIMIT is less than a small value like 0.001.

It can be done as

float diff = counter1 - COUNTER1_LIMIT;
if(diff > -0.001 && diff < 0.001) { 
    ...
}

This should work for you.

Upvotes: 1

Related Questions