L2CAP ERTM: Close channel on error
Add a reference to ILink to send disconnection request.
Bug: 145006212
Test: bluetooth_test_gd
Change-Id: I2214cc0f69d7a1281e92f60c92a23484f4f3766d
diff --git a/gd/l2cap/classic/internal/link.cc b/gd/l2cap/classic/internal/link.cc
index a23f5ec..ad8a51c 100644
--- a/gd/l2cap/classic/internal/link.cc
+++ b/gd/l2cap/classic/internal/link.cc
@@ -34,8 +34,9 @@
DynamicChannelServiceManagerImpl* dynamic_service_manager,
FixedChannelServiceManagerImpl* fixed_service_manager)
: l2cap_handler_(l2cap_handler), acl_connection_(std::move(acl_connection)),
- data_pipeline_manager_(l2cap_handler, acl_connection_->GetAclQueueEnd()), parameter_provider_(parameter_provider),
- dynamic_service_manager_(dynamic_service_manager), fixed_service_manager_(fixed_service_manager),
+ data_pipeline_manager_(l2cap_handler, this, acl_connection_->GetAclQueueEnd()),
+ parameter_provider_(parameter_provider), dynamic_service_manager_(dynamic_service_manager),
+ fixed_service_manager_(fixed_service_manager),
signalling_manager_(l2cap_handler_, this, &data_pipeline_manager_, dynamic_service_manager_,
&dynamic_channel_allocator_, fixed_service_manager_) {
ASSERT(l2cap_handler_ != nullptr);
diff --git a/gd/l2cap/classic/internal/signalling_manager.h b/gd/l2cap/classic/internal/signalling_manager.h
index 81c8c31..3ae95dd 100644
--- a/gd/l2cap/classic/internal/signalling_manager.h
+++ b/gd/l2cap/classic/internal/signalling_manager.h
@@ -112,7 +112,6 @@
l2cap::internal::DynamicChannelAllocator* channel_allocator_;
FixedChannelServiceManagerImpl* fixed_service_manager_;
std::unique_ptr<os::EnqueueBuffer<packet::BasePacketBuilder>> enqueue_buffer_;
- PendingCommand last_sent_command_;
std::queue<PendingCommand> pending_commands_;
os::Alarm alarm_;
SignalId next_signal_id_ = kInitialSignalId;
diff --git a/gd/l2cap/internal/data_pipeline_manager.cc b/gd/l2cap/internal/data_pipeline_manager.cc
index 1c8d973..674df95 100644
--- a/gd/l2cap/internal/data_pipeline_manager.cc
+++ b/gd/l2cap/internal/data_pipeline_manager.cc
@@ -29,7 +29,7 @@
void DataPipelineManager::AttachChannel(Cid cid, std::shared_ptr<ChannelImpl> channel) {
ASSERT(sender_map_.find(cid) == sender_map_.end());
sender_map_.emplace(std::piecewise_construct, std::forward_as_tuple(cid),
- std::forward_as_tuple(handler_, scheduler_.get(), channel));
+ std::forward_as_tuple(handler_, link_, scheduler_.get(), channel));
}
void DataPipelineManager::DetachChannel(Cid cid) {
diff --git a/gd/l2cap/internal/data_pipeline_manager.h b/gd/l2cap/internal/data_pipeline_manager.h
index fffbfc7..557a741 100644
--- a/gd/l2cap/internal/data_pipeline_manager.h
+++ b/gd/l2cap/internal/data_pipeline_manager.h
@@ -39,11 +39,12 @@
namespace bluetooth {
namespace l2cap {
namespace internal {
+class ILink;
/**
* Manages data pipeline from channel queue end to link queue end, per link.
* Contains a Scheduler and Receiver per link.
- * Contains a Sender and its corrsponding DataController per attached channel.
+ * Contains a Sender and its corresponding DataController per attached channel.
*/
class DataPipelineManager {
public:
@@ -54,8 +55,8 @@
using LowerDequeue = UpperEnqueue;
using LowerQueueUpEnd = common::BidiQueueEnd<LowerEnqueue, LowerDequeue>;
- DataPipelineManager(os::Handler* handler, LowerQueueUpEnd* link_queue_up_end)
- : handler_(handler), scheduler_(std::make_unique<Fifo>(this, link_queue_up_end, handler)),
+ DataPipelineManager(os::Handler* handler, ILink* link, LowerQueueUpEnd* link_queue_up_end)
+ : handler_(handler), link_(link), scheduler_(std::make_unique<Fifo>(this, link_queue_up_end, handler)),
receiver_(link_queue_up_end, handler, this) {}
virtual void AttachChannel(Cid cid, std::shared_ptr<ChannelImpl> channel);
@@ -67,6 +68,7 @@
private:
os::Handler* handler_;
+ ILink* link_;
std::unique_ptr<Scheduler> scheduler_;
Receiver receiver_;
std::unordered_map<Cid, Sender> sender_map_;
diff --git a/gd/l2cap/internal/data_pipeline_manager_mock.h b/gd/l2cap/internal/data_pipeline_manager_mock.h
index f5a24f4..9c2f6c8 100644
--- a/gd/l2cap/internal/data_pipeline_manager_mock.h
+++ b/gd/l2cap/internal/data_pipeline_manager_mock.h
@@ -32,7 +32,7 @@
class MockDataPipelineManager : public DataPipelineManager {
public:
MockDataPipelineManager(os::Handler* handler, LowerQueueUpEnd* link_queue_up_end)
- : DataPipelineManager(handler, link_queue_up_end) {}
+ : DataPipelineManager(handler, nullptr, link_queue_up_end) {}
MOCK_METHOD(void, AttachChannel, (Cid, std::shared_ptr<ChannelImpl>), (override));
MOCK_METHOD(void, DetachChannel, (Cid), (override));
MOCK_METHOD(DataController*, GetDataController, (Cid), (override));
diff --git a/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.cc b/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.cc
index 97f9702..13d31ea 100644
--- a/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.cc
+++ b/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.cc
@@ -21,6 +21,7 @@
#include <vector>
#include "common/bind.h"
+#include "l2cap/internal/ilink.h"
#include "os/alarm.h"
#include "packet/fragmenting_inserter.h"
#include "packet/raw_builder.h"
@@ -28,10 +29,10 @@
namespace bluetooth {
namespace l2cap {
namespace internal {
-ErtmController::ErtmController(Cid cid, Cid remote_cid, UpperQueueDownEnd* channel_queue_end, os::Handler* handler,
- Scheduler* scheduler)
- : cid_(cid), remote_cid_(remote_cid), enqueue_buffer_(channel_queue_end), handler_(handler), scheduler_(scheduler),
- pimpl_(std::make_unique<impl>(this, handler)) {}
+ErtmController::ErtmController(ILink* link, Cid cid, Cid remote_cid, UpperQueueDownEnd* channel_queue_end,
+ os::Handler* handler, Scheduler* scheduler)
+ : link_(link), cid_(cid), remote_cid_(remote_cid), enqueue_buffer_(channel_queue_end), handler_(handler),
+ scheduler_(scheduler), pimpl_(std::make_unique<impl>(this, handler)) {}
ErtmController::~ErtmController() = default;
@@ -770,7 +771,7 @@
}
void CloseChannel() {
- // TODO: Needs a reference to signaller
+ controller_->close_channel();
}
void pop_srej_list() {
@@ -1005,7 +1006,7 @@
}
void ErtmController::close_channel() {
- // TODO: Get a reference to signalling manager
+ link_->SendDisconnectionRequest(cid_, remote_cid_);
}
size_t ErtmController::CopyablePacketBuilder::size() const {
diff --git a/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.h b/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.h
index dd1ca64..184d6c1 100644
--- a/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.h
+++ b/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller.h
@@ -36,13 +36,14 @@
namespace bluetooth {
namespace l2cap {
namespace internal {
+class ILink;
class ErtmController : public DataController {
public:
using UpperEnqueue = packet::PacketView<packet::kLittleEndian>;
using UpperDequeue = packet::BasePacketBuilder;
using UpperQueueDownEnd = common::BidiQueueEnd<UpperEnqueue, UpperDequeue>;
- ErtmController(Cid cid, Cid remote_cid, UpperQueueDownEnd* channel_queue_end, os::Handler* handler,
+ ErtmController(ILink* link, Cid cid, Cid remote_cid, UpperQueueDownEnd* channel_queue_end, os::Handler* handler,
Scheduler* scheduler);
~ErtmController();
// Segmentation is handled here
@@ -53,6 +54,7 @@
void SetRetransmissionAndFlowControlOptions(const RetransmissionAndFlowControlConfigurationOption& option) override;
private:
+ ILink* link_;
Cid cid_;
Cid remote_cid_;
os::EnqueueBuffer<UpperEnqueue> enqueue_buffer_;
diff --git a/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller_test.cc b/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller_test.cc
index e1b5818..f813898 100644
--- a/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller_test.cc
+++ b/gd/l2cap/internal/enhanced_retransmission_mode_channel_data_controller_test.cc
@@ -18,6 +18,7 @@
#include <gtest/gtest.h>
+#include "l2cap/internal/ilink_mock.h"
#include "l2cap/internal/scheduler_mock.h"
#include "l2cap/l2cap_packets.h"
#include "packet/raw_builder.h"
@@ -73,7 +74,8 @@
TEST_F(ErtmDataControllerTest, transmit_no_fcs) {
common::BidiQueue<Scheduler::UpperEnqueue, Scheduler::UpperDequeue> channel_queue{10};
testing::MockScheduler scheduler;
- ErtmController controller{1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
+ testing::MockILink link;
+ ErtmController controller{&link, 1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
EXPECT_CALL(scheduler, OnPacketsReady(1, 1));
controller.OnSdu(CreateSdu({'a', 'b', 'c', 'd'}));
auto next_packet = controller.GetNextPacket();
@@ -95,7 +97,8 @@
TEST_F(ErtmDataControllerTest, receive_no_fcs) {
common::BidiQueue<Scheduler::UpperEnqueue, Scheduler::UpperDequeue> channel_queue{10};
testing::MockScheduler scheduler;
- ErtmController controller{1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
+ testing::MockILink link;
+ ErtmController controller{&link, 1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
auto segment = CreateSdu({'a', 'b', 'c', 'd'});
auto builder = EnhancedInformationFrameBuilder::Create(1, 0, Final::NOT_SET, 0,
SegmentationAndReassembly::UNSEGMENTED, std::move(segment));
@@ -111,7 +114,8 @@
TEST_F(ErtmDataControllerTest, reassemble_valid_sdu) {
common::BidiQueue<Scheduler::UpperEnqueue, Scheduler::UpperDequeue> channel_queue{10};
testing::MockScheduler scheduler;
- ErtmController controller{1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
+ testing::MockILink link;
+ ErtmController controller{&link, 1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
auto segment1 = CreateSdu({'a'});
auto segment2 = CreateSdu({'b', 'c'});
auto segment3 = CreateSdu({'d', 'e', 'f'});
@@ -133,10 +137,11 @@
EXPECT_EQ(data, "abcdef");
}
-TEST_F(ErtmDataControllerTest, reassemble_invalid_sdu_size_in_start_frame) {
+TEST_F(ErtmDataControllerTest, reassemble_invalid_sdu_size_in_start_frame_will_disconnect) {
common::BidiQueue<Scheduler::UpperEnqueue, Scheduler::UpperDequeue> channel_queue{10};
testing::MockScheduler scheduler;
- ErtmController controller{1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
+ testing::MockILink link;
+ ErtmController controller{&link, 1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
auto segment1 = CreateSdu({'a'});
auto segment2 = CreateSdu({'b', 'c'});
auto segment3 = CreateSdu({'d', 'e', 'f'});
@@ -150,6 +155,7 @@
auto builder3 = EnhancedInformationFrameBuilder::Create(1, 2, Final::NOT_SET, 0, SegmentationAndReassembly::END,
std::move(segment3));
base_view = GetPacketView(std::move(builder3));
+ EXPECT_CALL(link, SendDisconnectionRequest(1, 1));
controller.OnPdu(base_view);
sync_handler(queue_handler_);
auto payload = channel_queue.GetUpEnd()->TryDequeue();
@@ -159,7 +165,8 @@
TEST_F(ErtmDataControllerTest, transmit_with_fcs) {
common::BidiQueue<Scheduler::UpperEnqueue, Scheduler::UpperDequeue> channel_queue{10};
testing::MockScheduler scheduler;
- ErtmController controller{1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
+ testing::MockILink link;
+ ErtmController controller{&link, 1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
controller.EnableFcs(true);
EXPECT_CALL(scheduler, OnPacketsReady(1, 1));
controller.OnSdu(CreateSdu({'a', 'b', 'c', 'd'}));
@@ -182,7 +189,8 @@
TEST_F(ErtmDataControllerTest, receive_packet_with_fcs) {
common::BidiQueue<Scheduler::UpperEnqueue, Scheduler::UpperDequeue> channel_queue{10};
testing::MockScheduler scheduler;
- ErtmController controller{1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
+ testing::MockILink link;
+ ErtmController controller{&link, 1, 1, channel_queue.GetDownEnd(), queue_handler_, &scheduler};
controller.EnableFcs(true);
auto segment = CreateSdu({'a', 'b', 'c', 'd'});
auto builder = EnhancedInformationFrameWithFcsBuilder::Create(
diff --git a/gd/l2cap/internal/ilink_mock.h b/gd/l2cap/internal/ilink_mock.h
new file mode 100644
index 0000000..15c642b
--- /dev/null
+++ b/gd/l2cap/internal/ilink_mock.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include "l2cap/internal/ilink.h"
+
+#include <gmock/gmock.h>
+
+// Unit test interfaces
+namespace bluetooth {
+namespace l2cap {
+namespace internal {
+namespace testing {
+
+class MockILink : public ILink {
+ public:
+ MOCK_METHOD(hci::AddressWithType, GetDevice, (), (override));
+ MOCK_METHOD(void, SendDisconnectionRequest, (Cid, Cid), (override));
+};
+
+} // namespace testing
+} // namespace internal
+} // namespace l2cap
+} // namespace bluetooth
diff --git a/gd/l2cap/internal/sender.cc b/gd/l2cap/internal/sender.cc
index 1c146cf..d4884b2 100644
--- a/gd/l2cap/internal/sender.cc
+++ b/gd/l2cap/internal/sender.cc
@@ -28,9 +28,9 @@
namespace l2cap {
namespace internal {
-Sender::Sender(os::Handler* handler, Scheduler* scheduler, std::shared_ptr<ChannelImpl> channel)
- : handler_(handler), queue_end_(channel->GetQueueDownEnd()), scheduler_(scheduler), channel_id_(channel->GetCid()),
- remote_channel_id_(channel->GetRemoteCid()),
+Sender::Sender(os::Handler* handler, ILink* link, Scheduler* scheduler, std::shared_ptr<ChannelImpl> channel)
+ : handler_(handler), link_(link), queue_end_(channel->GetQueueDownEnd()), scheduler_(scheduler),
+ channel_id_(channel->GetCid()), remote_channel_id_(channel->GetRemoteCid()),
data_controller_(std::make_unique<BasicModeDataController>(channel_id_, remote_channel_id_, queue_end_, handler_,
scheduler_)) {
try_register_dequeue();
@@ -82,7 +82,7 @@
}
if (mode == RetransmissionAndFlowControlModeOption::ENHANCED_RETRANSMISSION) {
data_controller_ =
- std::make_unique<ErtmController>(channel_id_, remote_channel_id_, queue_end_, handler_, scheduler_);
+ std::make_unique<ErtmController>(link_, channel_id_, remote_channel_id_, queue_end_, handler_, scheduler_);
data_controller_->SetRetransmissionAndFlowControlOptions(config.local_retransmission_and_flow_control_);
data_controller_->EnableFcs(config.fcs_type_ == FcsType::DEFAULT);
return;
diff --git a/gd/l2cap/internal/sender.h b/gd/l2cap/internal/sender.h
index 46e0006..0e8e78d 100644
--- a/gd/l2cap/internal/sender.h
+++ b/gd/l2cap/internal/sender.h
@@ -37,6 +37,7 @@
namespace l2cap {
namespace internal {
class Scheduler;
+class ILink;
/**
* A middle layer between L2CAP channel and outgoing packet scheduler.
@@ -48,7 +49,7 @@
using UpperDequeue = packet::BasePacketBuilder;
using UpperQueueDownEnd = common::BidiQueueEnd<UpperEnqueue, UpperDequeue>;
- Sender(os::Handler* handler, Scheduler* scheduler, std::shared_ptr<ChannelImpl> channel);
+ Sender(os::Handler* handler, ILink* link, Scheduler* scheduler, std::shared_ptr<ChannelImpl> channel);
~Sender();
/**
@@ -67,6 +68,7 @@
private:
os::Handler* handler_;
+ ILink* link_;
UpperQueueDownEnd* queue_end_;
Scheduler* scheduler_;
const Cid channel_id_;
diff --git a/gd/l2cap/internal/sender_test.cc b/gd/l2cap/internal/sender_test.cc
index cecdfa4..bd24fd4 100644
--- a/gd/l2cap/internal/sender_test.cc
+++ b/gd/l2cap/internal/sender_test.cc
@@ -77,7 +77,7 @@
EXPECT_CALL(*mock_channel_, GetQueueDownEnd()).WillRepeatedly(Return(channel_queue_.GetDownEnd()));
EXPECT_CALL(*mock_channel_, GetCid()).WillRepeatedly(Return(cid_));
EXPECT_CALL(*mock_channel_, GetRemoteCid()).WillRepeatedly(Return(cid_));
- sender_ = new Sender(queue_handler_, &scheduler_, mock_channel_);
+ sender_ = new Sender(queue_handler_, nullptr, &scheduler_, mock_channel_);
}
void TearDown() override {
diff --git a/gd/l2cap/le/internal/link.h b/gd/l2cap/le/internal/link.h
index 2807353..945f54a 100644
--- a/gd/l2cap/le/internal/link.h
+++ b/gd/l2cap/le/internal/link.h
@@ -37,7 +37,7 @@
Link(os::Handler* l2cap_handler, std::unique_ptr<hci::AclConnection> acl_connection,
l2cap::internal::ParameterProvider* parameter_provider)
: l2cap_handler_(l2cap_handler), acl_connection_(std::move(acl_connection)),
- data_pipeline_manager_(l2cap_handler, acl_connection_->GetAclQueueEnd()),
+ data_pipeline_manager_(l2cap_handler, this, acl_connection_->GetAclQueueEnd()),
parameter_provider_(parameter_provider) {
ASSERT(l2cap_handler_ != nullptr);
ASSERT(acl_connection_ != nullptr);
