L2CAP: Fix SignallingManager request queuing

We should always wait for an outgoing request to be handled before
sending the next one. Fix the logic in queuing.

Test: run_cert_test.sh
Bug: 141557006
Change-Id: I3568a3dca4857f1ba4fa47618408f40e2f1f7d04

diff --git a/gd/l2cap/classic/cert/cert.cc b/gd/l2cap/classic/cert/cert.cc
index 4fedccc..2be2dd4 100644 (file)
--- a/gd/l2cap/classic/cert/cert.cc
+++ b/gd/l2cap/classic/cert/cert.cc
@@ -116,7 +116,7 @@ class L2capModuleCertService : public L2capModuleCert::Service {
       ::grpc::ServerContext* context, const ::bluetooth::l2cap::classic::cert::ConfigurationRequest* request,
       ::bluetooth::l2cap::classic::cert::SendConfigurationRequestResult* response) override {
     auto builder = ConfigurationRequestBuilder::Create(1, request->scid(), Continuation::END, {});
-    auto l2cap_builder = BasicFrameBuilder::Create(1, std::move(builder));
+    auto l2cap_builder = BasicFrameBuilder::Create(kClassicSignallingCid, std::move(builder));
     outgoing_packet_queue_.push(std::move(l2cap_builder));
     if (outgoing_packet_queue_.size() == 1) {
       acl_connection_->GetAclQueueEnd()->RegisterEnqueue(
@@ -127,8 +127,8 @@ class L2capModuleCertService : public L2capModuleCert::Service {
 
   ::grpc::Status SendDisconnectionRequest(::grpc::ServerContext* context, const cert::DisconnectionRequest* request,
                                           ::google::protobuf::Empty* response) override {
-    auto builder = DisconnectionRequestBuilder::Create(3, 0x40, 101);
-    auto l2cap_builder = BasicFrameBuilder::Create(1, std::move(builder));
+    auto builder = DisconnectionRequestBuilder::Create(3, request->dcid(), request->scid());
+    auto l2cap_builder = BasicFrameBuilder::Create(kClassicSignallingCid, std::move(builder));
     outgoing_packet_queue_.push(std::move(l2cap_builder));
     if (outgoing_packet_queue_.size() == 1) {
       acl_connection_->GetAclQueueEnd()->RegisterEnqueue(

diff --git a/gd/l2cap/classic/cert/simple_l2cap_test.py b/gd/l2cap/classic/cert/simple_l2cap_test.py
index e41a10a..3dfd0d7 100644 (file)
--- a/gd/l2cap/classic/cert/simple_l2cap_test.py
+++ b/gd/l2cap/classic/cert/simple_l2cap_test.py
@@ -113,28 +113,38 @@ class SimpleL2capTest(GdBaseTestClass):
             lambda packet: b"abc" in packet.payload
         )
 
-        self.cert_device.l2cap.SendDisconnectionRequest(l2cap_cert_pb2.DisconnectionRequest())
+        self.cert_device.l2cap.SendDisconnectionRequest(l2cap_cert_pb2.DisconnectionRequest(dcid=0x40, scid=101))
         time.sleep(1)
         dut_packet_stream.unsubscribe()
         cert_packet_stream.unsubscribe()
 
+    def test_open_two_channels(self):
+        cert_connection_stream = self.cert_device.l2cap.connection_complete_stream
+        cert_connection_stream.subscribe()
+        self.device_under_test.l2cap.OpenChannel(l2cap_facade_pb2.OpenChannelRequest(remote=self.cert_address, psm=0x01))
+        self.device_under_test.l2cap.OpenChannel(l2cap_facade_pb2.OpenChannelRequest(remote=self.cert_address, psm=0x03))
+        cert_connection_stream.assert_event_occurs(
+            lambda device: device.remote == self.dut_address
+        )
+        cert_connection_stream.unsubscribe()
+        time.sleep(1)
+        open_channels = self.cert_device.l2cap.FetchOpenedChannels(l2cap_cert_pb2.FetchOpenedChannelsRequest())
+        print (len(open_channels.cid))
+        assert len(open_channels.cid) == 2
+
     def test_basic_operation_request_connection(self):
         """
         L2CAP/COS/CED/BV-01-C [Request Connection]
         Verify that the IUT is able to request the connection establishment for an L2CAP data channel and
         initiate the configuration procedure.
         """
-        self.device_under_test.l2cap.RegisterChannel(l2cap_facade_pb2.RegisterChannelRequest(channel=2))
         cert_connection_stream = self.cert_device.l2cap.connection_complete_stream
         cert_connection_stream.subscribe()
-        self.device_under_test.l2cap.Connect(self.cert_address)
+        self.device_under_test.l2cap.OpenChannel(l2cap_facade_pb2.OpenChannelRequest(remote=self.cert_address, psm=0x01))
         cert_connection_stream.assert_event_occurs(
             lambda device: device.remote == self.dut_address
         )
         cert_connection_stream.unsubscribe()
-        self.device_under_test.l2cap.OpenChannel(l2cap_facade_pb2.OpenChannelRequest(remote=self.cert_address, psm=0x01))
-        time.sleep(1)
-
 
     def test_respond_to_echo_request(self):
         """

diff --git a/gd/l2cap/classic/internal/signalling_manager.cc b/gd/l2cap/classic/internal/signalling_manager.cc
index 44397f8..b06162c 100644 (file)
--- a/gd/l2cap/classic/internal/signalling_manager.cc
+++ b/gd/l2cap/classic/internal/signalling_manager.cc
@@ -52,8 +52,15 @@ ClassicSignallingManager::~ClassicSignallingManager() {
 }
 
 void ClassicSignallingManager::OnCommandReject(CommandRejectView command_reject_view) {
+  if (pending_commands_.empty()) {
+    LOG_WARN("Unexpected command reject: no pending request");
+    return;
+  }
+  auto last_sent_command = std::move(pending_commands_.front());
+  pending_commands_.pop();
+
   SignalId signal_id = command_reject_view.GetIdentifier();
-  if (last_sent_command_.signal_id_ != signal_id) {
+  if (last_sent_command.signal_id_ != signal_id) {
     LOG_WARN("Unknown command reject");
     return;
   }
@@ -147,20 +154,24 @@ void ClassicSignallingManager::OnConnectionRequest(SignalId signal_id, Psm psm,
 
 void ClassicSignallingManager::OnConnectionResponse(SignalId signal_id, Cid cid, Cid remote_cid,
                                                     ConnectionResponseResult result, ConnectionResponseStatus status) {
-  if (last_sent_command_.signal_id_ != signal_id ||
-      last_sent_command_.command_code_ != CommandCode::CONNECTION_REQUEST) {
+  if (pending_commands_.empty()) {
+    LOG_WARN("Unexpected response: no pending request");
+    return;
+  }
+  auto last_sent_command = std::move(pending_commands_.front());
+  pending_commands_.pop();
+  if (last_sent_command.signal_id_ != signal_id || last_sent_command.command_code_ != CommandCode::CONNECTION_REQUEST) {
     LOG_WARN("Received unexpected connection response");
     return;
   }
-  if (last_sent_command_.source_cid_ != cid) {
-    LOG_WARN("SCID doesn't match: expected %d, received %d", last_sent_command_.source_cid_, cid);
+  if (last_sent_command.source_cid_ != cid) {
+    LOG_WARN("SCID doesn't match: expected %d, received %d", last_sent_command.source_cid_, cid);
     return;
   }
   if (result != ConnectionResponseResult::SUCCESS) {
     return;
   }
-  Psm pending_psm = last_sent_command_.psm_;
-  last_sent_command_ = {};
+  Psm pending_psm = last_sent_command.psm_;
   auto new_channel = link_->AllocateReservedDynamicChannel(cid, pending_psm, remote_cid, {});
   if (new_channel == nullptr) {
     LOG_WARN("Can't allocate dynamic channel");
@@ -188,7 +199,15 @@ void ClassicSignallingManager::OnConfigurationRequest(SignalId signal_id, Cid ci
 
 void ClassicSignallingManager::OnConfigurationResponse(SignalId signal_id, Cid cid, Continuation is_continuation,
                                                        ConfigurationResponseResult result,
-                                                       std::vector<std::unique_ptr<ConfigurationOption>> option) {}
+                                                       std::vector<std::unique_ptr<ConfigurationOption>> option) {
+  if (pending_commands_.empty()) {
+    LOG_WARN("Unexpected response: no pending request");
+    return;
+  }
+
+  auto last_sent_command = std::move(pending_commands_.front());
+  pending_commands_.pop();
+}
 
 void ClassicSignallingManager::OnDisconnectionRequest(SignalId signal_id, Cid cid, Cid remote_cid) {
   // TODO: check cid match
@@ -204,8 +223,15 @@ void ClassicSignallingManager::OnDisconnectionRequest(SignalId signal_id, Cid ci
 }
 
 void ClassicSignallingManager::OnDisconnectionResponse(SignalId signal_id, Cid cid, Cid remote_cid) {
-  if (last_sent_command_.signal_id_ != signal_id ||
-      last_sent_command_.command_code_ != CommandCode::DISCONNECTION_REQUEST) {
+  if (pending_commands_.empty()) {
+    LOG_WARN("Unexpected response: no pending request");
+    return;
+  }
+  auto last_sent_command = std::move(pending_commands_.front());
+  pending_commands_.pop();
+
+  if (last_sent_command.signal_id_ != signal_id ||
+      last_sent_command.command_code_ != CommandCode::DISCONNECTION_REQUEST) {
     return;
   }
 
@@ -228,7 +254,14 @@ void ClassicSignallingManager::OnEchoRequest(SignalId signal_id, const PacketVie
 }
 
 void ClassicSignallingManager::OnEchoResponse(SignalId signal_id, const PacketView<kLittleEndian>& packet) {
-  if (last_sent_command_.signal_id_ != signal_id || last_sent_command_.command_code_ != CommandCode::ECHO_REQUEST) {
+  if (pending_commands_.empty()) {
+    LOG_WARN("Unexpected response: no pending request");
+    return;
+  }
+  auto last_sent_command = std::move(pending_commands_.front());
+  pending_commands_.pop();
+
+  if (last_sent_command.signal_id_ != signal_id || last_sent_command.command_code_ != CommandCode::ECHO_REQUEST) {
     return;
   }
   LOG_INFO("Echo response received");
@@ -259,8 +292,15 @@ void ClassicSignallingManager::OnInformationRequest(SignalId signal_id, Informat
 }
 
 void ClassicSignallingManager::OnInformationResponse(SignalId signal_id, const InformationResponseView& view) {
-  if (last_sent_command_.signal_id_ != signal_id ||
-      last_sent_command_.command_code_ != CommandCode::INFORMATION_REQUEST) {
+  if (pending_commands_.empty()) {
+    LOG_WARN("Unexpected response: no pending request");
+    return;
+  }
+  auto last_sent_command = std::move(pending_commands_.front());
+  pending_commands_.pop();
+
+  if (last_sent_command.signal_id_ != signal_id ||
+      last_sent_command.command_code_ != CommandCode::INFORMATION_REQUEST) {
     return;
   }
   if (view.GetResult() != InformationRequestResult::SUCCESS) {
@@ -395,16 +435,15 @@ void ClassicSignallingManager::handle_send_next_command() {
   if (pending_commands_.empty()) {
     return;
   }
-  last_sent_command_ = std::move(pending_commands_.front());
-  pending_commands_.pop();
-
-  auto signal_id = last_sent_command_.signal_id_;
-  auto psm = last_sent_command_.psm_;
-  auto source_cid = last_sent_command_.source_cid_;
-  auto destination_cid = last_sent_command_.destination_cid_;
-  auto info_type = last_sent_command_.info_type_;
-  auto config = std::move(last_sent_command_.config_);
-  switch (last_sent_command_.command_code_) {
+  auto& last_sent_command = pending_commands_.front();
+
+  auto signal_id = last_sent_command.signal_id_;
+  auto psm = last_sent_command.psm_;
+  auto source_cid = last_sent_command.source_cid_;
+  auto destination_cid = last_sent_command.destination_cid_;
+  auto info_type = last_sent_command.info_type_;
+  auto config = std::move(last_sent_command.config_);
+  switch (last_sent_command.command_code_) {
     case CommandCode::CONNECTION_REQUEST: {
       auto builder = ConnectionRequestBuilder::Create(signal_id.Value(), psm, source_cid);
       enqueue_buffer_->Enqueue(std::move(builder), handler_);
@@ -435,7 +474,7 @@ void ClassicSignallingManager::handle_send_next_command() {
       break;
     }
     default:
-      LOG_WARN("Unsupported command code 0x%x", static_cast<int>(last_sent_command_.command_code_));
+      LOG_WARN("Unsupported command code 0x%x", static_cast<int>(last_sent_command.command_code_));
   }
 }