From 122241168f1e40047e00e5b00a4eb5811740cf38 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Sat, 21 Aug 2021 14:27:06 +0200 Subject: [PATCH] Fixed16 comparisons operators (#570) Updates to the Fixed16 class: - adds comparison operators between Fixed16 values. - removes implicit conversion to int. This caused confusing behavior as many operations were executed by the compiler on ints instead of Fixed16's. - Adds some comments. === * Add unit test to fixed16. * Adds comparison operators to Fixed16. Removes the implicit int16 conversion because it makes C++ believe weird. Co-authored-by: Stuart Baker --- src/utils/Fixed16.cxxtest | 130 ++++++++++++++++++++++++++++++++++---- src/utils/Fixed16.hxx | 57 +++++++++++++++-- 2 files changed, 170 insertions(+), 17 deletions(-) diff --git a/src/utils/Fixed16.cxxtest b/src/utils/Fixed16.cxxtest index 5dac6e5ca..c1750ddb1 100644 --- a/src/utils/Fixed16.cxxtest +++ b/src/utils/Fixed16.cxxtest @@ -40,19 +40,18 @@ using ::testing::FloatNear; TEST(Fixed16Test, CreateRound) { Fixed16 v1(13); - EXPECT_EQ(13, (uint16_t)v1); + EXPECT_EQ(13, v1.round()); v1 = Fixed16(13, 0x7fff); - EXPECT_EQ(13, (uint16_t)v1); + EXPECT_EQ(13, v1.round()); v1 = Fixed16(13, 0x8000); - EXPECT_EQ(14, (uint16_t)v1); - EXPECT_EQ(14, (int)v1); + EXPECT_EQ(14, v1.round()); v1 = Fixed16(13, 0xff00); - EXPECT_EQ(14, (uint16_t)v1); + EXPECT_EQ(14, v1.round()); v1 = Fixed16(13, 0xffff); - EXPECT_EQ(14, (uint16_t)v1); + EXPECT_EQ(14, v1.round()); } TEST(Fixed16Test, ToFloat) @@ -124,20 +123,20 @@ TEST(Fixed16Test, Constexpr) TEST(Fixed16Test, Arithmetics) { Fixed16 v1(13); - EXPECT_EQ(13, (uint16_t)v1); + EXPECT_EQ(13, v1.round()); v1 += 4; - EXPECT_EQ(17, (uint16_t)v1); + EXPECT_EQ(17, v1.round()); v1 -= 2; - EXPECT_EQ(15, (uint16_t)v1); + EXPECT_EQ(15, v1.round()); v1 += Fixed16(0, 0x8000); - EXPECT_EQ(16, (uint16_t)v1); + EXPECT_EQ(16, v1.round()); EXPECT_EQ(15, v1.trunc()); v1 *= 2; - EXPECT_EQ(31, (uint16_t)v1); + EXPECT_EQ(31, v1.round()); EXPECT_EQ(31, v1.trunc()); EXPECT_EQ(0, v1.frac()); @@ -149,7 +148,7 @@ TEST(Fixed16Test, Arithmetics) Fixed16 v2 = 1; v2 /= 2; v1 += v2; - EXPECT_EQ(16, (uint16_t)v1); + EXPECT_EQ(16, v1.round()); EXPECT_THAT(v1.to_float(), FloatNear(16.0, 1e-5)); v1 += Fixed16(1) / 2; @@ -157,6 +156,44 @@ TEST(Fixed16Test, Arithmetics) EXPECT_THAT(v1.to_float(), FloatNear(16.5, 1e-5)); } +TEST(Fixed16Test, ArithmeticsNegative) +{ + Fixed16 v1(13); + v1 += -2; + EXPECT_EQ(11, v1.round()); + + v1 = -15; + + v1 += -2; + EXPECT_EQ(-17, v1.round()); + + v1 -= -2; + EXPECT_EQ(-15, v1.round()); + + v1 += 2; + EXPECT_EQ(-13, v1.round()); + + v1 *= 2; + EXPECT_EQ(-26, v1.round()); +} + +TEST(Fixed16Test, TruncNegative) +{ + Fixed16 v1(Fixed16::FROM_DOUBLE, -7.5); + EXPECT_EQ(-7, v1.trunc()); + EXPECT_EQ(0x8000, v1.frac()); + + // Note that the exact half is rounded away from zero. + EXPECT_EQ(-8, v1.round()); + + v1 = {Fixed16::FROM_DOUBLE, 7.5}; + EXPECT_EQ(7, v1.trunc()); + EXPECT_EQ(0x8000, v1.frac()); + + // Note that the exact half is rounded away from zero. + EXPECT_EQ(8, v1.round()); +} + TEST(Fixed16Test, Division) { Fixed16 v1(256); @@ -214,6 +251,64 @@ TEST(Fixed16Test, SignedZero) EXPECT_TRUE(v1.is_positive()); } +TEST(Fixed16Test, Compare) +{ + // This array is sorted. + Fixed16 arr[] = { + {-32767, 0xffff}, + {-32767, 0x8000}, + {-32767, 1}, + {-32767, 0}, + {-32766, 0xffff}, + {-32766, 0x8000}, + {-32766, 1}, + {-32766, 0}, + {-1, 0xffff}, + {-1, 0x8000}, + {-1, 1}, + {-1, 0}, + {0, 0}, + {0, 1}, + {0, 0x8000}, + {0, 0xffff}, + {1, 0}, + {1, 1}, + {1, 0x8000}, + {1, 0xffff}, + {32767, 0}, + {32767, 1}, + {32767, 0x8000}, + {32767, 0xffff} + }; + + for (unsigned i = 0; i < ARRAYSIZE(arr); i++) { + for (unsigned j = 0; j < i; j++) { + string s = StringPrintf("i [%d] %d:%d k=%08x j [%d] %d:%d k=%08x", + i, arr[i].trunc(), arr[i].frac(), arr[i].to_key(), j, + arr[j].trunc(), arr[j].frac(), arr[j].to_key()); + SCOPED_TRACE(s); + EXPECT_TRUE(arr[j] < arr[i]); + EXPECT_TRUE(arr[j] <= arr[i]); + EXPECT_FALSE(arr[j] > arr[i]); + EXPECT_FALSE(arr[j] >= arr[i]); + EXPECT_FALSE(arr[i] < arr[j]); + EXPECT_FALSE(arr[i] <= arr[j]); + EXPECT_TRUE(arr[i] > arr[j]); + EXPECT_TRUE(arr[i] >= arr[j]); + + EXPECT_TRUE(arr[i] != arr[j]); + EXPECT_FALSE(arr[i] == arr[j]); + } + EXPECT_FALSE(arr[i] < arr[i]); + EXPECT_FALSE(arr[i] > arr[i]); + EXPECT_TRUE(arr[i] <= arr[i]); + EXPECT_TRUE(arr[i] >= arr[i]); + + EXPECT_TRUE(arr[i] == arr[i]); + EXPECT_FALSE(arr[i] != arr[i]); + } +} + /// Helper function to test mulpow2 operation. Computes base mulpow2 shift with /// fixed16 and with double, and verifies that the results is within the given /// relative precision form each other. @@ -249,3 +344,14 @@ TEST(Fixed16Test, MulPow2) mulpow2_test(30481, -12.5, 0.00001); mulpow2_test(4377, 2.375, 0.0001); } + +TEST(Fixed16Test, IntegerDivision) +{ + Fixed16 ohmsMin{505}; + Fixed16 ohmsMax{9495}; + Fixed16 ohmsMid{ohmsMin + ((ohmsMax - ohmsMin) / 2)}; + Fixed16 stepSizeLow{(ohmsMid - ohmsMin) / 127}; + Fixed16 stepSizeHigh{(ohmsMax - ohmsMid) / 127}; + + EXPECT_NEAR(35.39, stepSizeLow.to_float(), 0.01); +} diff --git a/src/utils/Fixed16.hxx b/src/utils/Fixed16.hxx index 0dbeab734..76c79f6cd 100644 --- a/src/utils/Fixed16.hxx +++ b/src/utils/Fixed16.hxx @@ -42,6 +42,12 @@ class Fixed16 { public: + /// Constructs a Fixed16. + /// @param integer is the integer part and the sign. Valid values are from + /// -32767 to 32767. + /// @param frac is the fractional part. All uint16 values are valid. For + /// positive integer the fractional part goes above the int value, for + /// negative integers the fractional part goes below the int value. constexpr Fixed16(int16_t integer, uint16_t frac = 0) : value_(((integer < 0 ? -integer : integer) << 16) | frac) , sign_(integer < 0 ? 1 : 0) @@ -53,6 +59,9 @@ public: FROM_DOUBLE }; + /// Constructs a Fixed16. + /// @param value is the value to store. Valid values are -32767.99999 to + /// 32767.99999. constexpr Fixed16(FromDouble, double value) : value_(value < 0 ? -value * 65536 + 0.5 : value * 65536 + 0.5) , sign_(value < 0 ? 1 : 0) @@ -126,12 +135,42 @@ public: return ret; } - /// @return the rounded value to the nearest integer - operator uint16_t() const + /// Comparison operator. + bool operator<(Fixed16 o) { - return round(); + return to_key() < o.to_key(); } + /// Comparison operator. + bool operator<=(Fixed16 o) + { + return to_key() <= o.to_key(); + } + + /// Comparison operator. + bool operator>(Fixed16 o) + { + return to_key() > o.to_key(); + } + + /// Comparison operator. + bool operator>=(Fixed16 o) + { + return to_key() >= o.to_key(); + } + + /// Comparison operator. + bool operator==(Fixed16 o) + { + return to_key() == o.to_key(); + } + + /// Comparison operator. + bool operator!=(Fixed16 o) + { + return to_key() != o.to_key(); + } + /// Multiplies *this with pow(2, o). This is effectively a generalized /// shift operation that works on fractional numbers too. The precision is /// limited. @@ -189,7 +228,7 @@ public: return b; } - /// @return the integer part, rounded down + /// @return the integer part, rounded towards zero. int16_t trunc() const { int16_t b = value_ >> 16; @@ -198,6 +237,8 @@ public: } /// @return the fractional part, as an uint16 value between 0 and 0xffff + /// Note: the fractional part inherits the sign of the integer part, + /// similarly to the decimal notation. uint16_t frac() const { return value_ & 0xffff; @@ -245,6 +286,12 @@ public: void negate() { sign_ ^= 1; } + + /// Turns the value into a comparison key. + int32_t to_key() + { + return to_int(); + } private: /// Translates the current value to a signed fixed-point 32-bit integer. @@ -271,7 +318,7 @@ private: } value_ = v & 0x7fffffffu; } - + uint32_t value_ : 31; uint32_t sign_ : 1; };