0

Been tearing my hair out all day over this little problem I have that is proving hard to fix and I'm fairly sure there are things going on under the surface that I am not aware of. Any feedback or help is hugely welcomed.

I have a function which takes in a float value and returns a string of ASCII characters for output onto an LCD screen. The function is thus:

char returnString(float x, bool isTriggTemp)
{
    char numberString[3];
    char bufferString[2];
    char bufferStringDec[7];

    int integerVal = (int)x;

    if(isTriggTemp == false)
    {
        int decimalVal = (x-integerVal)*10000;
        sprintf(numberString, "%s.%s", itoa(bufferString,integerVal,10),
             itoa(bufferStringDec,decimalVal,10));
    }
    else
    {
        int decimalVal = (x-integerVal)*1000; 

        sprintf(numberString, "%s.%s", itoa(bufferString,integerVal,10),
             itoa(bufferStringDec,decimalVal,10));
    }

    return numberString;
}

The reason for the If statement and the bool is that there are two floats that can be passed to the function. One with 1 decimal place, and one with up to 5. I'm not to fussed about the 5 decimal place number as I am comparing the two floats later in the program and the relevant decimals seem roughly consistent.

My query stems from this line:

 int decimalVal = (x-integerVal)*10;

When used like this, which is what I would expect to have to use given the above logic, the outputted string reads "X.0" regardless of the X value.

Increasing it to 100:

 int decimalVal = (x-integerVal)*100;

Gives me the correct value for only even numbers. (X.2 is fine when the float is X.2), but with odd numbers I seem to get a rounded down version (X.3 float prints X.2, X.5 -> X.4) etc.

When I increase it to 1000, I begin to see the start of the problem:

int decimalVal = (x-integerVal)*1000;

For even values, X.4 for example, I get X.40 printed. And with odd numbers I get 0.01 less than the original float. E.g. X.5 - > X.49.

Obviously there's something amiss here and non-exact decimals are being cut off. A) how can I fix this? and B) Given the arithmetic I would have guessed that *10 should be used but *100 is the order of 10 that gives me the closest to the correct output.

Any help is much appreciated.

Danny_ds
  • 11,201
  • 1
  • 24
  • 46
