From 05b1466ec3a0026dceb49a10e4e6052c94f999ac Mon Sep 17 00:00:00 2001 From: Marie Janssen Date: Fri, 1 Nov 2024 17:48:26 +0000 Subject: [PATCH] pw_bluetooth_sapphire: Handle ISO Disconnect Events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: b/309014357 Test: added IsoStreamManagerTest.DisconnectCIS Change-Id: I78b8bcc53a7ad5827f3cba71f157d8b054d6fef7 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/246192 Reviewed-by: Josh Conner Reviewed-by: Ben Lawson Lint: Lint 🤖 Commit-Queue: Marie Janssen Docs-Not-Needed: Marie Janssen --- .../host/iso/iso_stream_manager.cc | 39 ++++++++++++- .../host/iso/iso_stream_manager_test.cc | 56 +++++++++++++++++++ .../internal/host/iso/iso_stream_manager.h | 5 ++ 3 files changed, 99 insertions(+), 1 deletion(-) diff --git a/pw_bluetooth_sapphire/host/iso/iso_stream_manager.cc b/pw_bluetooth_sapphire/host/iso/iso_stream_manager.cc index 092ff2b4c4..40077e0888 100644 --- a/pw_bluetooth_sapphire/host/iso/iso_stream_manager.cc +++ b/pw_bluetooth_sapphire/host/iso/iso_stream_manager.cc @@ -14,6 +14,9 @@ #include "pw_bluetooth_sapphire/internal/host/iso/iso_stream_manager.h" +#include "pw_bluetooth_sapphire/internal/host/hci-spec/protocol.h" +#include "pw_bluetooth_sapphire/internal/host/transport/emboss_control_packets.h" + namespace bt::iso { IsoStreamManager::IsoStreamManager(hci_spec::ConnectionHandle handle, @@ -26,18 +29,29 @@ IsoStreamManager::IsoStreamManager(hci_spec::ConnectionHandle handle, auto weak_self = GetWeakPtr(); cis_request_handler_ = cmd_->AddLEMetaEventHandler( hci_spec::kLECISRequestSubeventCode, - [self = std::move(weak_self)](const hci::EmbossEventPacket& event) { + [self = weak_self](const hci::EmbossEventPacket& event) { if (!self.is_alive()) { return hci::CommandChannel::EventCallbackResult::kRemove; } self->OnCisRequest(event); return hci::CommandChannel::EventCallbackResult::kContinue; }); + + disconnect_handler_ = cmd_->AddEventHandler( + hci_spec::kDisconnectionCompleteEventCode, + [self = std::move(weak_self)](const hci::EmbossEventPacket& event) { + if (!self.is_alive()) { + return hci::CommandChannel::EventCallbackResult::kRemove; + } + self->OnDisconnect(event); + return hci::CommandChannel::EventCallbackResult::kContinue; + }); } IsoStreamManager::~IsoStreamManager() { if (cmd_.is_alive()) { cmd_->RemoveEventHandler(cis_request_handler_); + cmd_->RemoveEventHandler(disconnect_handler_); } if (hci_.is_alive()) { hci::IsoDataChannel* iso_data_channel = hci_->iso_data_channel(); @@ -123,6 +137,29 @@ void IsoStreamManager::OnCisRequest(const hci::EmbossEventPacket& event) { AcceptCisRequest(event_view, std::move(cb)); } +void IsoStreamManager::OnDisconnect(const hci::EmbossEventPacket& event) { + PW_CHECK(event.event_code() == hci_spec::kDisconnectionCompleteEventCode); + auto event_view = + event.view(); + hci_spec::ConnectionHandle disconnected_handle = + event_view.connection_handle().Read(); + for (auto it = streams_.begin(); it != streams_.end(); ++it) { + if (it->second->cis_handle() == disconnected_handle) { + bt_log( + INFO, "iso", "CIS Disconnected at handle %#x", disconnected_handle); + if (hci_.is_alive()) { + hci::IsoDataChannel* iso_data_channel = hci_->iso_data_channel(); + if (iso_data_channel) { + iso_data_channel->UnregisterConnection(disconnected_handle); + } + } + streams_.erase(it); + // There shouldn't be any more, connections are unique. + return; + } + } +} + void IsoStreamManager::AcceptCisRequest( const pw::bluetooth::emboss::LECISRequestSubeventView& event_view, CisEstablishedCallback cb) { diff --git a/pw_bluetooth_sapphire/host/iso/iso_stream_manager_test.cc b/pw_bluetooth_sapphire/host/iso/iso_stream_manager_test.cc index a3fa0b35fe..3b4fb23205 100644 --- a/pw_bluetooth_sapphire/host/iso/iso_stream_manager_test.cc +++ b/pw_bluetooth_sapphire/host/iso/iso_stream_manager_test.cc @@ -202,4 +202,60 @@ TEST_F(IsoStreamManagerTest, MultipleCISAcceptRequests) { ASSERT_TRUE(iso_stream_manager()->HandlerRegistered(kId1)); } +// We should be able to process channel disconnects, and detach the connection +// from the stream. +TEST_F(IsoStreamManagerTest, DisconnectCIS) { + // A disconnect event outside of any CIS doesn't affect us. + auto disconnect_packet = testing::DisconnectionCompletePacket(0x01); + test_device()->SendCommandChannelPacket(disconnect_packet); + RunUntilIdle(); + + const CigCisIdentifier kId1(0x14, 0x04); + + // Set up an existing connection. + auto le_accept_cis_complete_packet = testing::CommandCompletePacket( + hci_spec::kLEAcceptCISRequest, + pw::bluetooth::emboss::StatusCode::SUCCESS); + bool cb1_invoked; + EXPECT_EQ(CallAcceptCis(kId1, &cb1_invoked), AcceptCisStatus::kSuccess); + ASSERT_TRUE(iso_stream_manager()->HandlerRegistered(kId1)); + // Matching request arrives for kId1, accept it and stop waiting on it + const hci_spec::ConnectionHandle kAltCisHandleId = kCisHandleId + 1; + auto le_accept_cis_packet = + testing::LEAcceptCisRequestCommandPacket(kAltCisHandleId); + EXPECT_CMD_PACKET_OUT( + test_device(), le_accept_cis_packet, &le_accept_cis_complete_packet); + auto request_packet = testing::LECisRequestEventPacket( + kAclConnectionHandleId1, kAltCisHandleId, kId1.cig_id(), kId1.cis_id()); + test_device()->SendCommandChannelPacket(request_packet); + RunUntilIdle(); + ASSERT_FALSE(iso_stream_manager()->HandlerRegistered(kId1)); + // Establish the stream and make sure the correct callback is invoked + auto le_cis_established_packet = + LECisEstablishedPacketWithDefaultValues(kAltCisHandleId); + test_device()->SendCommandChannelPacket(le_cis_established_packet); + RunUntilIdle(); + EXPECT_TRUE(cb1_invoked); + + // Disconnecting a non-CIS stream at this point is also harmless. + test_device()->SendCommandChannelPacket(disconnect_packet); + RunUntilIdle(); + + // Disconnecting the CIS we have connected should have an effect. + auto disconnect_cis_packet = + testing::DisconnectionCompletePacket(kAltCisHandleId); + test_device()->SendCommandChannelPacket(disconnect_cis_packet); + RunUntilIdle(); + + // Since we received a disconnect, the stream should not be alive anymore (it + // was closed) + ASSERT_EQ(iso_streams_.count(kId1), 1u); + ASSERT_FALSE(iso_streams_[kId1].is_alive()); + + // Disconnecting again is harmless as well, even if it's a repeat of the CIS + // handle. + test_device()->SendCommandChannelPacket(disconnect_cis_packet); + RunUntilIdle(); +} + } // namespace bt::iso diff --git a/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/iso/iso_stream_manager.h b/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/iso/iso_stream_manager.h index 4c65238b7d..ce2aa18102 100644 --- a/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/iso/iso_stream_manager.h +++ b/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/iso/iso_stream_manager.h @@ -55,6 +55,9 @@ class IsoStreamManager final { // Process an incoming CIS request. Currently rejects all requests. void OnCisRequest(const hci::EmbossEventPacket& event); + // Process a disconnection and shut down a CIS. + void OnDisconnect(const hci::EmbossEventPacket& event); + void AcceptCisRequest( const pw::bluetooth::emboss::LECISRequestSubeventView& event_view, CisEstablishedCallback cb); @@ -67,6 +70,8 @@ class IsoStreamManager final { // LE event handler for incoming CIS requests hci::CommandChannel::EventHandlerId cis_request_handler_; + // Event handler for disconnect events + hci::CommandChannel::EventHandlerId disconnect_handler_; hci::CommandChannel::WeakPtr cmd_;