updates after review

common/model-views.cpp

@@ -3173,7 +3173,7 @@ namespace rs2
             if (_reflectivity)
             {
                 _reflectivity->reset_history();
-                _stabilized_reflectivity.reset();
+                _stabilized_reflectivity.clear();
             }
         }
     }

common/utilities/number/stabilized-value.h

@@ -5,21 +5,23 @@
 
 #include <string>
 #include <atomic>
+#include <mutex>
 
 namespace utilities {
 namespace number {
 
-    // The stabilized value implement a history based stable value. 
-    // It supply a way to define and get values with a required stabilization.
-    // When the user adds a value, the value is inserted into the history
-    // When the user call "get" the last stable value is returned
-    // Inputs: Template <value_type>
-    //         required history size
-    //         required stabilization percentage
-    template<typename T> class stabilized_value
+// The stabilized value implement a history based stable value.
+// It supply a way to define and get values with a required stabilization.
+// When the user adds a value, the value is inserted into the history
+// When the user call "get" the last stable value is returned
+// Inputs: Template <value_type>
+//         required history size
+//         required stabilization percentage as a fraction , range [0-1]
+// The stabilized_value class is thread safe.
+template < typename T > class stabilized_value
 {
 public:
-    stabilized_value( size_t history_size, float stabilize_percentage )
+    stabilized_value( size_t history_size, float stabilize_percentage = 0.75f )
         : _history_size( history_size )
         , _last_stable_value( 0 )
         , _stabilize_percentage( stabilize_percentage )
@@ -29,27 +31,34 @@ namespace number {
                                       + std::to_string( stabilize_percentage ) );
     }
 
+    stabilized_value() = delete;
+    stabilized_value(const stabilized_value &) = delete;
+
+
     void add( T val )
     {
-        _recalc_stable_val = true;
-
+        const std::lock_guard< std::mutex > lock( _mutex );
         if( _values.empty() )
         {
             _values.push_back( val );
             _last_stable_value = val;
             return;
         }
 
+        _recalc_stable_val = true;
+
         _values.push_back( val );
         if( _values.size() > _history_size )
             _values.pop_front();
     }
 
-    T get()
+    T get() const
     {
+        std::lock_guard< std::mutex > lock( _mutex );
+
         if( _values.empty() )
         {
-            throw std::runtime_error( "No stable value exist, history is empty" );
+            throw std::runtime_error( "history is empty; no stable value" );
         }
 
         if( _recalc_stable_val )
@@ -58,10 +67,7 @@ namespace number {
             std::pair< T, int > most_stable_value = { 0, 0 };
             for( T val : _values )
             {
-                if( values_count_map.find( val ) != values_count_map.end() )
-                    values_count_map[val]++;
-                else
-                    values_count_map[val] = 1;
+                ++values_count_map[val];
 
                 if( most_stable_value.second < values_count_map[val] )
                 {
@@ -83,21 +89,29 @@ namespace number {
         return _last_stable_value;
     }
 
-    void reset()
+    void clear()
     {
+        const std::lock_guard< std::mutex > lock( _mutex );
+
         _values.clear();
         _last_stable_value = 0.0f;
         _recalc_stable_val = false;
     }
 
+    bool empty()
+    {
+        const std::lock_guard< std::mutex > lock( _mutex );
+        return _values.empty();
+    }
+
 private:
     std::deque< T > _values;
     const size_t _history_size;
-    T _last_stable_value;
+    mutable T _last_stable_value;
     const float _stabilize_percentage;
-    std::atomic_bool _recalc_stable_val = { true };
+    mutable std::atomic_bool _recalc_stable_val = { true };
+    mutable std::mutex _mutex;
 };
 
-
-}  // namespace value_stabilizer
+}  // namespace number
 }  // namespace utilities

unit-tests/utilities/number/stabilized/test-60-percent.cpp

@@ -5,7 +5,7 @@
 
 
 #include "../../../test.h"
-#include "../common/utilities/number/stabilized-value.h"
+#include <../common/utilities/number/stabilized-value.h>
 
 using namespace utilities::number;
 // Test group description:

unit-tests/utilities/number/stabilized/test-empty.cpp

@@ -0,0 +1,70 @@
+// License: Apache 2.0. See LICENSE file in root directory.
+// Copyright(c) 2020 Intel Corporation. All Rights Reserved.
+
+//#cmake:add-file ../../../../common/utilities/number/stabilized-value.h
+
+
+#include "../../../test.h"
+#include <../common/utilities/number/stabilized-value.h>
+
+using namespace utilities::number;
+
+// Test group description:
+//       * This tests group verifies stabilized_value class.
+//
+// Current test description:
+//       * Verify that empty function works as expected   
+TEST_CASE( "empty test", "[stabilized value]" )
+{
+    try
+    {
+        stabilized_value< float > stab_value(5, 1.0f);
+        CHECK(stab_value.empty());
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_FALSE(stab_value.empty());
+        stab_value.clear();
+        CHECK(stab_value.empty());
+    }
+    catch(const std::exception &e)
+    {
+        FAIL(" Exception caught: " << e.what());
+    }
+}
+
+TEST_CASE("stable value sanity - 40%", "[stabilized value]")
+{
+    try
+    {
+        stabilized_value< float > stab_value(5, 0.4f);
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+
+        CHECK(1.0f == stab_value.get());
+    }
+    catch (const std::exception &e)
+    {
+        FAIL("Exception caught: " << e.what());
+    }
+}
+
+TEST_CASE("stable value sanity - 25%", "[stabilized value]")
+{
+    try
+    {
+        stabilized_value< float > stab_value(5, 0.25f);
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+        CHECK_NOTHROW(stab_value.add(1.0f));
+
+        CHECK(1.0f == stab_value.get());
+    }
+    catch (const std::exception &e)
+    {
+        FAIL("Exception caught: " << e.what());
+    }
+}

unit-tests/utilities/number/stabilized/test-illegal-input.cpp

@@ -5,7 +5,7 @@
 
 
 #include "../../../test.h"
-#include "../common/utilities/number/stabilized-value.h"
+#include <../common/utilities/number/stabilized-value.h>
 
 using namespace utilities::number;
 

unit-tests/utilities/number/stabilized/test-multi-threading.cpp

@@ -0,0 +1,57 @@
+// License: Apache 2.0. See LICENSE file in root directory.
+// Copyright(c) 2020 Intel Corporation. All Rights Reserved.
+
+//#cmake:add-file ../../../../common/utilities/number/stabilized-value.h
+
+#include <thread>
+#include <chrono>
+#include "../../../test.h"
+#include <../common/utilities/number/stabilized-value.h>
+
+using namespace utilities::number;
+// Test group description:
+//       * This tests group verifies stabilized_value class.
+//
+// Current test description:
+//       * Verify that the stabilized_value class is thread safe
+
+TEST_CASE( "multi-threading", "[stabilized value]" )
+{
+    try
+    {
+        std::atomic<float> inserted_val_1 = { 20.0f };
+        std::atomic<float> inserted_val_2 = { 55.0f };
+        std::vector<float> values_vec;
+        stabilized_value< float > stab_value( 10, 0.6f );
+
+        // Fill history with values (> 60% is 'inserted_val_1')
+        std::thread first( [&]() {
+            std::this_thread::sleep_for(std::chrono::milliseconds(100));
+            for (int i = 0; i < 1000; i++)
+            {
+                if (i%3)
+                    stab_value.add(inserted_val_1);
+                else
+                    stab_value.add(inserted_val_2);
+            }
+        } );
+
+        // Sample stabilized value
+        std::thread second( [&]() {
+            while (stab_value.empty());
+            for (int i = 0; i < 1000; i++)
+                values_vec.push_back(stab_value.get());
+        } );
+
+        if (first.joinable()) first.join();
+        if (second.joinable()) second.join();
+
+        // Verify that all samples show the > 60% number inserted 'inserted_val_1'
+        for (auto val : values_vec)
+            CHECK(val == inserted_val_1);
+    }
+    catch( const std::exception & e )
+    {
+        FAIL( "Exception caught: " << e.what() );
+    }
+}

unit-tests/utilities/number/stabilized/test-not-full.cpp

@@ -0,0 +1,42 @@
+// License: Apache 2.0. See LICENSE file in root directory.
+// Copyright(c) 2020 Intel Corporation. All Rights Reserved.
+
+//#cmake:add-file ../../../../common/utilities/number/stabilized-value.h
+
+
+#include "../../../test.h"
+#include <../common/utilities/number/stabilized-value.h>
+
+using namespace utilities::number;
+
+// Test group description:
+//       * This tests group verifies stabilized_value class.
+//
+// Current test description:
+//       * Verify if history is not the logic works as expected and the percentage is calculated from the history current size
+TEST_CASE( "not full history", "[stabilized value]" )
+{
+    try
+    {
+        stabilized_value< float > stab_value(30, 0.6f);
+        CHECK_NOTHROW(stab_value.add(76.0f));
+        CHECK_NOTHROW(stab_value.add(76.0f));
+        CHECK_NOTHROW(stab_value.add(76.0f));
+        CHECK_NOTHROW(stab_value.add(76.0f));
+        CHECK(76.0f == stab_value.get());
+
+        CHECK_NOTHROW(stab_value.add(45.0f));
+        CHECK_NOTHROW(stab_value.add(45.0f));
+        CHECK_NOTHROW(stab_value.add(45.0f));
+        CHECK_NOTHROW(stab_value.add(45.0f));
+        CHECK(76.0f == stab_value.get());
+
+        CHECK_NOTHROW(stab_value.add(45.0f));
+        CHECK_NOTHROW(stab_value.add(45.0f)); // The stable value should change now (4 * 76.0 + 6 * 45.0 (total 10 values))
+        CHECK(45.0f == stab_value.get());
+    }
+    catch(const std::exception &e)
+    {
+        FAIL("Exception caught: " << e.what());
+    }
+}

unit-tests/utilities/number/stabilized/test-100-percente.cpp → unit-tests/utilities/number/stabilized/test-same-value.cpp

@@ -5,7 +5,7 @@
 
 
 #include "../../../test.h"
-#include "../common/utilities/number/stabilized-value.h"
+#include <../common/utilities/number/stabilized-value.h>
 
 using namespace utilities::number;
 
@@ -30,7 +30,7 @@ TEST_CASE( "stable value sanity - 100%", "[stabilized value]" )
     }
     catch(const std::exception &e)
     {
-        FAIL(" Exception caught: " << e.what());
+        FAIL("Exception caught: " << e.what());
     }
 }
 
@@ -49,7 +49,7 @@ TEST_CASE("stable value sanity - 40%", "[stabilized value]")
     }
     catch (const std::exception &e)
     {
-        FAIL(" Exception caught: " << e.what());
+        FAIL("Exception caught: " << e.what());
     }
 }
 
@@ -68,6 +68,6 @@ TEST_CASE("stable value sanity - 25%", "[stabilized value]")
     }
     catch (const std::exception &e)
     {
-        FAIL(" Exception caught: " << e.what());
+        FAIL("Exception caught: " << e.what());
     }
 }

unit-tests/utilities/number/stabilized/test-stay-last-value.cpp

@@ -5,7 +5,7 @@
 
 
 #include "../../../test.h"
-#include "../common/utilities/number/stabilized-value.h"
+#include <../common/utilities/number/stabilized-value.h>
 
 using namespace utilities::number;
 

unit-tests/utilities/number/stabilized/test-update-value.cpp

@@ -5,7 +5,7 @@
 
 
 #include "../../../test.h"
-#include "../common/utilities/number/stabilized-value.h"
+#include <../common/utilities/number/stabilized-value.h>
 
 using namespace utilities::number;