feat!: generic pairing/bonding classes updated (philips-software#330)

* Generic pairing/bonding classes updated * All functions on GapPairingObserver converted to pure virtual * unit tests for gap pairing implemented * unit tests for gap bonding implemented * two code smells fixed * missing field for pairing error added
gabrielfrasantos · Jul 21, 2023 · dbda405 · dbda405
1 parent a3f3cb5
commit dbda405
Show file tree

Hide file tree

Showing 15 changed files with 364 additions and 150 deletions.
diff --git a/services/ble/Gap.cpp b/services/ble/Gap.cpp
@@ -2,6 +2,88 @@
 
 namespace services
 {
+    void GapPairingDecorator::DisplayPasskey(int32_t passkey, bool numericComparison)
+    {
+        GapPairing::NotifyObservers([&passkey, &numericComparison](auto& obs)
+            {
+                obs.DisplayPasskey(passkey, numericComparison);
+            });
+    }
+
+    void GapPairingDecorator::PairingSuccessfullyCompleted()
+    {
+        GapPairing::NotifyObservers([](auto& obs)
+            {
+                obs.PairingSuccessfullyCompleted();
+            });
+    }
+
+    void GapPairingDecorator::PairingFailed(PairingErrorType error)
+    {
+        GapPairing::NotifyObservers([&error](auto& obs)
+            {
+                obs.PairingFailed(error);
+            });
+    }
+
+    void GapPairingDecorator::Pair()
+    {
+        GapPairingObserver::Subject().Pair();
+    }
+
+    void GapPairingDecorator::AllowPairing(bool allow)
+    {
+        GapPairingObserver::Subject().AllowPairing(allow);
+    }
+
+    void GapPairingDecorator::SetSecurityMode(SecurityMode mode, SecurityLevel level)
+    {
+        GapPairingObserver::Subject().SetSecurityMode(mode, level);
+    }
+
+    void GapPairingDecorator::SetIoCapabilities(IoCapabilities caps)
+    {
+        GapPairingObserver::Subject().SetIoCapabilities(caps);
+    }
+
+    void GapPairingDecorator::AuthenticateWithPasskey(uint32_t passkey)
+    {
+        GapPairingObserver::Subject().AuthenticateWithPasskey(passkey);
+    }
+
+    void GapPairingDecorator::NumericComparisonConfirm(bool accept)
+    {
+        GapPairingObserver::Subject().NumericComparisonConfirm(accept);
+    }
+
+    void GapBondingDecorator::NumberOfBondsChanged(std::size_t nrBonds)
+    {
+        GapBonding::NotifyObservers([&nrBonds](auto& obs)
+            {
+                obs.NumberOfBondsChanged(nrBonds);
+            });
+    }
+
+    void GapBondingDecorator::RemoveAllBonds()
+    {
+        GapBondingObserver::Subject().RemoveAllBonds();
+    }
+
+    void GapBondingDecorator::RemoveOldestBond()
+    {
+        GapBondingObserver::Subject().RemoveOldestBond();
+    }
+
+    std::size_t GapBondingDecorator::GetMaxNumberOfBonds() const
+    {
+        return GapBondingObserver::Subject().GetMaxNumberOfBonds();
+    }
+
+    std::size_t GapBondingDecorator::GetNumberOfBonds() const
+    {
+        return GapBondingObserver::Subject().GetNumberOfBonds();
+    }
+
     void GapPeripheralDecorator::StateChanged(GapState state)
     {
         GapPeripheral::NotifyObservers([&state](auto& obs)
@@ -50,22 +132,6 @@ namespace services
         GapPeripheralObserver::Subject().Standby();
     }
 
-    void GapCentralDecorator::AuthenticationSuccessfullyCompleted()
-    {
-        GapCentralObserver::SubjectType::NotifyObservers([](auto& obs)
-            {
-                obs.AuthenticationSuccessfullyCompleted();
-            });
-    }
-
-    void GapCentralDecorator::AuthenticationFailed(GapAuthenticationErrorType status)
-    {
-        GapCentralObserver::SubjectType::NotifyObservers([&status](auto& obs)
-            {
-                obs.AuthenticationFailed(status);
-            });
-    }
-
     void GapCentralDecorator::DeviceDiscovered(const GapAdvertisingReport& deviceDiscovered)
     {
         GapCentralObserver::SubjectType::NotifyObservers([&deviceDiscovered](auto& obs)

diff --git a/services/ble/Gap.hpp b/services/ble/Gap.hpp
@@ -19,15 +19,6 @@ namespace services
         advNonconnInd
     };
 
-    enum class GapIoCapabilities : uint8_t
-    {
-        display,
-        displayYesNo,
-        keyboard,
-        none,
-        keyboardDisplay
-    };
-
     enum class GapState : uint8_t
     {
         standby,
@@ -36,17 +27,6 @@ namespace services
         connected
     };
 
-    enum class GapAuthenticationErrorType : uint8_t
-    {
-        passkeyEntryFailed,
-        authenticationRequirementsNotMet,
-        pairingNotSupported,
-        insufficientEncryptionKeySize,
-        numericComparisonFailed,
-        timeout,
-        unknown,
-    };
-
     enum class GapAdvertisingEventType : uint8_t
     {
         advInd,
@@ -95,20 +75,6 @@ namespace services
         int8_t rssi;
     };
 
-    enum class GapSecurityMode : uint8_t
-    {
-        mode1,
-        mode2
-    };
-
-    enum class GapSecurityLevel : uint8_t
-    {
-        level1,
-        level2,
-        level3,
-        level4,
-    };
-
     struct GapAddress
     {
         hal::MacAddress address;
@@ -135,50 +101,127 @@ namespace services
         infra::ConstByteRange ParserAdvertisingData(GapAdvertisementDataType type) const;
     };
 
-    class GapPeripheralPairing;
+    class GapPairing;
 
-    class GapPeripheralPairingObserver
-        : public infra::Observer<GapPeripheralPairingObserver, GapPeripheralPairing>
+    class GapPairingObserver
+        : public infra::Observer<GapPairingObserver, GapPairing>
     {
     public:
-        using infra::Observer<GapPeripheralPairingObserver, GapPeripheralPairing>::Observer;
+        using infra::Observer<GapPairingObserver, GapPairing>::Observer;
+
+        enum class PairingErrorType : uint8_t
+        {
+            passkeyEntryFailed,
+            authenticationRequirementsNotMet,
+            pairingNotSupported,
+            insufficientEncryptionKeySize,
+            numericComparisonFailed,
+            timeout,
+            encryptionFailed,
+            unknown,
+        };
 
         virtual void DisplayPasskey(int32_t passkey, bool numericComparison) = 0;
+        virtual void PairingSuccessfullyCompleted() = 0;
+        virtual void PairingFailed(PairingErrorType error) = 0;
     };
 
-    class GapPeripheralPairing
-        : public infra::Subject<GapPeripheralPairingObserver>
+    class GapPairing
+        : public infra::Subject<GapPairingObserver>
     {
     public:
+        enum class IoCapabilities : uint8_t
+        {
+            display,
+            displayYesNo,
+            keyboard,
+            none,
+            keyboardDisplay
+        };
+
+        enum class SecurityMode : uint8_t
+        {
+            mode1,
+            mode2
+        };
+
+        enum class SecurityLevel : uint8_t
+        {
+            level1,
+            level2,
+            level3,
+            level4,
+        };
+
+        virtual void Pair() = 0;
+
         virtual void AllowPairing(bool allow) = 0;
 
-        virtual void SetSecurityMode(GapSecurityMode mode, GapSecurityLevel level) = 0;
-        virtual void SetIoCapabilities(GapIoCapabilities caps) = 0;
+        virtual void SetSecurityMode(SecurityMode mode, SecurityLevel level) = 0;
+        virtual void SetIoCapabilities(IoCapabilities caps) = 0;
 
         virtual void AuthenticateWithPasskey(uint32_t passkey) = 0;
         virtual void NumericComparisonConfirm(bool accept) = 0;
     };
 
-    class GapPeripheralBonding;
+    class GapPairingDecorator
+        : public GapPairingObserver
+        , public GapPairing
+    {
+    public:
+        using GapPairingObserver::GapPairingObserver;
+
+        // Implementation of GapPairingObserver
+        void DisplayPasskey(int32_t passkey, bool numericComparison) override;
+        void PairingSuccessfullyCompleted() override;
+        void PairingFailed(PairingErrorType error) override;
+
+        // Implementation of GapPairing
+        void Pair() override;
+        void AllowPairing(bool allow) override;
+        void SetSecurityMode(SecurityMode mode, SecurityLevel level) override;
+        void SetIoCapabilities(IoCapabilities caps) override;
+        void AuthenticateWithPasskey(uint32_t passkey) override;
+        void NumericComparisonConfirm(bool accept) override;
+    };
+
+    class GapBonding;
 
-    class GapPeripheralBondingObserver
-        : public infra::Observer<GapPeripheralBondingObserver, GapPeripheralBonding>
+    class GapBondingObserver
+        : public infra::Observer<GapBondingObserver, GapBonding>
     {
     public:
-        using infra::Observer<GapPeripheralBondingObserver, GapPeripheralBonding>::Observer;
+        using infra::Observer<GapBondingObserver, GapBonding>::Observer;
 
-        virtual void NumberOfBondsChanged(size_t nrBonds) = 0;
+        virtual void NumberOfBondsChanged(std::size_t nrBonds) = 0;
     };
 
-    class GapPeripheralBonding
-        : public infra::Subject<GapPeripheralBondingObserver>
+    class GapBonding
+        : public infra::Subject<GapBondingObserver>
     {
     public:
         virtual void RemoveAllBonds() = 0;
         virtual void RemoveOldestBond() = 0;
 
-        virtual size_t GetMaxNumberOfBonds() const = 0;
-        virtual size_t GetNumberOfBonds() const = 0;
+        virtual std::size_t GetMaxNumberOfBonds() const = 0;
+        virtual std::size_t GetNumberOfBonds() const = 0;
+    };
+
+    class GapBondingDecorator
+        : public GapBondingObserver
+        , public GapBonding
+    {
+    public:
+        using GapBondingObserver::GapBondingObserver;
+
+        // Implementation of GapBondingObserver
+        void NumberOfBondsChanged(std::size_t nrBonds) override;
+
+        // Implementation of GapBonding
+        void RemoveAllBonds() override;
+        void RemoveOldestBond() override;
+        std::size_t GetMaxNumberOfBonds() const override;
+        std::size_t GetNumberOfBonds() const override;
     };
 
     class GapPeripheral;
@@ -258,8 +301,6 @@ namespace services
     public:
         using infra::Observer<GapCentralObserver, GapCentral>::Observer;
 
-        virtual void AuthenticationSuccessfullyCompleted() = 0;
-        virtual void AuthenticationFailed(GapAuthenticationErrorType error) = 0;
         virtual void DeviceDiscovered(const GapAdvertisingReport& deviceDiscovered) = 0;
         virtual void StateChanged(GapState state) = 0;
     };
@@ -283,8 +324,6 @@ namespace services
         using GapCentralObserver::GapCentralObserver;
 
         // Implementation of GapCentralObserver
-        void AuthenticationSuccessfullyCompleted() override;
-        void AuthenticationFailed(GapAuthenticationErrorType error) override;
         void DeviceDiscovered(const GapAdvertisingReport& deviceDiscovered) override;
         void StateChanged(GapState state) override;
 

diff --git a/services/ble/test/CMakeLists.txt b/services/ble/test/CMakeLists.txt
@@ -11,7 +11,9 @@ target_link_libraries(services.ble_test PUBLIC
 target_sources(services.ble_test PRIVATE
     TestBondBlobPersistence.cpp
     TestBondStorageSynchronizer.cpp
+    TestGapBonding.cpp
     TestGapCentral.cpp
+    TestGapPairing.cpp
     TestGapPeripheral.cpp
     TestGatt.cpp
     TestGattClient.cpp

diff --git a/services/ble/test/TestGapBonding.cpp b/services/ble/test/TestGapBonding.cpp
@@ -0,0 +1,45 @@
+#include "infra/util/test_helper/MemoryRangeMatcher.hpp"
+#include "services/ble/Gap.hpp"
+#include "services/ble/test_doubles/GapBondingMock.hpp"
+#include "services/ble/test_doubles/GapBondingObserverMock.hpp"
+#include "gmock/gmock.h"
+
+namespace services
+{
+    namespace
+    {
+        class GapBondingDecoratorTest
+            : public testing::Test
+        {
+        public:
+            GapBondingMock gapBonding;
+            GapBondingDecorator decorator{ gapBonding };
+            GapBondingObserverMock gapBondingObserver{ decorator };
+        };
+    }
+
+    TEST_F(GapBondingDecoratorTest, forward_all_events_to_observers)
+    {
+        EXPECT_CALL(gapBondingObserver, NumberOfBondsChanged(::testing::Eq(10)));
+
+        gapBonding.NotifyObservers([](GapBondingObserver& obs)
+            {
+                obs.NumberOfBondsChanged(10);
+            });
+    }
+
+    TEST_F(GapBondingDecoratorTest, forward_all_calls_to_subject)
+    {
+        EXPECT_CALL(gapBonding, RemoveAllBonds());
+        decorator.RemoveAllBonds();
+
+        EXPECT_CALL(gapBonding, RemoveOldestBond());
+        decorator.RemoveOldestBond();
+
+        EXPECT_CALL(gapBonding, GetMaxNumberOfBonds()).WillOnce(testing::Return(5));
+        EXPECT_EQ(decorator.GetMaxNumberOfBonds(), 5);
+
+        EXPECT_CALL(gapBonding, GetNumberOfBonds()).WillOnce(testing::Return(5));
+        EXPECT_EQ(decorator.GetNumberOfBonds(), 5);
+    }
+}