James
  • 356
  • 2
  • 13
  • 2
    Why the convoluted approach for writing out %f? – George Houpis Jan 19 '16 at 18:21
  • 2
    Also, you are returning the wrong type. – George Houpis Jan 19 '16 at 18:21
  • You need to add half the resolution before you round down. So if you're going to round to the nearest 0.01, you need to add 0.005 first. – David Schwartz Jan 19 '16 at 18:25
  • 1
    The logic seems to work hard to avoid a direct solution such as `sprintf (buf, "%6.1f", x)`. Why? – wallyk Jan 19 '16 at 18:27
  • This is the way I was prescribed to do it I think, the device I'm working on accepts the returning of a char variable and my write to LCD method works with it. I'm not sure of the reasoning behind returning the type this way but it's the way I was told and it has been working fine. Whether this is correct coding practise is another matter so thanks for the feedback about it. @GeorgeHoupis I will also look into simplying the sprint() with just using the float value, I wasn't aware I could do it that way with multiple floats with different decimal places. – James Jan 19 '16 at 18:33
  • @wallyk Thanks I'll look into it, seems far more logical than the way I'm doing it now. However is the 6.1 in your example an arbitrary example or is there reasoning behind it? – James Jan 19 '16 at 18:34
  • If that is an embedded system, you really should rethink using `float` in general. Far by most times it is better just to treat an integer as fixed-point value. For output split into integer and fraction similar to what you already do and convert both as integers. Floats add quite a lot of overhead to your code (size, runtime, complexity). – too honest for this site Jan 19 '16 at 18:36
  • I'm not using float as an output, you are correct however in that this is an embedded system and the output to an LCD needs to be an ASCII string. which Is what I believe I am doing with the 'convoluted' sprintf. I am returning a string of ASCII chars. – James Jan 19 '16 at 18:38
  • I edited my comment. Just was a bit off-track on first reading. Also note that a floating point value has no specific number of fractional digits unless oyu explictily round it, and even then you might get unexpected results. Just ty to represent e.g. `0.1` as a sum of binary fraction. – too honest for this site Jan 19 '16 at 18:40
  • `sprintf("%s.%s", itoa(...), itoa(...));` into a buffer of size 3 is pretty much guaranteed undefined behavior (and pointless because you could just specify `"%d.%d"` and omit the two calls). – user4520 Jan 19 '16 at 18:41
  • `char returnString(...) { char numberString[3]; ... return numberString;}` is obviously wrong. Wrong type, small buffer, Buffer out of scope. – chux - Reinstate Monica Jan 19 '16 at 19:15
  • @szczurcio `"%d.%d"` would print `x= 123.001` as `123.1`. Perhaps `"%d.%03d"` ? – chux - Reinstate Monica Jan 19 '16 at 19:18
  • @Chux just for clarity, I am currently in the process of changing the type to char * as suggested. Is there s definitive value of a buffer size I should use? I was just under the impression that I needed to have one larger than the number of data elements. In terms of buffer out of scope, is that what you mean by make it static? I don't want to access the buffer outside of this function, simply return it to be used as a variable. – James Jan 19 '16 at 19:26
  • @Olaf: Many modern embedded systems have ample memory space and work natively with floats. ARM-7s in particular can do megaflops. ARM-9s do [1.3 Mflops per MHz](https://www.arm.com/products/processors/technologies/vector-floating-point.php)! – wallyk Jan 19 '16 at 20:09
  • @wallyk: 1) There is no ARM-7s. If you mean ARM7: that does not have an FPU and is replaced by Cortex-M (ARM-v6M and -v7M) of which most have **no** FPU at all. You might mean ARM-v7, but that is not the subject here and a very small part of the embedded market only. Please see the number of sold parts. 8 bitters are still around 50% with Cortex-M0(+) attacking and also replacing(!) M3/M4F of which only the latter has a limited (e.g. no double) FPU. Don't be confused by RasPi & Co; these systems are quite irrelevant. – too honest for this site Jan 19 '16 at 20:24
  • @wallyk: There are other factors like external components, temperature range or EMC which are relevant. If you ever tried passing an EMC compiance test, you happily reduce the system clock and avoid off-chip components. For memory, that is a matter of cost. If you have a yearly target of some 100k devices or even millions, you have to reduce costs per device to the minimum. So you spend more time optimising the code for size just to use a cheaper MCU. If that only safes you 1ct, that very much pays already. Oh, and there is no ARM-9 either. – too honest for this site Jan 19 '16 at 20:28
  • Oh, and there is no ARM-9. – too honest for this site Jan 19 '16 at 20:30
  • @James "I don't want to access the buffer outside of this function, simply return it to be used as a variable." is a contradiction. IAC, it is better to pass in the buffer and size. I will post an update. – chux - Reinstate Monica Jan 19 '16 at 20:40

4 Answers4

2

You can write the precision you want for floating-point numbers with %f. And even do the precision level dynamically:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

// Use malloc (drawback: need to track return and deallocate to prevent memory leak.)
char * returnString( float x, int isTriggTemp )
{
    char * buffer = malloc( 128 );
    if( buffer )
        sprintf( buffer, "%0.*f", isTriggTemp ? 4 : 5, x );
    return buffer;
}

// Use static buffer (drawback: non-reentrant)
char * returnStringStatic( float x, int isTriggTemp )
{
    static char buffer[128];
    sprintf( buffer, "%0.*f", isTriggTemp ? 4 : 5, x );
    return buffer;
}

// Use given buffer (drawback: caller needs to be aware of buffer size needs and additional parameters are involved)
char * returnStringGivenBuffer( char * buffer, float x, int isTriggTemp )
{
    sprintf( buffer, "%0.*f", isTriggTemp ? 4 : 5, x );
    return buffer;
}

