Explorar el Código

Fixed too many decimals places in float serialization (issue #543)

Benoit Blanchon hace 8 años
padre
commit
d41f7a8165

+ 1 - 0
CHANGELOG.md

@@ -8,6 +8,7 @@ HEAD
 * Fixed warning "dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]"
 * Fixed warning "floating constant exceeds range of 'float' [-Woverflow]" (issue #544)
 * Removed `ARDUINOJSON_DOUBLE_IS_64BITS` as it became useless.
+* Fixed too many decimals places in float serialization (issue #543)
 
 v5.11.0
 -------

+ 0 - 46
src/ArduinoJson/Polyfills/normalize.hpp

@@ -1,46 +0,0 @@
-// Copyright Benoit Blanchon 2014-2017
-// MIT License
-//
-// Arduino JSON library
-// https://bblanchon.github.io/ArduinoJson/
-// If you like this project, please add a star!
-
-#pragma once
-
-#include "../Configuration.hpp"
-#include "../TypeTraits/FloatTraits.hpp"
-
-namespace ArduinoJson {
-namespace Polyfills {
-template <typename T>
-int16_t normalize(T& value) {
-  using namespace TypeTraits;
-  int16_t powersOf10 = 0;
-
-  int8_t index = sizeof(T) == 8 ? 8 : 5;
-  int bit = 1 << index;
-
-  if (value >= ARDUINOJSON_POSITIVE_EXPONENTIATION_THRESHOLD) {
-    for (; index >= 0; index--) {
-      if (value >= FloatTraits<T>::positiveBinaryPowerOfTen(index)) {
-        value *= FloatTraits<T>::negativeBinaryPowerOfTen(index);
-        powersOf10 = int16_t(powersOf10 + bit);
-      }
-      bit >>= 1;
-    }
-  }
-
-  if (value > 0 && value <= ARDUINOJSON_NEGATIVE_EXPONENTIATION_THRESHOLD) {
-    for (; index >= 0; index--) {
-      if (value < FloatTraits<T>::negativeBinaryPowerOfTenPlusOne(index)) {
-        value *= FloatTraits<T>::positiveBinaryPowerOfTen(index);
-        powersOf10 = int16_t(powersOf10 - bit);
-      }
-      bit >>= 1;
-    }
-  }
-
-  return powersOf10;
-}
-}
-}

+ 94 - 0
src/ArduinoJson/Serialization/FloatParts.hpp

@@ -0,0 +1,94 @@
+// Copyright Benoit Blanchon 2014-2017
+// MIT License
+//
+// Arduino JSON library
+// https://bblanchon.github.io/ArduinoJson/
+// If you like this project, please add a star!
+
+#pragma once
+
+#include "../Configuration.hpp"
+#include "../Polyfills/math.hpp"
+#include "../TypeTraits/FloatTraits.hpp"
+
+namespace ArduinoJson {
+namespace Internals {
+
+template <typename TFloat>
+struct FloatParts {
+  uint32_t integral;
+  uint32_t decimal;
+  int16_t exponent;
+  int8_t decimalPlaces;
+
+  FloatParts(TFloat value) {
+    const uint32_t maxDecimalPart = sizeof(TFloat) >= 8 ? 1000000000 : 1000000;
+
+    exponent = normalize(value);
+
+    integral = uint32_t(value);
+    TFloat remainder = value - TFloat(integral);
+
+    remainder *= maxDecimalPart;
+    decimal = uint32_t(remainder);
+    remainder = remainder - TFloat(decimal);
+
+    // rounding:
+    // increment by 1 if remainder >= 0.5
+    decimal += uint32_t(remainder * 2);
+    if (decimal >= maxDecimalPart) {
+      decimal = 0;
+      integral++;
+      if (exponent && integral >= 10) {
+        exponent++;
+        integral = 1;
+      }
+    }
+
+    decimalPlaces = sizeof(TFloat) >= 8 ? 9 : 6;
+
+    // recude number of decimal places by the number of integral places
+    for (uint32_t tmp = integral; tmp >= 10; tmp /= 10) {
+      decimal /= 10;
+      decimalPlaces--;
+    }
+
+    // remove trailing zeros
+    while (decimal % 10 == 0 && decimalPlaces > 0) {
+      decimal /= 10;
+      decimalPlaces--;
+    }
+  }
+
+  static int16_t normalize(TFloat& value) {
+    typedef TypeTraits::FloatTraits<TFloat> traits;
+    int16_t powersOf10 = 0;
+
+    int8_t index = sizeof(TFloat) == 8 ? 8 : 5;
+    int bit = 1 << index;
+
+    if (value >= ARDUINOJSON_POSITIVE_EXPONENTIATION_THRESHOLD) {
+      for (; index >= 0; index--) {
+        if (value >= traits::positiveBinaryPowerOfTen(index)) {
+          value *= traits::negativeBinaryPowerOfTen(index);
+          powersOf10 = int16_t(powersOf10 + bit);
+        }
+        bit >>= 1;
+      }
+    }
+
+    if (value > 0 && value <= ARDUINOJSON_NEGATIVE_EXPONENTIATION_THRESHOLD) {
+      for (; index >= 0; index--) {
+        if (value < traits::negativeBinaryPowerOfTenPlusOne(index)) {
+          value *= traits::positiveBinaryPowerOfTen(index);
+          powersOf10 = int16_t(powersOf10 - bit);
+        }
+        bit >>= 1;
+      }
+    }
+
+    return powersOf10;
+  }
+};
+}
+}

+ 13 - 50
src/ArduinoJson/Serialization/JsonWriter.hpp

@@ -9,12 +9,9 @@
 
 #include <stdint.h>
 #include "../Data/Encoding.hpp"
-#include "../Data/JsonFloat.hpp"
 #include "../Data/JsonInteger.hpp"
 #include "../Polyfills/attributes.hpp"
-#include "../Polyfills/math.hpp"
-#include "../Polyfills/normalize.hpp"
-#include "../TypeTraits/FloatTraits.hpp"
+#include "../Serialization/FloatParts.hpp"
 
 namespace ArduinoJson {
 namespace Internals {
@@ -28,10 +25,6 @@ namespace Internals {
 // indentation.
 template <typename Print>
 class JsonWriter {
-  static const uint8_t maxDecimalPlaces = sizeof(JsonFloat) >= 8 ? 9 : 6;
-  static const uint32_t maxDecimalPart =
-      sizeof(JsonFloat) >= 8 ? 1000000000 : 1000000;
-
  public:
   explicit JsonWriter(Print &sink) : _sink(sink), _length(0) {}
 
@@ -87,7 +80,8 @@ class JsonWriter {
     }
   }
 
-  void writeFloat(JsonFloat value) {
+  template <typename TFloat>
+  void writeFloat(TFloat value) {
     if (Polyfills::isNaN(value)) return writeRaw("NaN");
 
     if (value < 0.0) {
@@ -97,28 +91,27 @@ class JsonWriter {
 
     if (Polyfills::isInfinity(value)) return writeRaw("Infinity");
 
-    uint32_t integralPart, decimalPart;
-    int16_t powersOf10;
-    splitFloat(value, integralPart, decimalPart, powersOf10);
+    FloatParts<TFloat> parts(value);
 
-    writeInteger(integralPart);
-    if (decimalPart) writeDecimals(decimalPart, maxDecimalPlaces);
+    writeInteger(parts.integral);
+    if (parts.decimalPlaces) writeDecimals(parts.decimal, parts.decimalPlaces);
 
-    if (powersOf10 < 0) {
+    if (parts.exponent < 0) {
       writeRaw("e-");
-      writeInteger(-powersOf10);
+      writeInteger(-parts.exponent);
     }
 
-    if (powersOf10 > 0) {
+    if (parts.exponent > 0) {
       writeRaw('e');
-      writeInteger(powersOf10);
+      writeInteger(parts.exponent);
     }
   }
 
   template <typename UInt>
   void writeInteger(UInt value) {
     char buffer[22];
-    char *ptr = buffer + sizeof(buffer) - 1;
+    char *end = buffer + sizeof(buffer) - 1;
+    char *ptr = end;
 
     *ptr = 0;
     do {
@@ -130,15 +123,9 @@ class JsonWriter {
   }
 
   void writeDecimals(uint32_t value, int8_t width) {
-    // remove trailing zeros
-    while (value % 10 == 0 && width > 0) {
-      value /= 10;
-      width--;
-    }
-
     // buffer should be big enough for all digits, the dot and the null
     // terminator
-    char buffer[maxDecimalPlaces + 2];
+    char buffer[16];
     char *ptr = buffer + sizeof(buffer) - 1;
 
     // write the string in reverse order
@@ -166,30 +153,6 @@ class JsonWriter {
 
  private:
   JsonWriter &operator=(const JsonWriter &);  // cannot be assigned
-
-  void splitFloat(JsonFloat value, uint32_t &integralPart,
-                  uint32_t &decimalPart, int16_t &powersOf10) {
-    powersOf10 = Polyfills::normalize(value);
-
-    integralPart = uint32_t(value);
-    JsonFloat remainder = value - JsonFloat(integralPart);
-
-    remainder *= maxDecimalPart;
-    decimalPart = uint32_t(remainder);
-    remainder = remainder - JsonFloat(decimalPart);
-
-    // rounding:
-    // increment by 1 if remainder >= 0.5
-    decimalPart += uint32_t(remainder * 2);
-    if (decimalPart >= maxDecimalPart) {
-      decimalPart = 0;
-      integralPart++;
-      if (powersOf10 && integralPart >= 10) {
-        powersOf10++;
-        integralPart = 1;
-      }
-    }
-  }
 };
 }
 }

+ 45 - 34
test/JsonWriter/writeFloat.cpp

@@ -14,7 +14,8 @@
 
 using namespace ArduinoJson::Internals;
 
-void check(double input, const std::string& expected) {
+template <typename TFloat>
+void check(TFloat input, const std::string& expected) {
   std::string output;
   DynamicStringBuilder<std::string> sb(output);
   JsonWriter<DynamicStringBuilder<std::string> > writer(sb);
@@ -23,83 +24,93 @@ void check(double input, const std::string& expected) {
   CHECK(expected == output);
 }
 
-TEST_CASE("JsonWriter::writeFloat()") {
+TEST_CASE("JsonWriter::writeFloat(double)") {
   SECTION("Pi") {
-    check(3.14159265359, "3.141592654");
+    check<double>(3.14159265359, "3.141592654");
   }
 
   SECTION("Signaling NaN") {
     double nan = std::numeric_limits<double>::signaling_NaN();
-    check(nan, "NaN");
+    check<double>(nan, "NaN");
   }
 
   SECTION("Quiet NaN") {
     double nan = std::numeric_limits<double>::quiet_NaN();
-    check(nan, "NaN");
+    check<double>(nan, "NaN");
   }
 
   SECTION("Infinity") {
     double inf = std::numeric_limits<double>::infinity();
-    check(inf, "Infinity");
-    check(-inf, "-Infinity");
+    check<double>(inf, "Infinity");
+    check<double>(-inf, "-Infinity");
   }
 
   SECTION("Zero") {
-    check(0.0, "0");
-    check(-0.0, "0");
+    check<double>(0.0, "0");
+    check<double>(-0.0, "0");
   }
 
   SECTION("Espilon") {
-    check(2.2250738585072014E-308, "2.225073859e-308");
-    check(-2.2250738585072014E-308, "-2.225073859e-308");
+    check<double>(2.2250738585072014E-308, "2.225073859e-308");
+    check<double>(-2.2250738585072014E-308, "-2.225073859e-308");
   }
 
   SECTION("Max double") {
-    check(1.7976931348623157E+308, "1.797693135e308");
-    check(-1.7976931348623157E+308, "-1.797693135e308");
+    check<double>(1.7976931348623157E+308, "1.797693135e308");
+    check<double>(-1.7976931348623157E+308, "-1.797693135e308");
   }
 
   SECTION("Big exponent") {
     // this test increases coverage of normalize()
-    check(1e255, "1e255");
-    check(1e-255, "1e-255");
+    check<double>(1e255, "1e255");
+    check<double>(1e-255, "1e-255");
   }
 
   SECTION("Exponentation when <= 1e-5") {
-    check(1e-4, "0.0001");
-    check(1e-5, "1e-5");
+    check<double>(1e-4, "0.0001");
+    check<double>(1e-5, "1e-5");
 
-    check(-1e-4, "-0.0001");
-    check(-1e-5, "-1e-5");
+    check<double>(-1e-4, "-0.0001");
+    check<double>(-1e-5, "-1e-5");
   }
 
   SECTION("Exponentation when >= 1e7") {
-    check(9999999.999, "9999999.999");
-    check(10000000, "1e7");
+    check<double>(9999999.999, "9999999.999");
+    check<double>(10000000.0, "1e7");
 
-    check(-9999999.999, "-9999999.999");
-    check(-10000000, "-1e7");
+    check<double>(-9999999.999, "-9999999.999");
+    check<double>(-10000000.0, "-1e7");
   }
 
   SECTION("Rounding when too many decimals") {
-    check(0.000099999999999, "0.0001");
-    check(0.0000099999999999, "1e-5");
-    check(0.9999999996, "1");
+    check<double>(0.000099999999999, "0.0001");
+    check<double>(0.0000099999999999, "1e-5");
+    check<double>(0.9999999996, "1");
   }
 
   SECTION("9 decimal places") {
-    check(0.100000001, "0.100000001");
-    check(0.999999999, "0.999999999");
+    check<double>(0.100000001, "0.100000001");
+    check<double>(0.999999999, "0.999999999");
 
-    check(9.000000001, "9.000000001");
-    check(9.999999999, "9.999999999");
+    check<double>(9.000000001, "9.000000001");
+    check<double>(9.999999999, "9.999999999");
   }
 
   SECTION("10 decimal places") {
-    check(0.1000000001, "0.1");
-    check(0.9999999999, "1");
+    check<double>(0.1000000001, "0.1");
+    check<double>(0.9999999999, "1");
 
-    check(9.0000000001, "9");
-    check(9.9999999999, "10");
+    check<double>(9.0000000001, "9");
+    check<double>(9.9999999999, "10");
+  }
+}
+
+TEST_CASE("JsonWriter::writeFloat(float)") {
+  SECTION("Pi") {
+    check<float>(3.14159265359f, "3.141593");
+  }
+
+  SECTION("999.9") {  // issue #543
+    check<float>(999.9f, "999.9");
   }
 }

+ 1 - 0
test/Misc/CMakeLists.txt

@@ -7,6 +7,7 @@
 
 add_executable(MiscTests 
 	deprecated.cpp
+	FloatParts.cpp
 	std_stream.cpp
 	std_string.cpp
 	StringBuilder.cpp

+ 47 - 0
test/Misc/FloatParts.cpp

@@ -0,0 +1,47 @@
+// Copyright Benoit Blanchon 2014-2017
+// MIT License
+//
+// Arduino JSON library
+// https://bblanchon.github.io/ArduinoJson/
+// If you like this project, please add a star!
+
+#include <ArduinoJson/Serialization/FloatParts.hpp>
+#include <catch.hpp>
+
+using namespace ArduinoJson::Internals;
+
+TEST_CASE("FloatParts<double>") {
+  SECTION("1.7976931348623157E+308") {
+    FloatParts<double> parts(1.7976931348623157E+308);
+    REQUIRE(parts.integral == 1);
+    REQUIRE(parts.decimal == 797693135);
+    REQUIRE(parts.decimalPlaces == 9);
+    REQUIRE(parts.exponent == 308);
+  }
+
+  SECTION("4.94065645841247e-324") {
+    FloatParts<double> parts(4.94065645841247e-324);
+    REQUIRE(parts.integral == 4);
+    REQUIRE(parts.decimal == 940656458);
+    REQUIRE(parts.decimalPlaces == 9);
+    REQUIRE(parts.exponent == -324);
+  }
+}
+
+TEST_CASE("FloatParts<float>") {
+  SECTION("3.4E+38") {
+    FloatParts<float> parts(3.4E+38f);
+    REQUIRE(parts.integral == 3);
+    REQUIRE(parts.decimal == 4);
+    REQUIRE(parts.decimalPlaces == 1);
+    REQUIRE(parts.exponent == 38);
+  }
+
+  SECTION("1.17549435e−38") {
+    FloatParts<float> parts(1.17549435e-38f);
+    REQUIRE(parts.integral == 1);
+    REQUIRE(parts.decimal == 175494);
+    REQUIRE(parts.decimalPlaces == 6);
+    REQUIRE(parts.exponent == -38);
+  }
+}

+ 0 - 1
test/Polyfills/CMakeLists.txt

@@ -8,7 +8,6 @@
 add_executable(PolyfillsTests 
 	isFloat.cpp
 	isInteger.cpp
-	normalize.cpp
 	parseFloat.cpp
 	parseInteger.cpp
 )

+ 0 - 43
test/Polyfills/normalize.cpp

@@ -1,43 +0,0 @@
-// Copyright Benoit Blanchon 2014-2017
-// MIT License
-//
-// Arduino JSON library
-// https://bblanchon.github.io/ArduinoJson/
-// If you like this project, please add a star!
-
-#include <ArduinoJson/Polyfills/normalize.hpp>
-#include <catch.hpp>
-
-using namespace ArduinoJson::Polyfills;
-
-TEST_CASE("normalize<double>()") {
-  SECTION("1.7976931348623157E+308") {
-    double value = 1.7976931348623157E+308;
-    int exp = normalize(value);
-    REQUIRE(value == Approx(1.7976931348623157));
-    REQUIRE(exp == 308);
-  }
-
-  SECTION("4.94065645841247e-324") {
-    double value = 4.94065645841247e-324;
-    int exp = normalize(value);
-    REQUIRE(value == Approx(4.94065645841247));
-    REQUIRE(exp == -324);
-  }
-}
-
-TEST_CASE("normalize<float>()") {
-  SECTION("3.4E+38") {
-    float value = 3.4E+38f;
-    int exp = normalize(value);
-    REQUIRE(value == Approx(3.4f));
-    REQUIRE(exp == 38);
-  }
-
-  SECTION("1.17549435e−38") {
-    float value = 1.17549435e-38f;
-    int exp = normalize(value);
-    REQUIRE(value == Approx(1.17549435));
-    REQUIRE(exp == -38);
-  }
-}