fix: better gap state update in gap central (#507)

* clean up GapCentralSt

- remove dynamic phy update, use default setting
- change state to connected without waiting for all update ops

* GapCentralSt: add missing connected state update

* GapCentralSt: Handle failed connection complete better

* update ci amp-devcontainer-cpp

Author: oguzcanoguz

.github/workflows/ci.yml

@@ -67,7 +67,7 @@ jobs:
   host_build_clang_msvc:
     name: Windows Host Build from Devcontainer
     runs-on: [ubuntu-latest]
-    container: ghcr.io/philips-software/amp-devcontainer-cpp:5.2.0@sha256:c47fcc83b59fb08f3a3a6e591b18bad49b3862acc35770fca6cec9ad0adb9cb2 # v5.2.0
+    container: ghcr.io/philips-software/amp-devcontainer-cpp:v5.6.0@sha256:884732270a353e8446f813e649f2918ac53ce446a6c0dd4f7f84a6d58fb26bce # v5.6.0
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
       - uses: actions/cache@1bd1e32a3bdc45362d1e726936510720a7c30a57 # v4.2.0

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 "infra/util/Function.hpp"
+#include "services/ble/Gap.hpp"
 #include <algorithm>
 #include <chrono>
 #include <cmath>
@@ -136,14 +138,32 @@ namespace hal
 
     void GapCentralSt::HandleHciLeConnectionCompleteEvent(evt_le_meta_event* metaEvent)
     {
-        GapSt::HandleHciLeConnectionCompleteEvent(metaEvent);
+        UpdateStateOnConnectionComplete(metaEvent);
+
         initiatingStateTimer.Cancel();
+
+        GapSt::HandleHciLeConnectionCompleteEvent(metaEvent);
     }
 
     void GapCentralSt::HandleHciLeEnhancedConnectionCompleteEvent(evt_le_meta_event* metaEvent)
     {
-        GapSt::HandleHciLeEnhancedConnectionCompleteEvent(metaEvent);
+        UpdateStateOnConnectionComplete(metaEvent);
+
         initiatingStateTimer.Cancel();
+
+        GapSt::HandleHciLeEnhancedConnectionCompleteEvent(metaEvent);
+    }
+
+    void GapCentralSt::UpdateStateOnConnectionComplete(evt_le_meta_event* metaEvent)
+    {
+        auto status = reinterpret_cast<hci_le_enhanced_connection_complete_event_rp0*>(metaEvent->data)->Status;
+
+        services::GapState state = status == BLE_STATUS_SUCCESS ? services::GapState::connected : services::GapState::standby;
+
+        infra::Subject<services::GapCentralObserver>::NotifyObservers([state](services::GapCentralObserver& observer)
+            {
+                observer.StateChanged(state);
+            });
     }
 
     void GapCentralSt::HandleGapProcedureCompleteEvent(evt_blecore_aci* vendorEvent)
@@ -213,17 +233,6 @@ namespace hal
                         SetDataLength();
                     });
             };
-        else
-            onMtuExchangeDone = [this]()
-            {
-                infra::EventDispatcherWithWeakPtr::Instance().Schedule([this]()
-                    {
-                        SetPhy();
-                    });
-            };
-
-        if (onDataLengthChanged)
-            onDataLengthChanged();
     }
 
     void GapCentralSt::HandleHciLePhyUpdateCompleteEvent(evt_le_meta_event* metaEvent)
@@ -235,11 +244,6 @@ namespace hal
         really_assert(phyUpdateCompleteEvent.Connection_Handle == connectionContext.connectionHandle);
 
         onMtuExchangeDone = nullptr;
-
-        infra::Subject<services::GapCentralObserver>::NotifyObservers([](services::GapCentralObserver& observer)
-            {
-                observer.StateChanged(services::GapState::connected);
-            });
     }
 
     void GapCentralSt::HandleGapDiscoveryProcedureEvent()
@@ -269,24 +273,10 @@ namespace hal
 
     void GapCentralSt::SetDataLength()
     {
-        onDataLengthChanged = [this]()
-        {
-            infra::EventDispatcherWithWeakPtr::Instance().Schedule([this]()
-                {
-                    SetPhy();
-                });
-        };
-
         auto status = hci_le_set_data_length(this->connectionContext.connectionHandle, services::GapConnectionParameters::connectionInitialMaxTxOctets, services::GapConnectionParameters::connectionInitialMaxTxTime);
         assert(status == BLE_STATUS_SUCCESS);
     }
 
-    void GapCentralSt::SetPhy() const
-    {
-        auto status = hci_le_set_phy(this->connectionContext.connectionHandle, GapSt::allPhys, GapSt::speed2Mbps, GapSt::speed2Mbps, 0);
-        assert(status == BLE_STATUS_SUCCESS);
-    }
-
     void GapCentralSt::HandleAdvertisingReport(const Advertising_Report_t& advertisingReport)
     {
         services::GapAdvertisingReport discoveredDevice;
@@ -314,5 +304,6 @@ namespace hal
 
         SetIoCapabilities(services::GapPairing::IoCapabilities::none);
         SetSecurityMode(services::GapPairing::SecurityMode::mode1, services::GapPairing::SecurityLevel::level1);
+        hci_le_set_default_phy(allPhys, speed2Mbps, speed2Mbps);
     }
 }

hal_st/middlewares/ble_middleware/GapCentralSt.hpp

@@ -42,10 +42,10 @@ namespace hal
         void HandleGapDirectConnectionProcedureCompleteEvent();
 
         void HandleAdvertisingReport(const Advertising_Report_t& advertisingReport);
-        void SetPhy() const;
         void SetDataLength();
         void MtuExchange() const;
         void Initialize(const GapService& gapService);
+        void UpdateStateOnConnectionComplete(evt_le_meta_event* metaEvent);
 
     private:
         static const services::GapConnectionParameters connectionUpdateParameters;
@@ -63,7 +63,6 @@ namespace hal
         bool discovering = false;
         services::GapConnectionParameters connectionParameters;
         infra::AutoResetFunction<void()> onMtuExchangeDone;
-        infra::AutoResetFunction<void()> onDataLengthChanged;
         infra::TimerSingleShot initiatingStateTimer;
     };
 }