diff --git a/deps/microReticulum b/deps/microReticulum index 17244c12..89d0aae7 160000 --- a/deps/microReticulum +++ b/deps/microReticulum @@ -1 +1 @@ -Subproject commit 17244c12c6c6f4a87f2374f972c99c86502737bc +Subproject commit 89d0aae780a96691ad485761363d8c3de453d4c3 diff --git a/lib/ble_interface/BLEInterface.cpp b/lib/ble_interface/BLEInterface.cpp index 259daef1..2e345c49 100644 --- a/lib/ble_interface/BLEInterface.cpp +++ b/lib/ble_interface/BLEInterface.cpp @@ -157,6 +157,20 @@ void BLEInterface::loop() { DEBUG("BLEInterface: Processing deferred handshake for " + pending.identity.toHex().substr(0, 8) + "..."); + // Check for duplicate identity — only keep one connection per identity + PeerInfo* existing = _peer_manager.getPeerByIdentity(pending.identity); + if (existing && existing->isConnected() && existing->mac_address != pending.mac) { + INFO("BLEInterface: Duplicate identity " + pending.identity.toHex().substr(0, 8) + + " - disconnecting new connection"); + // Disconnect the new (duplicate) connection + PeerInfo* new_peer = _peer_manager.getPeerByMac(pending.mac); + if (new_peer && new_peer->conn_handle != 0xFFFF) { + _platform->disconnect(new_peer->conn_handle); + } + _peer_manager.connectionFailed(pending.mac); + continue; + } + // Update peer manager with identity _peer_manager.setPeerIdentity(pending.mac, pending.identity); _peer_manager.connectionSucceeded(pending.identity); @@ -572,16 +586,20 @@ void BLEInterface::onConnected(const ConnectionHandle& conn) { _peer_manager.setPeerState(mac, PeerState::HANDSHAKING); _peer_manager.setPeerHandle(mac, conn.handle); + // Set MTU from connection (onMTUChange only fires for peripheral connections) + if (conn.mtu > 0) { + _peer_manager.setPeerMTU(mac, conn.mtu); + } + // Mark as central connection (we initiated the connection) PeerInfo* peer = _peer_manager.getPeerByMac(mac); if (peer) { peer->is_central = true; // We ARE central in this connection - INFO("BLEInterface: Stored conn_handle=" + std::to_string(conn.handle) + - " for peer " + conn.peer_address.toString()); } INFO("BLE: Connected to " + conn.peer_address.toString() + - " (we are central)"); + " handle=" + std::to_string(conn.handle) + + " mtu=" + std::to_string(conn.mtu) + " (we are central)"); // Discover services _platform->discoverServices(conn.handle); @@ -602,6 +620,7 @@ void BLEInterface::onDisconnected(const ConnectionHandle& conn, uint8_t reason) } } _reassembler.clearForPeer(identity); + _peer_manager.setPeerHandle(identity, 0xFFFF); _peer_manager.setPeerState(identity, PeerState::DISCOVERED); } else { // Peer might still be in CONNECTING state (no identity yet) @@ -672,11 +691,16 @@ void BLEInterface::onServicesDiscovered(const ConnectionHandle& conn, bool succe // Store the peer's identity - handshake complete for receiving direction _identity_manager.completeHandshake(mac, identity, true); - // Now send our identity directly (don't use initiateHandshake which - // creates a session that would time out since we already have the mapping) + // Send our identity to the peer's IDENTITY_CHAR. + // Use Write Without Response (false) to avoid blocking — + // the handshake is already complete from our side. if (_identity_manager.hasLocalIdentity()) { - _platform->write(handle, _identity_manager.getLocalIdentity(), true); - DEBUG("BLEInterface: Sent identity handshake to peer"); + ConnectionHandle c = _platform->getConnection(handle); + if (c.identity_handle != 0) { + _platform->writeCharacteristic(handle, c.identity_handle, + _identity_manager.getLocalIdentity(), false); + } + DEBUG("BLEInterface: Sent identity to peer"); } } else { WARNING("BLEInterface: Failed to read peer identity, trying write-based handshake"); @@ -715,8 +739,8 @@ void BLEInterface::onCentralConnected(const ConnectionHandle& conn) { peer->is_central = false; // We are NOT central in this connection } - DEBUG("BLEInterface: Central connected: " + conn.peer_address.toString() + - " (we are peripheral)"); + INFO("BLEInterface: Central connected: " + conn.peer_address.toString() + + " handle=" + std::to_string(conn.handle) + " (we are peripheral)"); } void BLEInterface::onCentralDisconnected(const ConnectionHandle& conn) { @@ -846,6 +870,11 @@ void BLEInterface::processDiscoveredPeers() { INFO("BLE: Peers=" + std::to_string(all_peers.size()) + " localMAC=" + _peer_manager.getLocalMac().toString()); for (PeerInfo* peer : all_peers) { + if (peer->mac_address.size() < Limits::MAC_SIZE) { + WARNING("BLE: Peer with empty MAC, state=" + + std::to_string(static_cast(peer->state))); + continue; + } bool should_initiate = _peer_manager.shouldInitiateConnection(peer->mac_address); INFO("BLE: Peer " + BLEAddress(peer->mac_address.data()).toString() + " state=" + std::to_string(static_cast(peer->state)) + @@ -855,7 +884,7 @@ void BLEInterface::processDiscoveredPeers() { last_peer_log = now; } - if (candidate) { + if (candidate && candidate->mac_address.size() >= Limits::MAC_SIZE) { INFO("BLE: Connection candidate: " + BLEAddress(candidate->mac_address.data()).toString() + " type=" + std::to_string(candidate->address_type) + " canAccept=" + std::string(_peer_manager.canAcceptConnection() ? "yes" : "no")); @@ -889,6 +918,25 @@ void BLEInterface::sendKeepalives() { auto connected = _peer_manager.getConnectedPeers(); for (PeerInfo* peer : connected) { if (peer->hasIdentity()) { + // Verify connection handle is valid before sending + if (peer->conn_handle == 0xFFFF) { + WARNING("BLEInterface: Peer " + peer->identity.toHex().substr(0, 8) + + " state=CONNECTED but conn_handle=INVALID, resetting"); + _peer_manager.setPeerState(peer->identity, PeerState::DISCOVERED); + continue; + } + + // Cross-check with platform connection table + ConnectionHandle platformConn = _platform->getConnection(peer->conn_handle); + if (!platformConn.isValid()) { + WARNING("BLEInterface: Peer " + peer->identity.toHex().substr(0, 8) + + " has stale conn_handle=" + std::to_string(peer->conn_handle) + + ", resetting"); + _peer_manager.setPeerHandle(peer->identity, 0xFFFF); + _peer_manager.setPeerState(peer->identity, PeerState::DISCOVERED); + continue; + } + bool sent = false; if (peer->is_central) { sent = _platform->write(peer->conn_handle, keepalive, false); @@ -905,6 +953,14 @@ void BLEInterface::sendKeepalives() { std::to_string(peer->consecutive_keepalive_failures) + " times, disconnecting " + peer->identity.toHex().substr(0, 8)); _platform->disconnect(peer->conn_handle); + + // Force-remove peer if disconnect keeps failing + if (peer->consecutive_keepalive_failures >= PeerInfo::MAX_KEEPALIVE_FAILURES * 2) { + WARNING("BLEInterface: Force-removing unresponsive peer " + + peer->identity.toHex().substr(0, 8)); + _identity_manager.removeMapping(peer->mac_address); + _peer_manager.removePeer(peer->identity); + } } } } diff --git a/lib/ble_interface/BLEInterface.h b/lib/ble_interface/BLEInterface.h index d30d550e..0d9538ee 100644 --- a/lib/ble_interface/BLEInterface.h +++ b/lib/ble_interface/BLEInterface.h @@ -19,12 +19,12 @@ #include "Interface.h" #include "Bytes.h" #include "Type.h" -#include "BLE/BLETypes.h" -#include "BLE/BLEPlatform.h" -#include "BLE/BLEFragmenter.h" -#include "BLE/BLEReassembler.h" -#include "BLE/BLEPeerManager.h" -#include "BLE/BLEIdentityManager.h" +#include "BLETypes.h" +#include "BLEPlatform.h" +#include "BLEFragmenter.h" +#include "BLEReassembler.h" +#include "BLEPeerManager.h" +#include "BLEIdentityManager.h" #include diff --git a/lib/ble_interface/BLEPeerManager.cpp b/lib/ble_interface/BLEPeerManager.cpp index 26966e9d..5e1eed6f 100644 --- a/lib/ble_interface/BLEPeerManager.cpp +++ b/lib/ble_interface/BLEPeerManager.cpp @@ -399,10 +399,18 @@ void BLEPeerManager::setPeerState(const Bytes& identifier, PeerState state) { } void BLEPeerManager::setPeerHandle(const Bytes& identifier, uint16_t conn_handle) { + // Validate handle is in range (ESP32 NimBLE uses small handles) + if (conn_handle != 0xFFFF && conn_handle >= MAX_CONN_HANDLES) { + WARNING("BLEPeerManager: Rejecting invalid conn_handle=" + + std::to_string(conn_handle) + " (max=" + + std::to_string(MAX_CONN_HANDLES - 1) + ")"); + return; + } + PeerInfo* peer = findPeer(identifier); if (peer) { // Remove old handle mapping if exists - if (peer->conn_handle != 0xFFFF) { + if (peer->conn_handle != 0xFFFF && peer->conn_handle < MAX_CONN_HANDLES) { clearHandleToPeer(peer->conn_handle); } peer->conn_handle = conn_handle; @@ -673,11 +681,23 @@ void BLEPeerManager::promoteToIdentityKeyed(const Bytes& mac_address, const Byte return; } - // Find an empty slot in identity pool - PeerByIdentitySlot* identity_slot = findEmptyPeerByIdentitySlot(); - if (!identity_slot) { - WARNING("BLEPeerManager: Identity pool is full, cannot promote peer"); - return; + // Check if there's already an identity-keyed entry for this peer + // (e.g. from a previous connection that was disconnected but not cleared) + PeerByIdentitySlot* identity_slot = findPeerByIdentitySlot(identity); + if (identity_slot) { + // Reuse existing slot — clear old handle mapping first + if (identity_slot->peer.conn_handle != 0xFFFF && + identity_slot->peer.conn_handle < MAX_CONN_HANDLES) { + clearHandleToPeer(identity_slot->peer.conn_handle); + } + DEBUG("BLEPeerManager: Reusing existing identity slot for peer"); + } else { + // Find an empty slot in identity pool + identity_slot = findEmptyPeerByIdentitySlot(); + if (!identity_slot) { + WARNING("BLEPeerManager: Identity pool is full, cannot promote peer"); + return; + } } // Copy peer info to identity pool @@ -686,9 +706,15 @@ void BLEPeerManager::promoteToIdentityKeyed(const Bytes& mac_address, const Byte identity_slot->peer = mac_slot->peer; identity_slot->peer.identity = identity; - // Update handle mapping to point to new location + // Validate and update handle mapping to point to new location if (identity_slot->peer.conn_handle != 0xFFFF) { - setHandleToPeer(identity_slot->peer.conn_handle, &identity_slot->peer); + if (identity_slot->peer.conn_handle < MAX_CONN_HANDLES) { + setHandleToPeer(identity_slot->peer.conn_handle, &identity_slot->peer); + } else { + WARNING("BLEPeerManager: Promoted peer has invalid conn_handle=" + + std::to_string(identity_slot->peer.conn_handle) + ", clearing"); + identity_slot->peer.conn_handle = 0xFFFF; + } } // Add MAC-to-identity mapping diff --git a/lib/ble_interface/BLEPeerManager.h b/lib/ble_interface/BLEPeerManager.h index 6548c53c..10e5feb6 100644 --- a/lib/ble_interface/BLEPeerManager.h +++ b/lib/ble_interface/BLEPeerManager.h @@ -54,6 +54,10 @@ struct PeerInfo { uint8_t consecutive_failures = 0; double blacklisted_until = 0.0; + // Keepalive failure tracking + uint8_t consecutive_keepalive_failures = 0; + static constexpr uint8_t MAX_KEEPALIVE_FAILURES = 3; + // BLE connection handle (platform-specific) uint16_t conn_handle = 0xFFFF; diff --git a/lib/ble_interface/BLEPlatform.h b/lib/ble_interface/BLEPlatform.h index f6a3cad1..85bc357e 100644 --- a/lib/ble_interface/BLEPlatform.h +++ b/lib/ble_interface/BLEPlatform.h @@ -190,6 +190,16 @@ public: */ virtual bool write(uint16_t conn_handle, const Bytes& data, bool response = true) = 0; + /** + * @brief Write to a specific characteristic by handle + */ + virtual bool writeCharacteristic(uint16_t conn_handle, uint16_t char_handle, + const Bytes& data, bool response = true) { + // Default: fall back to general write (subclass can override) + (void)char_handle; + return write(conn_handle, data, response); + } + /** * @brief Read from a characteristic * diff --git a/lib/ble_interface/BLETypes.h b/lib/ble_interface/BLETypes.h index 14ade1ba..6cd915ca 100644 --- a/lib/ble_interface/BLETypes.h +++ b/lib/ble_interface/BLETypes.h @@ -54,8 +54,8 @@ namespace UUID { namespace MTU { static constexpr uint16_t REQUESTED = 517; // Request maximum MTU (BLE 5.0) static constexpr uint16_t MINIMUM = 23; // BLE 4.0 minimum MTU - static constexpr uint16_t INITIAL = 185; // Conservative default (BLE 4.2) static constexpr uint16_t ATT_OVERHEAD = 3; // ATT protocol header overhead + static constexpr uint16_t INITIAL = 185 - ATT_OVERHEAD; // Conservative default (BLE 4.2), minus ATT overhead } //============================================================================= @@ -66,11 +66,13 @@ namespace Timing { static constexpr double KEEPALIVE_INTERVAL = 15.0; // Seconds between keepalives static constexpr double REASSEMBLY_TIMEOUT = 30.0; // Seconds to complete reassembly static constexpr double CONNECTION_TIMEOUT = 30.0; // Seconds to establish connection - static constexpr double HANDSHAKE_TIMEOUT = 10.0; // Seconds for identity exchange + static constexpr double HANDSHAKE_TIMEOUT = 30.0; // Seconds for identity exchange (match Columba) static constexpr double SCAN_INTERVAL = 5.0; // Seconds between scans static constexpr double PEER_TIMEOUT = 30.0; // Seconds before peer removal static constexpr double POST_MTU_DELAY = 0.15; // Seconds after MTU negotiation static constexpr double BLACKLIST_BASE_BACKOFF = 60.0; // Base backoff seconds + static constexpr double ZOMBIE_TIMEOUT = 45.0; // Seconds with no activity before force-disconnect + static constexpr double ADVERTISING_REFRESH_INTERVAL = 60.0; // Seconds between advertising refreshes } //============================================================================= @@ -111,9 +113,10 @@ namespace Fragment { static constexpr size_t HEADER_SIZE = 5; enum Type : uint8_t { + LONE = 0x00, // Single fragment (complete message) START = 0x01, // First fragment of multi-fragment message CONTINUE = 0x02, // Middle fragment - END = 0x03 // Last fragment (or single fragment) + END = 0x03 // Last fragment }; } diff --git a/lib/ble_interface/platforms/NimBLEPlatform.cpp b/lib/ble_interface/platforms/NimBLEPlatform.cpp index 4b3fbe89..c683694e 100644 --- a/lib/ble_interface/platforms/NimBLEPlatform.cpp +++ b/lib/ble_interface/platforms/NimBLEPlatform.cpp @@ -251,8 +251,19 @@ void NimBLEPlatform::loop() { } void NimBLEPlatform::shutdown() { + // Guard re-entrant shutdown (e.g. recoverBLEStack -> shutdown -> callback -> recoverBLEStack -> shutdown) + if (_shutting_down) { + WARNING("NimBLEPlatform: Shutdown already in progress, skipping"); + return; + } + INFO("NimBLEPlatform: Beginning graceful shutdown"); + // Mark as shutting down FIRST to prevent: + // 1. Re-entrant shutdown calls + // 2. Callbacks from doing cleanup (onDisconnect would double-free clients) + _shutting_down = true; + // CONC-H4: Graceful shutdown timeout for active write operations const uint32_t SHUTDOWN_TIMEOUT_MS = 10000; uint32_t start = millis(); @@ -288,11 +299,13 @@ void NimBLEPlatform::shutdown() { // Stop advertising and scanning stop(); - // Disconnect and cleanup clients with mutex protection + // Notify higher layers about all disconnections BEFORE deinit, + // so the peer manager can reset peer states properly. + // Do NOT delete clients individually — deinit(true) handles all client cleanup. if (xSemaphoreTake(_conn_mutex, pdMS_TO_TICKS(1000))) { - for (auto& kv : _clients) { - if (kv.second) { - NimBLEDevice::deleteClient(kv.second); + if (_on_disconnected) { + for (auto& kv : _connections) { + _on_disconnected(kv.second, 0x16); // 0x16 = local host terminated } } _clients.clear(); @@ -302,10 +315,9 @@ void NimBLEPlatform::shutdown() { xSemaphoreGive(_conn_mutex); } else { WARNING("NimBLEPlatform: Could not acquire mutex for cleanup - forcing cleanup"); - // Force cleanup anyway to prevent leaks - for (auto& kv : _clients) { - if (kv.second) { - NimBLEDevice::deleteClient(kv.second); + if (_on_disconnected) { + for (auto& kv : _connections) { + _on_disconnected(kv.second, 0x16); } } _clients.clear(); @@ -314,7 +326,8 @@ void NimBLEPlatform::shutdown() { _discovered_order.clear(); } - // Deinit NimBLE stack + // Deinit NimBLE stack — deinit(true) disconnects and deletes all clients/server. + // We do NOT delete clients individually above to avoid double-free. if (_initialized) { NimBLEDevice::deinit(true); _initialized = false; @@ -328,6 +341,8 @@ void NimBLEPlatform::shutdown() { _scan = nullptr; _advertising_obj = nullptr; + _shutting_down = false; + INFO("NimBLEPlatform: Shutdown complete" + std::string(wasCleanShutdown() ? "" : " (unclean - verify on boot)")); } @@ -681,6 +696,27 @@ bool NimBLEPlatform::startScan(uint16_t duration_ms) { return true; } + // Wait for host sync before trying to scan (host may be resetting after connection failure) + if (!ble_hs_synced()) { + DEBUG("NimBLEPlatform: Host not synced, waiting before scan..."); + uint32_t sync_wait = millis(); + while (!ble_hs_synced() && (millis() - sync_wait) < 2000) { + delay(50); + } + if (!ble_hs_synced()) { + _scan_fail_count++; + WARNING("NimBLEPlatform: Host still not synced after 2s, scan aborted (fail " + + std::to_string(_scan_fail_count) + "/" + + std::to_string(SCAN_FAIL_RECOVERY_THRESHOLD) + ")"); + if (_scan_fail_count >= SCAN_FAIL_RECOVERY_THRESHOLD) { + WARNING("NimBLEPlatform: Host stuck, triggering full stack recovery"); + _scan_fail_count = 0; + recoverBLEStack(); + } + return false; + } + } + // Log GAP hardware state before checking DEBUG("NimBLEPlatform: Pre-scan GAP state: disc=" + std::to_string(ble_gap_disc_active()) + " adv=" + std::to_string(ble_gap_adv_active()) + @@ -759,6 +795,7 @@ bool NimBLEPlatform::startScan(uint16_t duration_ms) { _scan_fail_count++; if (_scan_fail_count >= SCAN_FAIL_RECOVERY_THRESHOLD) { WARNING("NimBLEPlatform: Too many scan failures, entering error recovery"); + _scan_fail_count = 0; // Reset so we don't immediately re-enter after recovery enterErrorRecovery(); } @@ -1036,6 +1073,11 @@ int NimBLEPlatform::nativeGapEventHandler(struct ble_gap_event* event, void* arg } } + // During shutdown, skip cleanup — shutdown() handles it + if (platform->_shutting_down) { + break; + } + // Clean up established connections (handles MAC rotation, out of range, etc.) auto conn_it = platform->_connections.find(disc_handle); if (conn_it != platform->_connections.end()) { @@ -1082,18 +1124,14 @@ int NimBLEPlatform::nativeGapEventHandler(struct ble_gap_event* event, void* arg bool NimBLEPlatform::connectNative(const BLEAddress& address, uint16_t timeout_ms) { INFO("NimBLEPlatform: Connecting to " + address.toString() + " type=" + std::to_string(address.type)); - // Verify host-controller sync + // Verify host-controller sync — don't trigger recovery here, + // just return false and let the host recover naturally. A single + // connection failure (574) can cause a temporary host reset that + // resolves on its own. Triggering recoverBLEStack() here would + // kill all existing connections unnecessarily. if (!ble_hs_synced()) { - WARNING("NimBLEPlatform: Host not synced before connect"); - uint32_t sync_start = millis(); - while (!ble_hs_synced() && (millis() - sync_start) < 1000) { - delay(10); - } - if (!ble_hs_synced()) { - ERROR("NimBLEPlatform: Host sync timeout, entering error recovery"); - enterErrorRecovery(); - return false; - } + WARNING("NimBLEPlatform: Host not synced before connect, skipping"); + return false; } if (address.type > 3) { @@ -1117,32 +1155,41 @@ bool NimBLEPlatform::connectNative(const BLEAddress& address, uint16_t timeout_m client->setConnectionParams(24, 40, 0, 256); // 30-50ms interval, 2.56s timeout client->setConnectTimeout(timeout_ms); // milliseconds + // Suppress _on_connected in onConnect callback — we'll fire it from here + // after connect() returns. The onConnect callback runs in the NimBLE host + // task, and _on_connected triggers blocking GATT operations (service + // discovery) that would deadlock the host task. + _native_connect_pending = true; + // Connect (blocking) — NimBLE handles GAP event management internally bool connected = client->connect(nimAddr, false); // deleteAttributes=false + _native_connect_pending = false; + if (!connected) { INFO("NimBLEPlatform: Connection failed to " + address.toString()); NimBLEDevice::deleteClient(client); return false; } + // onConnect callback already stored in _connections/_clients. + // Update MTU (exchange happens after onConnect fires). uint16_t conn_handle = client->getConnHandle(); + uint16_t negotiated_mtu = client->getMTU() - MTU::ATT_OVERHEAD; + + auto conn_it = _connections.find(conn_handle); + if (conn_it != _connections.end()) { + conn_it->second.mtu = negotiated_mtu; + } + INFO("NimBLEPlatform: Connected to " + address.toString() + " handle=" + std::to_string(conn_handle) + - " MTU=" + std::to_string(client->getMTU())); - - // Track the connection - ConnectionHandle conn; - conn.handle = conn_handle; - conn.peer_address = address; - conn.local_role = Role::CENTRAL; - conn.state = ConnectionState::CONNECTED; - conn.mtu = client->getMTU() - MTU::ATT_OVERHEAD; - - _connections[conn_handle] = conn; - _clients[conn_handle] = client; + " MTU=" + std::to_string(negotiated_mtu)); + // Fire _on_connected from THIS task (BLEInterface loop), not the host task. + // This allows the callback to safely do blocking GATT operations. if (_on_connected) { + ConnectionHandle conn = getConnection(conn_handle); _on_connected(conn); } @@ -1282,6 +1329,18 @@ bool NimBLEPlatform::startAdvertising() { return true; } + // Wait for host sync before advertising (host may be resetting) + if (!ble_hs_synced()) { + uint32_t sync_wait = millis(); + while (!ble_hs_synced() && (millis() - sync_wait) < 1000) { + delay(50); + } + if (!ble_hs_synced()) { + DEBUG("NimBLEPlatform: Host not synced, cannot start advertising"); + return false; + } + } + // Check if we can start advertising if (!canStartAdvertising()) { DEBUG("NimBLEPlatform: Cannot start advertising - state check failed" + @@ -1403,6 +1462,7 @@ void NimBLEPlatform::setIdentityData(const Bytes& identity) { bool NimBLEPlatform::write(uint16_t conn_handle, const Bytes& data, bool response) { auto conn_it = _connections.find(conn_handle); if (conn_it == _connections.end()) { + DEBUG("NimBLEPlatform::write: no connection for handle " + std::to_string(conn_handle)); return false; } @@ -1412,20 +1472,35 @@ bool NimBLEPlatform::write(uint16_t conn_handle, const Bytes& data, bool respons // We are central - write to peripheral's RX characteristic auto client_it = _clients.find(conn_handle); if (client_it == _clients.end() || !client_it->second) { + WARNING("NimBLEPlatform::write: no client for handle " + std::to_string(conn_handle)); return false; } NimBLEClient* client = client_it->second; + if (!client->isConnected()) { + WARNING("NimBLEPlatform::write: client not connected for handle " + std::to_string(conn_handle)); + return false; + } + NimBLERemoteService* service = client->getService(UUID::SERVICE); - if (!service) return false; + if (!service) { + WARNING("NimBLEPlatform::write: service not found for handle " + std::to_string(conn_handle)); + return false; + } NimBLERemoteCharacteristic* rxChar = service->getCharacteristic(UUID::RX_CHAR); - if (!rxChar) return false; + if (!rxChar) { + WARNING("NimBLEPlatform::write: RX char not found for handle " + std::to_string(conn_handle)); + return false; + } // CONC-H4: Track active write for graceful shutdown beginWriteOperation(); bool result = rxChar->writeValue(data.data(), data.size(), response); endWriteOperation(); + if (!result) { + WARNING("NimBLEPlatform::write: writeValue failed for handle " + std::to_string(conn_handle)); + } return result; } else { // We are peripheral - this shouldn't be used, use notify instead @@ -1434,6 +1509,34 @@ bool NimBLEPlatform::write(uint16_t conn_handle, const Bytes& data, bool respons } } +bool NimBLEPlatform::writeCharacteristic(uint16_t conn_handle, uint16_t char_handle, + const Bytes& data, bool response) { + auto client_it = _clients.find(conn_handle); + if (client_it == _clients.end() || !client_it->second) { + return false; + } + + NimBLEClient* client = client_it->second; + if (!client->isConnected()) return false; + + NimBLERemoteService* service = client->getService(UUID::SERVICE); + if (!service) return false; + + // Find characteristic by handle + NimBLERemoteCharacteristic* chr = nullptr; + auto conn_it = _connections.find(conn_handle); + if (conn_it != _connections.end() && char_handle == conn_it->second.identity_handle) { + chr = service->getCharacteristic(UUID::IDENTITY_CHAR); + } + // Fall through to RX_CHAR if not identity + if (!chr) { + chr = service->getCharacteristic(UUID::RX_CHAR); + } + if (!chr) return false; + + return chr->writeValue(data.data(), data.size(), response); +} + bool NimBLEPlatform::read(uint16_t conn_handle, uint16_t char_handle, std::function callback) { auto client_it = _clients.find(conn_handle); @@ -1677,6 +1780,8 @@ void NimBLEPlatform::onConnect(NimBLEServer* pServer, NimBLEConnInfo& connInfo) } void NimBLEPlatform::onDisconnect(NimBLEServer* pServer, NimBLEConnInfo& connInfo, int reason) { + if (_shutting_down) return; // shutdown() handles cleanup + uint16_t conn_handle = connInfo.getConnHandle(); auto it = _connections.find(conn_handle); @@ -1778,7 +1883,11 @@ void NimBLEPlatform::onConnect(NimBLEClient* pClient) { _async_connect_pending = false; _async_connect_failed = false; - if (_on_connected) { + // When _native_connect_pending is true, connectNative() is doing a blocking + // connect and will fire _on_connected itself from the calling task. + // Firing it here (in the NimBLE host task) would deadlock because _on_connected + // triggers blocking GATT operations that require the host task to be free. + if (!_native_connect_pending && _on_connected) { _on_connected(conn); } } @@ -1797,6 +1906,14 @@ void NimBLEPlatform::onConnectFail(NimBLEClient* pClient, int reason) { void NimBLEPlatform::onDisconnect(NimBLEClient* pClient, int reason) { uint16_t conn_handle = pClient->getConnHandle(); + // During shutdown, cleanup is handled by shutdown() itself. + // Calling deleteClient here would double-free. + if (_shutting_down) { + DEBUG("NimBLEPlatform: onDisconnect during shutdown, skipping cleanup for handle " + + std::to_string(conn_handle)); + return; + } + auto it = _connections.find(conn_handle); if (it != _connections.end()) { ConnectionHandle conn = it->second; diff --git a/lib/ble_interface/platforms/NimBLEPlatform.h b/lib/ble_interface/platforms/NimBLEPlatform.h index 2b620d11..3ca26d1b 100644 --- a/lib/ble_interface/platforms/NimBLEPlatform.h +++ b/lib/ble_interface/platforms/NimBLEPlatform.h @@ -118,6 +118,8 @@ public: // GATT Operations bool write(uint16_t conn_handle, const Bytes& data, bool response = true) override; + bool writeCharacteristic(uint16_t conn_handle, uint16_t char_handle, + const Bytes& data, bool response = true) override; bool read(uint16_t conn_handle, uint16_t char_handle, std::function callback) override; bool enableNotifications(uint16_t conn_handle, bool enable) override; @@ -246,6 +248,7 @@ private: PlatformConfig _config; bool _initialized = false; bool _running = false; + volatile bool _shutting_down = false; Bytes _identity_data; unsigned long _scan_stop_time = 0; // millis() when to stop continuous scan diff --git a/src/main.cpp b/src/main.cpp index 78dc5253..dcd3f645 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1253,6 +1253,9 @@ void loop() { // Process Reticulum reticulum->loop(); + // Periodically persist identity/transport data (display names, paths, etc.) + reticulum->should_persist_data(); + // Process TCP interface if (tcp_interface) { tcp_interface->loop();