int main( int argc, char ** argv )
{
    float val = 3.14159;
    int highprecision = 0;

    printf( "Using sprintf\n" );
    printf( "%0.1f\n", val );
    printf( "%0.5f\n", val );
    printf( "%0.*f\n", ( highprecision ? 5 : 1 ), val );
    highprecision = 1;
    printf( "%0.*f\n", ( highprecision ? 5 : 1 ), val );

    printf( "\nUsing sprintf\n" );
    char buffer[128];
    sprintf( buffer, "%0.1f", val );
    printf( "%s\n", buffer );
    sprintf( buffer, "%0.5f", val );
    printf( "%s\n", buffer );
    sprintf( buffer, "%0.*f", ( highprecision ? 5 : 1 ), val );
    printf( "%s\n", buffer );
    highprecision = 1;
    sprintf( buffer, "%0.*f", ( highprecision ? 5 : 1 ), val );
    printf( "%s\n", buffer );

    printf( "\nUsing dynamic allocation\n" );
    char * fval = returnString( val, 0 );
    printf( "%s\n", fval ? fval : "" );
    if( fval ) free( fval );
    fval = returnString( val, 1 );
    printf( "%s\n", fval ? fval : "" );
    if( fval ) free( fval );

    printf( "\nUsing static buffer\n" );
    char * ptr = returnStringStatic( val, 0 );
    printf( "%s\n", ptr );
    ptr = returnStringStatic( val, 1 );
    printf( "%s\n", ptr );

    printf( "\nUsing given buffer\n" );
    ptr = returnStringGivenBuffer( buffer, val, 0 );
    printf( "%s\n", ptr );
    ptr = returnStringGivenBuffer( buffer, val, 1 );
    printf( "%s\n", ptr );

    return 0;
}

Results:

Using sprintf
3.1
3.14159
3.1
3.14159

Using sprintf
3.1
3.14159
3.14159
3.14159

Using dynamic allocation
3.14159
3.1416

Using static buffer
3.14159
3.1416

