Fix rounding bug when formatting floats with minimal_printf

platform/source/minimal-printf/mbed_printf_implementation.c

@@ -267,20 +267,51 @@ static void mbed_minimal_formatted_string_double(char *buffer, size_t length, in
 {
     /* get integer part */
     MBED_SIGNED_STORAGE integer = value;
+    /* fractional part represented as the int that will be formatted after the dot, e.g. 95 for 1.95 */
+    MBED_SIGNED_STORAGE decimal = 0;
 
     if (dec_precision == PRECISION_DEFAULT) {
         dec_precision = MBED_CONF_PLATFORM_MINIMAL_PRINTF_SET_FLOATING_POINT_MAX_DECIMALS;
     }
 
     if (dec_precision != 0) {
+        /* get decimal part */
+        MBED_SIGNED_STORAGE precision = 1;
+        for (int index = 0; index < dec_precision; index++) {
+            precision *= 10;
+        }
+
+        /* Multiply the frac part so we get an int value with the required accuracy.
+           E.g. For 0.1234 and dec_precision=3 you'd get 123.4 */
+        double decimal_double = (value - integer) * precision;
+        if (value < 0) {
+            /* The part after the dot does not have a sign, so negate the value before rounding */
+            decimal = -decimal_double + 0.5;
+            if (decimal >= precision) {
+                /* Rounding carries over to value's integer part (e.g. -1.95 with dec_precision=1 -> -2.0) */
+                integer--;
+                decimal = 0;
+            }
+        } else {
+            /* Round the value */
+            decimal = decimal_double + 0.5;
+            if (decimal >= precision) {
+                /* Rounding carries over to value's integer part (e.g. 1.95 with dec_precision=1 -> 2.0) */
+                integer++;
+                decimal = 0;
+            }
+        }
+
         width_size -= dec_precision + 1; // decimal precision plus '.'
         if (width_size < 0) {
             width_size = 0;
         }
     } else {
         value = (value - integer) * 1.0;
-        if (!((value > -0.5) && (value < 0.5))) {
+        if (value > 0.5) {
             integer++;
+        } else if (value < -0.5) {
+            integer--;
         }
     }
 
@@ -294,30 +325,6 @@ static void mbed_minimal_formatted_string_double(char *buffer, size_t length, in
     if (dec_precision != 0) {
         /* write decimal point */
         mbed_minimal_putchar(buffer, length, result, '.', stream);
-
-        /* get decimal part */
-        double precision = 1.0;
-
-        for (size_t index = 0; index < dec_precision; index++) {
-            precision *= 10;
-        }
-
-        value = (value - integer) * precision;
-
-        /* convert to positive number */
-        if (value < 0.0) {
-            value *= -1.0;
-        }
-
-        MBED_UNSIGNED_STORAGE decimal = value;
-
-        /* round up or down */
-        value -= decimal;
-
-        if (!((value > -0.5) && (value < 0.5))) {
-            decimal++;
-        }
-
         /* write decimal part */
         mbed_minimal_formatted_string_integer(buffer, length, result, decimal, INT_UNSIGNED, dec_precision, true, stream);
     }

platform/tests/UNITTESTS/CMakeLists.txt

@@ -4,3 +4,4 @@
 add_subdirectory(doubles)
 add_subdirectory(ATCmdParser)
 add_subdirectory(CircularBuffer)
+add_subdirectory(minimal-printf)

platform/tests/UNITTESTS/minimal-printf/CMakeLists.txt

@@ -0,0 +1,27 @@
+# Copyright (c) 2021 ARM Limited. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+set(TEST_NAME minimal-printf-unittest)
+
+add_executable(${TEST_NAME})
+
+target_sources(${TEST_NAME}
+    PRIVATE
+        ${mbed-os_SOURCE_DIR}/platform/source/minimal-printf/mbed_printf_implementation.c
+        test_minimal-printf-implementation.cpp
+)
+
+target_include_directories(mbed-headers-platform
+    INTERFACE
+        ${mbed-os_SOURCE_DIR}/platform/source/minimal-printf
+)
+
+target_link_libraries(${TEST_NAME}
+    PRIVATE
+        mbed-stubs-platform
+        gmock_main
+)
+
+add_test(NAME "${TEST_NAME}" COMMAND ${TEST_NAME})
+
+set_tests_properties(${TEST_NAME} PROPERTIES LABELS "platform")

platform/tests/UNITTESTS/minimal-printf/test_minimal-printf-implementation.cpp

@@ -0,0 +1,144 @@
+/*
+ * Copyright (c) 2019, Arm Limited and affiliates.
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <cmath>
+#include <cstdarg>
+#include "gtest/gtest.h"
+
+/* "mbed_printf_implementation.h" does not declare it as extern "C", so pull it in manually */
+extern "C" int mbed_minimal_formatted_string(char *buffer, size_t length, const char *format, va_list arguments, FILE *stream);
+
+namespace
+{
+    std::string format(const char *fmt, ...)
+    {
+        va_list arguments;
+        va_start(arguments, fmt);
+
+        constexpr int buf_size = 128;
+        char buffer[buf_size];
+        int n = mbed_minimal_formatted_string(buffer, buf_size, fmt, arguments, nullptr);
+        va_end(arguments); 
+
+        if (n >= buf_size) {
+            return "overflow";
+        }
+
+        return std::string(buffer);
+    }
+}
+
+TEST(minimal_printf, floats)
+{
+    /* std::nextafter is used for the values below to make sure the numbers are 
+       floating point representations of at least the value we want to test. 
+       This hopefully mitigates false failures due to different floating point
+       implementations */
+
+    /* Positive numbers */
+    float value = std::nextafter(1.5f, 0.0f); /* Something like 1.499999, i.e just below 1.5 */
+    EXPECT_EQ("1", format("%.0f", value));
+    EXPECT_EQ("1.5", format("%.1f", value));
+    EXPECT_EQ("1.50", format("%.2f", value));
+    EXPECT_EQ("1.500", format("%.3f", value));
+    EXPECT_EQ("1.5000", format("%.4f", value));
+    EXPECT_EQ("1.50000", format("%.5f", value));
+    EXPECT_EQ("1.500000", format("%.6f", value));
+
+    value = std::nextafter(1.5f, 2.0f); /* Something like 1.500001, i.e just above 1.5 */
+    EXPECT_EQ("2", format("%.0f", value));
+    EXPECT_EQ("1.5", format("%.1f", value));
+    EXPECT_EQ("1.50", format("%.2f", value));
+    EXPECT_EQ("1.500", format("%.3f", value));
+    EXPECT_EQ("1.5000", format("%.4f", value));
+    EXPECT_EQ("1.50000", format("%.5f", value));
+    EXPECT_EQ("1.500000", format("%.6f", value));
+
+    value = std::nextafter(2.0f, 0.0f); /* Something like 1.999999, i.e just below 2.0 */
+    EXPECT_EQ("2", format("%.0f", value));
+    EXPECT_EQ("2.0", format("%.1f", value));
+    EXPECT_EQ("2.00", format("%.2f", value));
+    EXPECT_EQ("2.000", format("%.3f", value));
+    EXPECT_EQ("2.0000", format("%.4f", value));
+    EXPECT_EQ("2.00000", format("%.5f", value));
+    EXPECT_EQ("2.000000", format("%.6f", value));
+
+    /* Negative numbers */
+    value = std::nextafter(-1.5f, 0.0f); /* Something like -1.499999, i.e just above -1.5 */
+    EXPECT_EQ("-1", format("%.0f", value));
+    EXPECT_EQ("-1.5", format("%.1f", value));
+    EXPECT_EQ("-1.50", format("%.2f", value));
+    EXPECT_EQ("-1.500", format("%.3f", value));
+    EXPECT_EQ("-1.5000", format("%.4f", value));
+    EXPECT_EQ("-1.50000", format("%.5f", value));
+    EXPECT_EQ("-1.500000", format("%.6f", value));
+
+    value = std::nextafter(-1.5f, -2.0f); /* Something like -1.500001, i.e just below -1.5 */
+    EXPECT_EQ("-2", format("%.0f", value));
+    EXPECT_EQ("-1.5", format("%.1f", value));
+    EXPECT_EQ("-1.50", format("%.2f", value));
+    EXPECT_EQ("-1.500", format("%.3f", value));
+    EXPECT_EQ("-1.5000", format("%.4f", value));
+    EXPECT_EQ("-1.50000", format("%.5f", value));
+    EXPECT_EQ("-1.500000", format("%.6f", value));
+
+    value = std::nextafter(-2.0f, 0.0f);  /* Something like -1.999999, i.e just above -2.0 */
+    EXPECT_EQ("-2", format("%.0f", value));
+    EXPECT_EQ("-2.0", format("%.1f", value));
+    EXPECT_EQ("-2.00", format("%.2f", value));
+    EXPECT_EQ("-2.000", format("%.3f", value));
+    EXPECT_EQ("-2.0000", format("%.4f", value));
+    EXPECT_EQ("-2.00000", format("%.5f", value));
+    EXPECT_EQ("-2.000000", format("%.6f", value));
+
+    // And some near zero stuff
+    EXPECT_EQ("-0", format("%.0f", std::nextafter(0.0f, -1.0f)));
+    EXPECT_EQ("0", format("%.0f", 0.0));
+    EXPECT_EQ("0", format("%.0f", std::nextafter(0.0f, 1.0f)));
+}
+
+TEST(minimal_printf, padding)
+{
+    // Right aligned integers
+    EXPECT_EQ("12", format("%1d", 12));
+    EXPECT_EQ("12", format("%2d", 12));
+    EXPECT_EQ(" 12", format("%3d", 12));
+    EXPECT_EQ("  12", format("%4d", 12));
+
+    EXPECT_EQ("-12", format("%1d", -12));
+    EXPECT_EQ("-12", format("%3d", -12));
+    EXPECT_EQ(" -12", format("%4d", -12));
+    EXPECT_EQ("  -12", format("%5d", -12));
+
+    EXPECT_EQ("0012", format("%04d", 12));
+    EXPECT_EQ("-012", format("%04d", -12));
+
+    // Right aligned floats
+    EXPECT_EQ("12.3", format("%1.1f", 12.345f));
+    EXPECT_EQ("12.3", format("%4.1f", 12.345));
+    EXPECT_EQ(" 12.3", format("%5.1f", 12.345));
+    EXPECT_EQ("  12.3", format("%6.1f", 12.345));
+
+    EXPECT_EQ("-12.3", format("%1.1f", -12.345f));
+    EXPECT_EQ("-12.3", format("%4.1f", -12.345));
+    EXPECT_EQ(" -12.3", format("%6.1f", -12.345));
+    EXPECT_EQ("  -12.3", format("%7.1f", -12.345));
+
+    EXPECT_EQ("012.3", format("%05.1f", 12.345));
+    EXPECT_EQ("-012.3", format("%06.1f", -12.345));
+}
+