fix: ble central requires a timeout when tries to start a connection (#…

…420) * add a timeout in order to change from state initiating to standby in case of failure * fix transition between states * add initiating timeout to function connect * update amp-embedded-infra-lib hash * make sure initiating timeout is inside the proper interval * add cancel connection function * update embedded-infra hash * Update CMakeLists.txt * Update CMakeLists.txt * process review comments * fix comments * Update CMakeLists.txt * fix cmakepreset according to changes on embedded-infra * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update amp-embedded-infra-lib --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Richard Peters <[email protected]>
philips-software · Oct 28, 2024 · bb209a3 · bb209a3
1 parent a1d4115
commit bb209a3
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 32 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -19,7 +19,7 @@ if (HALST_STANDALONE)
     FetchContent_Declare(
         emil
         GIT_REPOSITORY https://github.com/philips-software/amp-embedded-infra-lib.git
-        GIT_TAG        ca31705a21937ceb07a654d8828eb8f982f468e1 # unreleased
+        GIT_TAG        28ef5f86cf50a4b0d00207d7484f2332d1f7c024 # unreleased
     )
     FetchContent_MakeAvailable(emil)
 

diff --git a/CMakePresets.json b/CMakePresets.json
@@ -17,7 +17,8 @@
       "inherits": "defaults",
       "cacheVariables": {
         "CMAKE_CONFIGURATION_TYPES": "Debug;Release;RelWithDebInfo;MinSizeRel",
-        "EMIL_BUILD_ECHO_COMPILERS": true
+        "EMIL_BUILD_ECHO_COMPILERS": true,
+        "EMIL_FETCH_ECHO_COMPILERS": false
       },
       "generator": "Ninja Multi-Config"
     },

diff --git a/hal_st/middlewares/ble_middleware/GapCentralSt.cpp b/hal_st/middlewares/ble_middleware/GapCentralSt.cpp
@@ -1,6 +1,8 @@
 #include "hal_st/middlewares/ble_middleware/GapCentralSt.hpp"
 #include "ble_defs.h"
 #include "infra/event/EventDispatcherWithWeakPtr.hpp"
+#include <chrono>
+#include <cmath>
 
 namespace hal
 {
@@ -50,7 +52,7 @@ namespace hal
             });
     }
 
-    void GapCentralSt::Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType)
+    void GapCentralSt::Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType, infra::Duration initiatingTimeout)
     {
         auto peerAddress = addressType == services::GapDeviceAddressType::publicAddress ? GAP_PUBLIC_ADDR : GAP_STATIC_RANDOM_ADDR;
 
@@ -59,6 +61,25 @@ namespace hal
             connectionUpdateParameters.minConnIntMultiplier, connectionUpdateParameters.maxConnIntMultiplier,
             connectionUpdateParameters.slaveLatency, connectionUpdateParameters.supervisorTimeoutMs,
             minConnectionEventLength, maxConnectionEventLength);
+
+        infra::Subject<services::GapCentralObserver>::NotifyObservers([](auto& observer)
+            {
+                observer.StateChanged(services::GapState::initiating);
+            });
+
+        initiatingStateTimer.Start(initiatingTimeout, [this]()
+            {
+                aci_gap_terminate_gap_proc(GAP_DIRECT_CONNECTION_ESTABLISHMENT_PROC);
+            });
+    }
+
+    void GapCentralSt::CancelConnect()
+    {
+        if (initiatingStateTimer.Armed())
+        {
+            initiatingStateTimer.Cancel();
+            aci_gap_terminate_gap_proc(GAP_DIRECT_CONNECTION_ESTABLISHMENT_PROC);
+        }
     }
 
     void GapCentralSt::Disconnect()