Using given buffer
3.14159
3.1416
George Houpis
  • 1,729
  • 1
  • 9
  • 5
  • Potentially I wasn't clear, sorry it's been a long day. This function is required to turn a supplied float value into an ASCII string. Which is the reason for the sprintf and the itoa function calls. While your answer does help in terms of understanding setting a precision, in the context of my question however, I am confused as why *10 isn't the way to go in getting the correct decimal number from the float. – James Jan 19 '16 at 18:44
  • Your answer is ok for a hosted environment, e.g. a PC. On a typical bare-metal embedded system that massively bloats the code by adding floating point support to `printf` (typically the libraries use stripped-down functions if you don't need floating point support). – too honest for this site Jan 19 '16 at 18:45
  • Using IEEE floating point numbers is not infinite precision. You cannot expect to modify the original value with 100% precision. If you want the value in a string, you can still use sprintf with the exact same functionality of printf with the only difference being the first parameter (i.e. the target buffer). – George Houpis Jan 19 '16 at 18:46
  • @James: Please see the manpage of the `printf` family and the syntax of the format string. But please also see my other comment. – too honest for this site Jan 19 '16 at 18:47
  • @Olaf Thanks but the system is required to use floating point numbers so using those libraries are a necessary evil. GeorgeHopis, am I right in understanding that then I require some way of rounding the values? I wasn't aware floating points weren't as precise as I thought, especially to the minor amount of decimal places – James Jan 19 '16 at 18:54
  • @James Olaf was talking specifically about floating point extensions to the `printf` family. As answers show, you don't need to do it that way. – Weather Vane Jan 19 '16 at 18:56
  • @James floating point is not always precise, [see this](http://stackoverflow.com/questions/588004/is-floating-point-math-broken) – Weather Vane Jan 19 '16 at 19:01
  • @GeorgeHoupis Wow! Many thanks for the effort that went into that! I have received an answer below which fixes my problem, but it's worth mentioning that I'm endeavouring to use one of the examples above as per proper coding practice so I might learn something on the way. Many thanks for your time. – James Jan 19 '16 at 19:21
  • @WeatherVane: While I primarily really meant the `printf` functions, it is most times advisable not to use floatingpoint in general in an embedded project. While the ARM Cortex-M4F relaxed that recommendation a bit with its FPU, Yet it still is a good idea to use float sparsely (but not anymore "avoid at all cost"). `double` still is a no-go (the CM4F e.g. does not support `double` floats in hardware). Quite often `float can be easily avoided if internal interfaces have been planned carefully by an experienced embedded systems architect. Too bad that appears to be a vanishing skill. – too honest for this site Jan 19 '16 at 19:24
  • @James: You really should read [this](http://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html). All: please note this is linked on the C info-page here and should be always promoted for such basic questions about floats. – too honest for this site Jan 19 '16 at 19:25
  • @Olaf thanks I'll have a read, the main cause of confusion was why it was rounding the float up at only 1 decimal place. something I've never seen before when having issues with floats. I appreciate the further reading however – James Jan 19 '16 at 19:28
  • @Olaf you are quite right. As the architect of many embedded projects I have at different times used fixed point arithmetic, or written my own floating point library due to the manufacturer's supplied library being so inefficient. – Weather Vane Jan 19 '16 at 19:29
  • And @Olaf I also wrote my own MPU emulations for PC (the manufacturers' offerings, if any, were next to useless) for whatever I was writing for, which would run at real speed and include watches and breakpoints and use the symbol table. That's the way to properly understand the unit you are working with, and it was necessary also because the hardware would never be available until a week before release date. Things have moved on now, and become more complex too, but support has improved. But as you say, my skill is a dying one. – Weather Vane Jan 19 '16 at 19:41
  • @WeatherVane: "... support has improved" - seriously? Ok, there are forums now, but I would not call user-discussion forums at the vendor "support". But maybe it's just me always asking the "wrong" questions: those which really need detailed knowledge of the module internals. – too honest for this site Jan 19 '16 at 20:03
1

OP approach to printing a fraction fails under various situations

  1. fraction is close to 1.0. returnString(0.999999, isTriggTemp)

  2. x value greater than INT_MAX.

  3. Negative numbers.

A more reliable method to to scale first and then break into integer/fraction parts.

char *returnString(float x, bool isTriggTemp) {
   float scale = 10000;
   x = roundf(x * scale);
   float ipart = x/scale;
   float dpart = fmodf(fabsf(x), scale);
   itoa(bufferString,ipart,10);
   itoa(bufferStringDec,dpart,10);  // May need to add leading zeros
   ...

[Edit]

An easy way to have leading zeros:

   static char bufferString[20+7];
   char bufferStringDec[7];

   itoa(bufferStringDec,dpart + scale,10);
   bufferStringDec[0] = '.';  // Overwrite leading `1`.
   return strcat(bufferStringDec, bufferStringDec);

Of course code could use

void returnString(char *numberString, size_t size, float x, bool isTriggTemp) {
  snprintf(numberString, sizeof numberString, "%.*f", isTriggTemp ? 3 : 4, x);
}

Insure buffers are static or otherwise available after the function completes.

Check return type.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
1

Are you sure this code doesn't crash? Your buffers are so small I think you might be overflowing your buffers. Anyway, the reason evens work and odds don't may be because when you do things like int decimalVal = (x-integerVal)*10000; things get rounded down. For example, int x = (int)((2.29 - 2)*10) will give you 2, not 3. In this example, the fix is to add .05 before you multiply by 10. I'm sure you can figure out the general rule.

Bing Bang
  • 524
  • 7
  • 16
  • This answer did indeed fix the problem for me (so many thanks for that!), however there have been many valid points raised above as to proper coding practice or better ways of doing it so I'm going to change the way in which I go about the function purpose. However thanks again for a concise response! – James Jan 19 '16 at 19:18
0

Mr. Houpis answer is the best thing to pursue, but for your edification, you probably need to do the calculations as floats:

int decimalVal = (x - (float) integerVal) * 10000.0;

In addition, your declaration for returnString should be char* rather than char.

Rumbleweed
  • 370
  • 1
  • 9