Cleaned up library structure (#13)

* Cleaned up library structure

* Fixed signal normalisation

* Corrected import and fixed scripts

extern/guitar_fx_lib/include/guitar_fx_lib/fx/clean.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <guitar_fx_lib/interfaces/effect.hpp>
+#include <guitar_fx_lib/fx/effect.hpp>
 
 class Clean : public Effect
 {

extern/guitar_fx_lib/include/guitar_fx_lib/fx/delay.hpp

@@ -2,7 +2,7 @@
 
 #include <array>
 
-#include <guitar_fx_lib/interfaces/effect.hpp>
+#include <guitar_fx_lib/fx/effect.hpp>
 
 class Delay : public Effect
 {

extern/guitar_fx_lib/include/guitar_fx_lib/fx/distortion.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <guitar_fx_lib/interfaces/effect.hpp>
+#include <guitar_fx_lib/fx/effect.hpp>
 
 class Distortion : public Effect
 {

extern/guitar_fx_lib/include/guitar_fx_lib/fx/echo.hpp

@@ -2,7 +2,7 @@
 
 #include <array>
 
-#include <guitar_fx_lib/interfaces/effect.hpp>
+#include <guitar_fx_lib/fx/effect.hpp>
 
 class Echo : public Effect
 {

extern/guitar_fx_lib/include/guitar_fx_lib/fx/effects.hpp

@@ -0,0 +1,13 @@
+#pragma once
+
+// List of all supported effects.
+// To save memory, use this enum to identify the effects and dynamically allocate
+// only the effect that is used
+enum class Effects
+{
+    CLEAN,
+    DISTORTION,
+    ECHO,
+    DELAY,
+    REVERB
+};

extern/guitar_fx_lib/include/guitar_fx_lib/fx_factory.hpp → extern/guitar_fx_lib/include/guitar_fx_lib/fx/fx_factory.hpp

@@ -6,46 +6,32 @@
 #include <guitar_fx_lib/fx/delay.hpp>
 #include <guitar_fx_lib/fx/distortion.hpp>
 #include <guitar_fx_lib/fx/echo.hpp>
+#include <guitar_fx_lib/fx/effect.hpp>
+#include <guitar_fx_lib/fx/effects.hpp>
 #include <guitar_fx_lib/fx/reverb.hpp>
-#include <guitar_fx_lib/interfaces/effect.hpp>
-
-enum class Effects
-{
-    CLEAN,
-    DISTORTION,
-    ECHO,
-    DELAY,
-    REVERB
-};
 
 class FxFactory
 {
   public:
     FxFactory() = default;
     ~FxFactory() = default;
 
-    std::unique_ptr<Effect> make(const Effects &fx)
+    EffectPtr make(const Effects &fx)
     {
         switch (fx)
         {
         case Effects::CLEAN:
             return std::make_unique<Clean>();
-            break;
         case Effects::DISTORTION:
             return std::make_unique<Distortion>();
-            break;
         case Effects::ECHO:
             return std::make_unique<Echo>();
-            break;
         case Effects::DELAY:
             return std::make_unique<Delay>();
-            break;
         case Effects::REVERB:
             return std::make_unique<Reverb>();
-
         default:
             return nullptr;
-            break;
         }
         return nullptr;
     }

extern/guitar_fx_lib/include/guitar_fx_lib/fx/reverb.hpp

@@ -1,7 +1,7 @@
 #pragma once
 
 #include <guitar_fx_lib/fx/delay.hpp>
-#include <guitar_fx_lib/interfaces/effect.hpp>
+#include <guitar_fx_lib/fx/effect.hpp>
 
 class Reverb : public Effect
 {

extern/guitar_fx_lib/include/guitar_fx_lib/fx_manager.hpp

@@ -6,21 +6,29 @@
 #include <string>
 #include <vector>
 
-#include <guitar_fx_lib/fx/clean.hpp>
-#include <guitar_fx_lib/fx/delay.hpp>
-#include <guitar_fx_lib/fx/distortion.hpp>
-#include <guitar_fx_lib/fx/echo.hpp>
-#include <guitar_fx_lib/fx_factory.hpp>
-#include <guitar_fx_lib/interfaces/effect.hpp>
+#include <guitar_fx_lib/fx/effect.hpp>
+#include <guitar_fx_lib/fx/effects.hpp>
+#include <guitar_fx_lib/fx/fx_factory.hpp>
 
 class FxManager
 {
   public:
     FxManager(const uint32_t &min, const uint32_t &max)
-        : signal_min(min), signal_max(max), signal_middle((max - min) / 2), factory()
+        : FxManager(min, max,
+                    {Effects::CLEAN, Effects::DISTORTION, Effects::ECHO, Effects::DELAY,
+                     Effects::REVERB})
     {
-        effects = {Effects::CLEAN, Effects::DISTORTION, Effects::ECHO, Effects::DELAY,
-                   Effects::REVERB};
+    }
+
+    FxManager(const uint32_t &min, const uint32_t &max,
+              const std::vector<Effects> &effects_list)
+        : signal_min(min), signal_max(max), factory(), effects(effects_list)
+    {
+        if (effects.empty())
+        {
+            // Ideally we would throw, but some platform don't support exceptions
+            effects.push_back(Effects::CLEAN);
+        }
         effects_it = effects.begin();
         active_fx = factory.make(*effects_it);
     }
@@ -74,11 +82,11 @@ class FxManager
         float normalised_output(0.0f);
         output = 0;
 
-        normalised_input = normalise_input(input);
+        normalised_input = normalise(input);
 
         if (active_fx->process(normalised_input, normalised_output))
         {
-            output = convert_output(normalised_output);
+            output = denormalise(normalised_output);
             return true;
         }
 
@@ -91,24 +99,38 @@ class FxManager
     }
 
   private:
-    float normalise_input(const uint32_t &input)
+    float normalise(const uint32_t &input)
     {
-        return (static_cast<float>(input) - signal_middle) / (signal_middle + 1.0f);
+        // Constraint the input before the conversion
+        uint32_t safe_input =
+            std::min<uint32_t>(std::max<uint32_t>(input, signal_min), signal_max);
+
+        return map(static_cast<float>(safe_input), static_cast<float>(signal_min),
+                   static_cast<float>(signal_max), norm_signal_min, norm_signal_max);
     }
 
-    uint32_t convert_output(const float &normalised_output)
+    uint32_t denormalise(const float &normalised_output)
     {
-        float output(0.0f);
-
         // Constraint the output before the conversion
-        output = std::min<float>(std::max<float>(normalised_output, -1.0f), 1.0f);
+        float safe_output = std::min<float>(
+            std::max<float>(normalised_output, norm_signal_min), norm_signal_max);
 
-        return output * ((signal_middle + 1) + signal_middle);
+        return static_cast<uint32_t>(map(safe_output, norm_signal_min, norm_signal_max,
+                                         static_cast<float>(signal_min),
+                                         static_cast<float>(signal_max)) +
+                                     0.5f); // add .5f to avoid rounding errors
+    }
+
+    float map(const float &x, const float &in_min, const float &in_max,
+              const float &out_min, const float &out_max)
+    {
+        return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
     }
 
-    uint32_t signal_max;
-    uint32_t signal_min;
-    uint32_t signal_middle;
+    const uint32_t signal_max;
+    const uint32_t signal_min;
+    const float norm_signal_max = 1.0f;
+    const float norm_signal_min = -1.0f;
 
     FxFactory factory;
     std::vector<Effects> effects;

extern/guitar_fx_lib/scripts/install.sh

@@ -2,8 +2,12 @@
 
 set -euo pipefail
 
+if [[ -d build ]]; then
+    rm -rf build
+fi
+
 # Build the automated test suite
 mkdir -p build
 cd build
 cmake -DCMAKE_INSTALL_PREFIX:PATH=/usr/local -DBUILD_TESTS=OFF ..
-cmake --build . --config Release --target install -- -j $(nproc)
+cmake --build . -j $(nproc) --config Release --target install --

extern/guitar_fx_lib/scripts/test.sh

@@ -2,15 +2,19 @@
 
 set -euo pipefail
 
+if [[ -d build ]]; then
+    rm -rf build
+fi
+
 # Build the automated test suite
 mkdir -p build
 cd build
 cmake -DCMAKE_INSTALL_PREFIX:PATH=/tmp/guitar_fx_lib -DBUILD_TESTS=ON ..
-cmake --build . --config Release -- -j $(nproc)
+cmake --build . -j $(nproc) --config Release --
 
 # Run the test
 export GTEST_COLOR=1
-cmake --build . --config Release --target run_tests -- -j $(nproc)
+cmake --build . --config Release -j $(nproc) --target run_tests --
 
 # Test install target
-cmake --build . --config Release --target install -- -j $(nproc)
+cmake --build . --config Release -j $(nproc) --target install --

extern/guitar_fx_lib/tests/test_fx_factory.cpp

@@ -7,7 +7,7 @@
 #include <guitar_fx_lib/fx/delay.hpp>
 #include <guitar_fx_lib/fx/distortion.hpp>
 #include <guitar_fx_lib/fx/echo.hpp>
-#include <guitar_fx_lib/fx_factory.hpp>
+#include <guitar_fx_lib/fx/fx_factory.hpp>
 
 class TestFxFactory
     : public ::testing::TestWithParam<std::tuple<Effects, std::shared_ptr<Effect>>>

extern/guitar_fx_lib/tests/test_fx_manager.cpp

@@ -1,5 +1,6 @@
 #include <gtest/gtest.h>
 
+#include <guitar_fx_lib/fx/effects.hpp>
 #include <guitar_fx_lib/fx_manager.hpp>
 
 class TestFxManager : public ::testing::Test
@@ -88,3 +89,17 @@ TEST_F(TestFxManager, testProcessSignal)
         }
     }
 }
+
+TEST_F(TestFxManager, testSignalInternalConversion)
+{
+    // Use only the CLEAN effect with all the possible inputs to verify that
+    // any internal signal manipulation does not alter the output
+    for (uint32_t input = 0; input < 4096; ++input)
+    {
+        FxManager fx(0, 4095, {Effects::CLEAN});
+
+        uint32_t output = 0;
+        ASSERT_TRUE(fx.process(input, output)) << "Failed with input " << input;
+        ASSERT_EQ(input, output) << "Failed with input " << input;
+    }
+}

scripts/build.sh

@@ -1,6 +1,12 @@
 #!/usr/bin/env bash
 
+set -euo pipefail
+
+if [[ -d build ]]; then
+    rm -rf build
+fi
+
 mkdir -p build
 cd build
 cmake ..
-cmake --build . --config Release -- -j $(nproc)
+cmake --build . -j $(nproc) --config Release --