@@ -73,10 +94,9 @@ namespace hal
 
     void GapCentralSt::StartDeviceDiscovery()
     {
-        if (!discovering)
+        if (!std::exchange(discovering, true))
         {
             aci_gap_start_general_discovery_proc(leScanInterval, leScanWindow, GAP_RESOLVABLE_PRIVATE_ADDR, filterDuplicatesEnabled);
-            discovering = true;
             infra::Subject<services::GapCentralObserver>::NotifyObservers([](auto& observer)
                 {
                     observer.StateChanged(services::GapState::scanning);
@@ -86,16 +106,12 @@ namespace hal
 
     void GapCentralSt::StopDeviceDiscovery()
     {
-        if (discovering)
-        {
+        if (std::exchange(discovering, false))
             aci_gap_terminate_gap_proc(GAP_GENERAL_DISCOVERY_PROC);
-            discovering = false;
-        }
     }
 
     void GapCentralSt::AllowPairing(bool)
-    {
-    }
+    {}
 
     void GapCentralSt::HandleHciDisconnectEvent(hci_event_pckt& eventPacket)
     {
@@ -117,14 +133,16 @@ namespace hal
             HandleAdvertisingReport(advertisingEvent.Advertising_Report[i]);
     }
 
-    void GapCentralSt::HandleHciLeConnectionUpdateCompleteEvent(evt_le_meta_event* metaEvent)
+    void GapCentralSt::HandleHciLeConnectionCompleteEvent(evt_le_meta_event* metaEvent)
     {
-        GapSt::HandleHciLeConnectionUpdateCompleteEvent(metaEvent);
-
-        const auto evtConnectionUpdate = reinterpret_cast<hci_le_connection_update_complete_event_rp0*>(metaEvent->data);
+        GapSt::HandleHciLeConnectionCompleteEvent(metaEvent);
+        initiatingStateTimer.Cancel();
+    }
 
-        connectionParameters.slaveLatency = evtConnectionUpdate->Conn_Latency;
-        connectionParameters.supervisorTimeoutMs = evtConnectionUpdate->Supervision_Timeout * 10;
+    void GapCentralSt::HandleHciLeEnhancedConnectionCompleteEvent(evt_le_meta_event* metaEvent)
+    {
+        GapSt::HandleHciLeEnhancedConnectionCompleteEvent(metaEvent);
+        initiatingStateTimer.Cancel();
     }
 
     void GapCentralSt::HandleGapProcedureCompleteEvent(evt_blecore_aci* vendorEvent)
@@ -137,6 +155,8 @@ namespace hal
 
         if (gapProcedureEvent.Procedure_Code == GAP_LIMITED_DISCOVERY_PROC || gapProcedureEvent.Procedure_Code == GAP_GENERAL_DISCOVERY_PROC)
             HandleGapDiscoveryProcedureEvent();
+        else if (gapProcedureEvent.Procedure_Code == GAP_DIRECT_CONNECTION_ESTABLISHMENT_PROC)
+            HandleGapDirectConnectionProcedureCompleteEvent();
     }
 
     void GapCentralSt::HandleGattCompleteEvent(evt_blecore_aci* vendorEvent)
@@ -184,12 +204,18 @@ namespace hal
         if (!IsTxDataLengthConfigured(dataLengthChangeEvent))
             onMtuExchangeDone = [this]()
             {
-                SetDataLength();
+                infra::EventDispatcherWithWeakPtr::Instance().Schedule([this]()
+                    {
+                        SetDataLength();
+                    });
             };
         else
             onMtuExchangeDone = [this]()
             {
-                SetPhy();
+                infra::EventDispatcherWithWeakPtr::Instance().Schedule([this]()
+                    {
+                        SetPhy();
+                    });
             };
 
         if (onDataLengthChanged)
@@ -222,6 +248,14 @@ namespace hal
             });
     }
 
+    void GapCentralSt::HandleGapDirectConnectionProcedureCompleteEvent()
+    {
+        infra::Subject<services::GapCentralObserver>::NotifyObservers([](services::GapCentralObserver& observer)
+            {
+                observer.StateChanged(services::GapState::standby);
+            });
+    }
+
     void GapCentralSt::MtuExchange() const
     {
         auto status = aci_gatt_exchange_config(this->connectionContext.connectionHandle);
@@ -232,7 +266,10 @@ namespace hal
     {
         onDataLengthChanged = [this]()
         {
-            SetPhy();
+            infra::EventDispatcherWithWeakPtr::Instance().Schedule([this]()
+                {
+                    SetPhy();
+                });
         };
 
         auto status = hci_le_set_data_length(this->connectionContext.connectionHandle, services::GapConnectionParameters::connectionInitialMaxTxOctets, services::GapConnectionParameters::connectionInitialMaxTxTime);

diff --git a/hal_st/middlewares/ble_middleware/GapCentralSt.hpp b/hal_st/middlewares/ble_middleware/GapCentralSt.hpp
@@ -3,6 +3,7 @@
 
 #include "ble/ble.h"
 #include "hal_st/middlewares/ble_middleware/GapSt.hpp"
+#include "infra/timer/Timer.hpp"
 #include "infra/util/AutoResetFunction.hpp"
 
 namespace hal
@@ -15,7 +16,8 @@ namespace hal
         GapCentralSt(hal::HciEventSource& hciEventSource, services::BondStorageSynchronizer& bondStorageSynchronizer, const Configuration& configuration);
 
         // Implementation of services::GapCentral
-        void Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType) override;
+        void Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType, infra::Duration initiatingTimeout) override;
+        void CancelConnect() override;
         void Disconnect() override;
         void SetAddress(hal::MacAddress macAddress, services::GapDeviceAddressType addressType) override;
         void StartDeviceDiscovery() override;
@@ -27,7 +29,8 @@ namespace hal
     protected:
         void HandleHciDisconnectEvent(hci_event_pckt& eventPacket) override;
         void HandleHciLeAdvertisingReportEvent(evt_le_meta_event* metaEvent) override;
