BTAdapter Server: Offload waiting for client connection from BTAdapter::mgmtEvDeviceConnectedHCI() HCIHandler callback to BTDevice::processL2CAPSetup() dedicated thread

Waiting for the client connection on the HCIHandler callback and thread blocks upcoming events, e.g. disconnections etc.

Hence moving fetching and waiting for the client connection where it belongs, on the BTDevice's dedicated thread
performing the L2CAP setup for the client connection and optional SMP protocol before deviceReady().

Further, the fetch and wait method BTAdapter::get_l2cap_connection() also tests whether the device is still connected.

Timeout for waiting for a client connection after having announced the connection via HCI is 1 second.
Actually measured connection duration is 0 - 40ms.

Impact: Resolves disconnect issues using one BT5 not well-known working adapter involved.

4 files changed, 52 insertions, 26 deletions

diff --git a/api/direct_bt/BTAdapter.hpp b/api/direct_bt/BTAdapter.hpp
index f6bf2748..58ad73c1 100644
--- a/api/direct_bt/BTAdapter.hpp
+++ b/api/direct_bt/BTAdapter.hpp
@@ -505,6 +505,7 @@ namespace direct_bt {
             void l2capServerWork(jau::service_runner& sr);
             void l2capServerInit(jau::service_runner& sr);
             void l2capServerEnd(jau::service_runner& sr);
+            std::unique_ptr<L2CAPClient> get_l2cap_connection(std::shared_ptr<BTDevice> device);
 
             bool mgmtEvNewSettingsMgmt(const MgmtEvent& e) noexcept;
             void updateAdapterSettings(const bool off_thread, const AdapterSetting new_settings, const bool sendEvent, const uint64_t timestamp) noexcept;
diff --git a/api/direct_bt/DBTConst.hpp b/api/direct_bt/DBTConst.hpp
index 0687e74f..a2810ba6 100644
--- a/api/direct_bt/DBTConst.hpp
+++ b/api/direct_bt/DBTConst.hpp
@@ -70,6 +70,11 @@ namespace direct_bt {
     inline constexpr const jau::nsize_t SMP_NEXT_EVENT_TIMEOUT_MS = 2000;
 
     /**
+     * Maximum time in milliseconds to wait for L2CAP client connection when adapter is in server mode.
+     */
+    inline constexpr const jau::nsize_t L2CAP_CLIENT_CONNECT_TIMEOUT_MS = 1000;
+
+    /**
      * Maximum number of enabling discovery in background in case of failure
      */
     inline constexpr const jau::nsize_t MAX_BACKGROUND_DISCOVERY_RETRY = 3;
diff --git a/src/direct_bt/BTAdapter.cpp b/src/direct_bt/BTAdapter.cpp
index 4db45089..c6204575 100644
--- a/src/direct_bt/BTAdapter.cpp
+++ b/src/direct_bt/BTAdapter.cpp
@@ -1813,7 +1813,7 @@ void BTAdapter::l2capServerWork(jau::service_runner& sr) {
     (void)sr;
     std::unique_ptr<L2CAPClient> l2cap_att_ = l2cap_att_srv.accept();
     if( BTRole::Slave == getRole() && nullptr != l2cap_att_ ) {
-        DBG_PRINT("BTAdapter::l2capServer connected.1: %s", l2cap_att_->toString().c_str());
+        DBG_PRINT("L2CAP-ACCEPT: BTAdapter::l2capServer connected.1: %s", l2cap_att_->toString().c_str());
 
         std::unique_lock<std::mutex> lock(mtx_l2cap_att); // RAII-style acquire and relinquish via destructor
         l2cap_att = std::move( l2cap_att_ );
@@ -1821,9 +1821,37 @@ void BTAdapter::l2capServerWork(jau::service_runner& sr) {
         cv_l2cap_att.notify_all(); // notify waiting getter
 
     } else if( nullptr != l2cap_att_ ) {
-        DBG_PRINT("BTAdapter::l2capServer connected.2: %s", l2cap_att_->toString().c_str());
+        DBG_PRINT("L2CAP-ACCEPT: BTAdapter::l2capServer connected.2: %s", l2cap_att_->toString().c_str());
     } else {
-        DBG_PRINT("BTAdapter::l2capServer connected.0: nullptr");
+        DBG_PRINT("L2CAP-ACCEPT: BTAdapter::l2capServer connected.0: nullptr");
+    }
+}
+
+std::unique_ptr<L2CAPClient> BTAdapter::get_l2cap_connection(std::shared_ptr<BTDevice> device) {
+    if( BTRole::Slave == getRole() ) {
+        const BDAddressAndType& clientAddrAndType = device->getAddressAndType();
+        const jau::nsize_t timeout_ms = L2CAP_CLIENT_CONNECT_TIMEOUT_MS;
+
+        std::unique_lock<std::mutex> lock(mtx_l2cap_att); // RAII-style acquire and relinquish via destructor
+        while( device->getConnected() && ( nullptr == l2cap_att || l2cap_att->getRemoteAddressAndType() != clientAddrAndType ) ) {
+            std::chrono::steady_clock::time_point t0 = std::chrono::steady_clock::now();
+            std::cv_status s = cv_l2cap_att.wait_until(lock, t0 + std::chrono::milliseconds(timeout_ms));
+            if( std::cv_status::timeout == s && ( nullptr == l2cap_att || l2cap_att->getRemoteAddressAndType() != clientAddrAndType ) ) {
+                DBG_PRINT("L2CAP-ACCEPT: BTAdapter:get_l2cap_connection(dev_id %d): l2cap_att TIMEOUT", dev_id);
+                return nullptr;
+            }
+        }
+        if( nullptr != l2cap_att ) {
+            std::unique_ptr<L2CAPClient> l2cap_att_ = std::move( l2cap_att );
+            DBG_PRINT("L2CAP-ACCEPT: BTAdapter:get_l2cap_connection(dev_id %d): l2cap_att %s", dev_id, l2cap_att_->toString().c_str());
+            return l2cap_att_; // copy elision
+        } else {
+            DBG_PRINT("L2CAP-ACCEPT: BTAdapter:get_l2cap_connection(dev_id %d): Might got disconnected", dev_id);
+            return nullptr;
+        }
+    } else {
+        DBG_PRINT("L2CAP-ACCEPT: BTAdapter:get_l2cap_connection(dev_id %d): Not in server mode", dev_id);
+        return nullptr;
     }
 }
 
@@ -1869,7 +1897,6 @@ jau::nsize_t BTAdapter::smp_timeoutfunc(jau::simple_timer& timer) {
 
 bool BTAdapter::mgmtEvDeviceConnectedHCI(const MgmtEvent& e) noexcept {
     const MgmtEvtDeviceConnected &event = *static_cast<const MgmtEvtDeviceConnected *>(&e);
-    const BDAddressAndType deviceAddressAndType(event.getAddress(), event.getAddressType());
     EInfoReport ad_report;
     {
         ad_report.setSource(EInfoReport::Source::EIR);
@@ -1880,22 +1907,6 @@ bool BTAdapter::mgmtEvDeviceConnectedHCI(const MgmtEvent& e) noexcept {
     }
     DBG_PRINT("BTAdapter:hci:DeviceConnected(dev_id %d): %s: %s", dev_id, e.toString().c_str(), ad_report.toString().c_str());
 
-    std::unique_ptr<L2CAPClient> l2cap_att_;
-    if( BTRole::Slave == getRole() ) {
-        const uint32_t timeout_ms = 10000; // FIXME: Configurable?
-        std::unique_lock<std::mutex> lock(mtx_l2cap_att); // RAII-style acquire and relinquish via destructor
-        while( nullptr == l2cap_att || l2cap_att->getRemoteAddressAndType() != deviceAddressAndType ) {
-            std::chrono::steady_clock::time_point t0 = std::chrono::steady_clock::now();
-            std::cv_status s = cv_l2cap_att.wait_until(lock, t0 + std::chrono::milliseconds(timeout_ms));
-            if( std::cv_status::timeout == s && ( nullptr == l2cap_att || l2cap_att->getRemoteAddressAndType() != deviceAddressAndType ) ) {
-                DBG_PRINT("BTAdapter:hci:DeviceConnected(dev_id %d): l2cap_att TIMEOUT", dev_id);
-                return true;
-            }
-        }
-        l2cap_att_ = std::move( l2cap_att );
-        DBG_PRINT("BTAdapter:hci:DeviceConnected(dev_id %d): l2cap_att %s", dev_id, l2cap_att_->toString().c_str());
-    }
-
     int new_connect = 0;
     bool slave_unpair = false;
     BTDeviceRef device = findConnectedDevice(event.getAddress(), event.getAddressType());
@@ -1925,7 +1936,6 @@ bool BTAdapter::mgmtEvDeviceConnectedHCI(const MgmtEvent& e) noexcept {
     }
     bool has_smp_key;
     if( BTRole::Slave == getRole() ) {
-        device->l2cap_att = std::move(l2cap_att_);
         has_smp_key = nullptr != findSMPKeyBin( device->getAddressAndType() ); // PERIPHERAL_ADAPTER_MANAGES_SMP_KEYS
     } else {
         has_smp_key = false;
diff --git a/src/direct_bt/BTDevice.cpp b/src/direct_bt/BTDevice.cpp
index ed177e10..5d1cc160 100644
--- a/src/direct_bt/BTDevice.cpp
+++ b/src/direct_bt/BTDevice.cpp
@@ -579,7 +579,7 @@ void BTDevice::processL2CAPSetup(std::shared_ptr<BTDevice> sthis) {
     bool smp_auto = false;
     const bool is_local_server = BTRole::Master == btRole; // -> local GattRole::Server
 
-    if( addressAndType.isLEAddress() && ( !l2cap_att->is_open() || is_local_server ) ) {
+    if( addressAndType.isLEAddress() && ( is_local_server || !l2cap_att->is_open() ) ) {
         std::unique_lock<std::recursive_mutex> lock_pairing(mtx_pairing); // RAII-style acquire and relinquish via destructor
 
         DBG_PRINT("BTDevice::processL2CAPSetup: Start dev_id %u, %s", adapter.dev_id, toString().c_str());
@@ -609,11 +609,21 @@ void BTDevice::processL2CAPSetup(std::shared_ptr<BTDevice> sthis) {
                 to_string(sec_level).c_str());
 
         bool l2cap_open;
-        if( is_local_server && l2cap_att->is_open() ) {
-            if( BTSecurityLevel::UNSET < sec_level ) {
-                l2cap_open = l2cap_att->setBTSecurityLevel(sec_level);
+        if( is_local_server ) {
+            const uint64_t t0 = ( jau::environment::get().debug ) ? jau::getCurrentMilliseconds() : 0;
+            std::unique_ptr<L2CAPClient> l2cap_att_new = adapter.get_l2cap_connection(sthis);
+            const uint64_t td = ( jau::environment::get().debug ) ? jau::getCurrentMilliseconds() - t0 : 0;
+            if( nullptr == l2cap_att_new ) {
+                DBG_PRINT("L2CAP-ACCEPT: BTDevice::processL2CAPSetup: dev_id %d, td %" PRIu64 "ms, NULL l2cap_att", adapter.dev_id, td);
+                l2cap_open = false;
             } else {
-                l2cap_open = true;
+                l2cap_att = std::move(l2cap_att_new);
+                DBG_PRINT("L2CAP-ACCEPT: BTDevice::processL2CAPSetup: dev_id %d, td %" PRIu64 "ms, l2cap_att %s", adapter.dev_id, td, l2cap_att->toString().c_str());
+                if( BTSecurityLevel::UNSET < sec_level ) {
+                    l2cap_open = l2cap_att->setBTSecurityLevel(sec_level);
+                } else {
+                    l2cap_open = true;
+                }
             }
         } else {
             l2cap_open = l2cap_att->open(*this, sec_level); // initiates hciSMPMsgCallback() if sec_level > BT_SECURITY_LOW