[DONE] chprintf: float precision

[russian]

While chvprintf has 'precision' mentioned in the comment, it seems to me that currently the 'precision' variable is only used as maximum string length?

And %f is hard-coded to be five digit precision. Would you consider a patch which uses this variable for floating conversion precision?

Code: Select all

Index: chprintf.c
===================================================================
--- chprintf.c   (revision 6773)
+++ chprintf.c   (working copy)
@@ -73,10 +73,12 @@
 }
 
 #if CHPRINTF_USE_FLOAT
-static char *ftoa(char *p, double num) {
+static char *ftoa(char *p, double num, unsigned long precision) {
   long l;
-  unsigned long precision = FLOAT_PRECISION;
+  if (precision == 0)
+    precision = FLOAT_PRECISION;
 
+
   l = (long)num;
   p = long_to_string_with_divisor(p, l, 10, 0);
   *p++ = '.';
@@ -211,7 +213,7 @@
         *p++ = '-';
         f = -f;
       }
-      p = ftoa(p, f);
+      p = ftoa(p, f, precision);
       break;
 #endif
     case 'X':

With this patch code like

Code: Select all

    chprintf(chp, "d %f\r\n", 0.12345678);
    chprintf(chp, "0 %..1f\r\n", 0.12345678);
    chprintf(chp, "3 %..1000f\r\n", 0.12345678);
    chprintf(chp, "6 %..1000000f\r\n", 0.12345678);

outputs

Code: Select all

d 0.12345
0 0.0
3 0.123
6 0.123456

(yes, with two dots between % and f)

[Giovanni]

Why the double dot? is this standard?

Giovanni

[russian]

Let me try to figure out why the double dot. I just know that it works this way with just a 3 line change of the official source code - I am trying to minimize the patch.

The standard would probably be .5 instead of .10000 anyway, and 10000 is the default precision.

[russian]

The double dot is not standard, the double dot is existing Chibi implementation - the first dot turns off filler? something like that, the second dot is precision marker. I have not understood this implementation yet, but I am pretty sure that that's what is in the source code.

What is in the standard is that precision is defined as "%.2f" if you want to get "3.14" and not "3.1415"

Here is a patch which makes a step toward this by at least making precision configurable:

Code: Select all

Index: os/various/chprintf.c
===================================================================
--- os/various/chprintf.c   (revision 6820)
+++ os/various/chprintf.c   (working copy)
@@ -32,7 +32,7 @@
 #include "memstreams.h"
 
 #define MAX_FILLER 11
-#define FLOAT_PRECISION 100000
+#define FLOAT_PRECISION 5
 
 static char *long_to_string_with_divisor(char *p,
                                          long num,
@@ -73,10 +73,15 @@
 }
 
 #if CHPRINTF_USE_FLOAT
-static char *ftoa(char *p, double num) {
+static long pow10[] = {10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000, 10000000000, 100000000000, 1000000000000, 10000000000000};
+
+static char *ftoa(char *p, double num, unsigned long precision) {
   long l;
-  unsigned long precision = FLOAT_PRECISION;
+  if (precision == 0)
+    precision = FLOAT_PRECISION;
+  precision = pow10[precision - 1];
 
+
   l = (long)num;
   p = long_to_string_with_divisor(p, l, 10, 0);
   *p++ = '.';
@@ -211,7 +216,7 @@
         *p++ = '-';
         f = -f;
       }
-      p = ftoa(p, f);
+      p = ftoa(p, f, precision);
       break;
 #endif
     case 'X':

Is there any chance for this patch to make it into the official source code?

[Giovanni]

I need time to consider changes, chprintf is not exactly on top of priorities.

Giovanni

[enly1]

Good timing - was just about to address this myself today as I need specific precision - manually added the patch by hand (don't know how to patch the code with tools) and got it working fine.

The only issue now is that the float handling doesn't actually work properly and has some rounding issues.

-2.123f @ 3 digit precision comes out as -2.122
-2.123f @ 5 digit precision comes out as -2.12299

But one for another night as its getting late ...

[skute]

Good timing - was just about to address this myself today as I need specific precision - manually added the patch by hand (don't know how to patch the code with tools) and got it working fine.

The only issue now is that the float handling doesn't actually work properly and has some rounding issues.

-2.123f @ 3 digit precision comes out as -2.122
-2.123f @ 5 digit precision comes out as -2.12299

But one for another night as its getting late ...

This is the correct behavior for how chprintf is written. Floating point is an approximation; -2.123f equates to -2.12299991.

[enly1]

Yes, I realised that - just not what I was expecting the function to perform.

I originally expected accurate responses at least to a few decimal places - so maybe my terminology was wrong - just need a different ftoa implementation.

[utzig]

The reason for the two dots in float notation is that the filler code is accepting a dot too:

Code: Select all

filler = ' ';
if ((*fmt == '.') || (*fmt == '0')) {
  fmt++;
  filler = '0';
}

This doesn't seem to be correct. Filler should be only '0' in this case:

http://www.cplusplus.com/reference/cstdio/printf/

I think the code should be changed to:

Code: Select all

filler = ' ';
if (*fmt == '0') {
  fmt++;
  filler = '0';
}

Best regards,
Fabio Utzig

[Giovanni]

Added to the TODO list, I will merge both suggestions.

Giovanni