From d41f7a816593af4d182c05d445f1792cbf4a0421 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Fri, 14 Jul 2017 10:51:46 +0200 Subject: [PATCH] Fixed too many decimals places in float serialization (issue #543) --- CHANGELOG.md | 1 + src/ArduinoJson/Polyfills/normalize.hpp | 46 ---------- src/ArduinoJson/Serialization/FloatParts.hpp | 94 ++++++++++++++++++++ src/ArduinoJson/Serialization/JsonWriter.hpp | 63 +++---------- test/JsonWriter/writeFloat.cpp | 79 +++++++++------- test/Misc/CMakeLists.txt | 1 + test/Misc/FloatParts.cpp | 47 ++++++++++ test/Polyfills/CMakeLists.txt | 1 - test/Polyfills/normalize.cpp | 43 --------- 9 files changed, 201 insertions(+), 174 deletions(-) delete mode 100644 src/ArduinoJson/Polyfills/normalize.hpp create mode 100644 src/ArduinoJson/Serialization/FloatParts.hpp create mode 100644 test/Misc/FloatParts.cpp delete mode 100644 test/Polyfills/normalize.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 1737674a..1985e92d 100644 --- a/CHANGELOG.md +++ b/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 ------- diff --git a/src/ArduinoJson/Polyfills/normalize.hpp b/src/ArduinoJson/Polyfills/normalize.hpp deleted file mode 100644 index a3c25eb4..00000000 --- a/src/ArduinoJson/Polyfills/normalize.hpp +++ /dev/null @@ -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 -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::positiveBinaryPowerOfTen(index)) { - value *= FloatTraits::negativeBinaryPowerOfTen(index); - powersOf10 = int16_t(powersOf10 + bit); - } - bit >>= 1; - } - } - - if (value > 0 && value <= ARDUINOJSON_NEGATIVE_EXPONENTIATION_THRESHOLD) { - for (; index >= 0; index--) { - if (value < FloatTraits::negativeBinaryPowerOfTenPlusOne(index)) { - value *= FloatTraits::positiveBinaryPowerOfTen(index); - powersOf10 = int16_t(powersOf10 - bit); - } - bit >>= 1; - } - } - - return powersOf10; -} -} -} diff --git a/src/ArduinoJson/Serialization/FloatParts.hpp b/src/ArduinoJson/Serialization/FloatParts.hpp new file mode 100644 index 00000000..d7524774 --- /dev/null +++ b/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 +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 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; + } +}; +} +} diff --git a/src/ArduinoJson/Serialization/JsonWriter.hpp b/src/ArduinoJson/Serialization/JsonWriter.hpp index 560f95e8..2d54a2a0 100644 --- a/src/ArduinoJson/Serialization/JsonWriter.hpp +++ b/src/ArduinoJson/Serialization/JsonWriter.hpp @@ -9,12 +9,9 @@ #include #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 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 + 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 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 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; - } - } - } }; } } diff --git a/test/JsonWriter/writeFloat.cpp b/test/JsonWriter/writeFloat.cpp index e2d62f05..a5d5ad6c 100644 --- a/test/JsonWriter/writeFloat.cpp +++ b/test/JsonWriter/writeFloat.cpp @@ -14,7 +14,8 @@ using namespace ArduinoJson::Internals; -void check(double input, const std::string& expected) { +template +void check(TFloat input, const std::string& expected) { std::string output; DynamicStringBuilder sb(output); JsonWriter > 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(3.14159265359, "3.141592654"); } SECTION("Signaling NaN") { double nan = std::numeric_limits::signaling_NaN(); - check(nan, "NaN"); + check(nan, "NaN"); } SECTION("Quiet NaN") { double nan = std::numeric_limits::quiet_NaN(); - check(nan, "NaN"); + check(nan, "NaN"); } SECTION("Infinity") { double inf = std::numeric_limits::infinity(); - check(inf, "Infinity"); - check(-inf, "-Infinity"); + check(inf, "Infinity"); + check(-inf, "-Infinity"); } SECTION("Zero") { - check(0.0, "0"); - check(-0.0, "0"); + check(0.0, "0"); + check(-0.0, "0"); } SECTION("Espilon") { - check(2.2250738585072014E-308, "2.225073859e-308"); - check(-2.2250738585072014E-308, "-2.225073859e-308"); + check(2.2250738585072014E-308, "2.225073859e-308"); + check(-2.2250738585072014E-308, "-2.225073859e-308"); } SECTION("Max double") { - check(1.7976931348623157E+308, "1.797693135e308"); - check(-1.7976931348623157E+308, "-1.797693135e308"); + check(1.7976931348623157E+308, "1.797693135e308"); + check(-1.7976931348623157E+308, "-1.797693135e308"); } SECTION("Big exponent") { // this test increases coverage of normalize() - check(1e255, "1e255"); - check(1e-255, "1e-255"); + check(1e255, "1e255"); + check(1e-255, "1e-255"); } SECTION("Exponentation when <= 1e-5") { - check(1e-4, "0.0001"); - check(1e-5, "1e-5"); + check(1e-4, "0.0001"); + check(1e-5, "1e-5"); - check(-1e-4, "-0.0001"); - check(-1e-5, "-1e-5"); + check(-1e-4, "-0.0001"); + check(-1e-5, "-1e-5"); } SECTION("Exponentation when >= 1e7") { - check(9999999.999, "9999999.999"); - check(10000000, "1e7"); + check(9999999.999, "9999999.999"); + check(10000000.0, "1e7"); - check(-9999999.999, "-9999999.999"); - check(-10000000, "-1e7"); + check(-9999999.999, "-9999999.999"); + check(-10000000.0, "-1e7"); } SECTION("Rounding when too many decimals") { - check(0.000099999999999, "0.0001"); - check(0.0000099999999999, "1e-5"); - check(0.9999999996, "1"); + check(0.000099999999999, "0.0001"); + check(0.0000099999999999, "1e-5"); + check(0.9999999996, "1"); } SECTION("9 decimal places") { - check(0.100000001, "0.100000001"); - check(0.999999999, "0.999999999"); + check(0.100000001, "0.100000001"); + check(0.999999999, "0.999999999"); - check(9.000000001, "9.000000001"); - check(9.999999999, "9.999999999"); + check(9.000000001, "9.000000001"); + check(9.999999999, "9.999999999"); } SECTION("10 decimal places") { - check(0.1000000001, "0.1"); - check(0.9999999999, "1"); + check(0.1000000001, "0.1"); + check(0.9999999999, "1"); - check(9.0000000001, "9"); - check(9.9999999999, "10"); + check(9.0000000001, "9"); + check(9.9999999999, "10"); + } +} + +TEST_CASE("JsonWriter::writeFloat(float)") { + SECTION("Pi") { + check(3.14159265359f, "3.141593"); + } + + SECTION("999.9") { // issue #543 + check(999.9f, "999.9"); } } diff --git a/test/Misc/CMakeLists.txt b/test/Misc/CMakeLists.txt index df78266d..531df655 100644 --- a/test/Misc/CMakeLists.txt +++ b/test/Misc/CMakeLists.txt @@ -7,6 +7,7 @@ add_executable(MiscTests deprecated.cpp + FloatParts.cpp std_stream.cpp std_string.cpp StringBuilder.cpp diff --git a/test/Misc/FloatParts.cpp b/test/Misc/FloatParts.cpp new file mode 100644 index 00000000..eabe59f1 --- /dev/null +++ b/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 +#include + +using namespace ArduinoJson::Internals; + +TEST_CASE("FloatParts") { + SECTION("1.7976931348623157E+308") { + FloatParts 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 parts(4.94065645841247e-324); + REQUIRE(parts.integral == 4); + REQUIRE(parts.decimal == 940656458); + REQUIRE(parts.decimalPlaces == 9); + REQUIRE(parts.exponent == -324); + } +} + +TEST_CASE("FloatParts") { + SECTION("3.4E+38") { + FloatParts 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 parts(1.17549435e-38f); + REQUIRE(parts.integral == 1); + REQUIRE(parts.decimal == 175494); + REQUIRE(parts.decimalPlaces == 6); + REQUIRE(parts.exponent == -38); + } +} diff --git a/test/Polyfills/CMakeLists.txt b/test/Polyfills/CMakeLists.txt index 4800b105..bbc9165c 100644 --- a/test/Polyfills/CMakeLists.txt +++ b/test/Polyfills/CMakeLists.txt @@ -8,7 +8,6 @@ add_executable(PolyfillsTests isFloat.cpp isInteger.cpp - normalize.cpp parseFloat.cpp parseInteger.cpp ) diff --git a/test/Polyfills/normalize.cpp b/test/Polyfills/normalize.cpp deleted file mode 100644 index 2ebd55f8..00000000 --- a/test/Polyfills/normalize.cpp +++ /dev/null @@ -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 -#include - -using namespace ArduinoJson::Polyfills; - -TEST_CASE("normalize()") { - 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()") { - 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); - } -}