-        void HandleHciLeConnectionUpdateCompleteEvent(evt_le_meta_event* metaEvent) override;
+        void HandleHciLeConnectionCompleteEvent(evt_le_meta_event* metaEvent) override;
+        void HandleHciLeEnhancedConnectionCompleteEvent(evt_le_meta_event* metaEvent) override;
         void HandleHciLeDataLengthChangeEvent(evt_le_meta_event* metaEvent) override;
         void HandleHciLePhyUpdateCompleteEvent(evt_le_meta_event* metaEvent) override;
         void HandleGapProcedureCompleteEvent(evt_blecore_aci* vendorEvent) override;
@@ -36,6 +39,7 @@ namespace hal
 
     private:
         void HandleGapDiscoveryProcedureEvent();
+        void HandleGapDirectConnectionProcedureCompleteEvent();
 
         void HandleAdvertisingReport(const Advertising_Report_t& advertisingReport);
         void SetPhy() const;
@@ -60,6 +64,7 @@ namespace hal
         services::GapConnectionParameters connectionParameters;
         infra::AutoResetFunction<void()> onMtuExchangeDone;
         infra::AutoResetFunction<void()> onDataLengthChanged;
+        infra::TimerSingleShot initiatingStateTimer;
     };
 }
 

diff --git a/hal_st/middlewares/ble_middleware/GapSt.cpp b/hal_st/middlewares/ble_middleware/GapSt.cpp
@@ -166,21 +166,15 @@ namespace hal
         auto connectionCompleteEvent = *reinterpret_cast<hci_le_connection_complete_event_rp0*>(metaEvent->data);
 
         if (connectionCompleteEvent.Status == BLE_STATUS_SUCCESS)
-        {
             SetConnectionContext(connectionCompleteEvent.Connection_Handle, connectionCompleteEvent.Peer_Address_Type, &connectionCompleteEvent.Peer_Address[0]);
-            maxAttMtu = defaultMaxAttMtuSize;
-        }
     }
 
     void GapSt::HandleHciLeEnhancedConnectionCompleteEvent(evt_le_meta_event* metaEvent)
     {
         auto connectionCompleteEvt = *reinterpret_cast<hci_le_enhanced_connection_complete_event_rp0*>(metaEvent->data);
 
         if (connectionCompleteEvt.Status == BLE_STATUS_SUCCESS)
-        {
             SetConnectionContext(connectionCompleteEvt.Connection_Handle, connectionCompleteEvt.Peer_Address_Type, &connectionCompleteEvt.Peer_Address[0]);
-            maxAttMtu = defaultMaxAttMtuSize;
-        }
     }
 
     void GapSt::HandleBondLostEvent(evt_blecore_aci* vendorEvent)
@@ -338,6 +332,7 @@ namespace hal
             }
         };
 
+        maxAttMtu = defaultMaxAttMtuSize;
         connectionContext.connectionHandle = connectionHandle;
         connectionContext.peerAddressType = deducePeerAddressType(peerAddressType);
         std::copy_n(peerAddress, connectionContext.peerAddress.size(), std::begin(connectionContext.peerAddress));

diff --git a/hal_st/middlewares/ble_middleware/TracingGapCentralSt.cpp b/hal_st/middlewares/ble_middleware/TracingGapCentralSt.cpp
@@ -7,13 +7,15 @@ namespace hal
         , tracer(tracer)
     {}
 
-    void TracingGapCentralSt::Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType)
+    void TracingGapCentralSt::Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType, infra::Duration initiatingTimeout)
     {
         tracer.Trace() << "TracingGapCentralSt::Connect, MAC address: "
                        << infra::AsMacAddress(macAddress)
                        << ", type: "
-                       << addressType;
-        GapCentralSt::Connect(macAddress, addressType);
+                       << addressType
+                       << ", initiating timeout (ms): "
+                       << std::chrono::duration_cast<std::chrono::milliseconds>(initiatingTimeout).count();
+        GapCentralSt::Connect(macAddress, addressType, initiatingTimeout);
     }
 
     void TracingGapCentralSt::Disconnect()

diff --git a/hal_st/middlewares/ble_middleware/TracingGapCentralSt.hpp b/hal_st/middlewares/ble_middleware/TracingGapCentralSt.hpp
@@ -13,7 +13,7 @@ namespace hal
         TracingGapCentralSt(hal::HciEventSource& hciEventSource, services::BondStorageSynchronizer& bondStorageSynchronizer, const Configuration& configuration, services::Tracer& tracer);
 
         // Implementation of services::GapCentral
-        void Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType) override;
+        void Connect(hal::MacAddress macAddress, services::GapDeviceAddressType addressType, infra::Duration initiatingTimeout) override;
         void Disconnect() override;
         void SetAddress(hal::MacAddress macAddress, services::GapDeviceAddressType addressType) override;
         void StartDeviceDiscovery() override;