pw_bluetooth_sapphire: Remove CommandCompleteCallbackVariant

Only use the emboss version of CommandCompleteCallback in SequentialCommandRunner. Bug: b/42167863 Test: pw presubmit --step gn_chre_googletest_nanopb_sapphire_build Change-Id: I5ad610b31ab7642563d625b3ad6bc238df9f3bc8 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/247253 Docs-Not-Needed: Josh Conner <[email protected]> Lint: Lint 🤖 <[email protected]> Presubmit-Verified: CQ Bot Account <[email protected]> Reviewed-by: Ben Lawson <[email protected]> Commit-Queue: Josh Conner <[email protected]>
google · Nov 6, 2024 · 8e21d97 · 8e21d97
1 parent 05c13c4
commit 8e21d97
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 43 deletions.
diff --git a/pw_bluetooth_sapphire/host/gap/adapter.cc b/pw_bluetooth_sapphire/host/gap/adapter.cc
@@ -1364,7 +1364,7 @@ void AdapterImpl::InitializeStep3() {
                                  kConnectedIsochronousStreamHostSupport));
     params.bit_value().Write(pw::bluetooth::emboss::GenericEnableParam::ENABLE);
     init_seq_runner_->QueueCommand(
-        std::move(cmd_packet), [](const hci::EventPacket& event) {
+        std::move(cmd_packet), [](const hci::EmbossEventPacket& event) {
           hci_is_error(
               event, WARN, "gap", "Set Host Feature (ISO support) failed");
         });

diff --git a/pw_bluetooth_sapphire/host/hci/sequential_command_runner.cc b/pw_bluetooth_sapphire/host/hci/sequential_command_runner.cc
@@ -30,7 +30,7 @@ SequentialCommandRunner::SequentialCommandRunner(
 
 void SequentialCommandRunner::QueueCommand(
     EmbossCommandPacket command_packet,
-    CommandCompleteCallbackVariant callback,
+    EmbossCommandCompleteCallback callback,
     bool wait,
     hci_spec::EventCode complete_event_code,
     std::unordered_set<hci_spec::OpCode> exclusions) {
@@ -50,7 +50,7 @@ void SequentialCommandRunner::QueueCommand(
 void SequentialCommandRunner::QueueLeAsyncCommand(
     EmbossCommandPacket command_packet,
     hci_spec::EventCode le_meta_subevent_code,
-    CommandCompleteCallbackVariant callback,
+    EmbossCommandCompleteCallback callback,
     bool wait) {
   command_queue_.emplace(
       QueuedCommand{.packet = std::move(command_packet),
@@ -113,15 +113,10 @@ void SequentialCommandRunner::TryRunNextQueuedCommand(Result<> status) {
     std::optional<EmbossEventPacket> emboss_packet;
     hci::Result<> event_result =
         bt::ToResult(pw::bluetooth::emboss::StatusCode::SUCCESS);
-    using T = std::decay_t<decltype(cmd_cb)>;
-    if constexpr (std::is_same_v<T, CommandCompleteCallback>) {
-      event_result = event_packet.ToResult();
-    } else {
-      emboss_packet = EmbossEventPacket::New(event_packet.view().size());
-      MutableBufferView buffer = emboss_packet->mutable_data();
-      event_packet.view().data().Copy(&buffer);
-      event_result = emboss_packet->ToResult();
-    }
+    emboss_packet = EmbossEventPacket::New(event_packet.view().size());
+    MutableBufferView buffer = emboss_packet->mutable_data();
+    event_packet.view().data().Copy(&buffer);
+    event_result = emboss_packet->ToResult();
 
     if (self.is_alive() && seq_no != self->sequence_number_) {
       bt_log(TRACE,
@@ -146,21 +141,9 @@ void SequentialCommandRunner::TryRunNextQueuedCommand(Result<> status) {
       return;
     }
 
-    std::visit(
-        [&event_packet, &emboss_packet](auto& cmd_cb_arg) {
-          using cmd_cb_t = std::decay_t<decltype(cmd_cb_arg)>;
-          if constexpr (std::is_same_v<cmd_cb_t, CommandCompleteCallback>) {
-            if (cmd_cb_arg) {
-              cmd_cb_arg(event_packet);
-            }
-          } else if constexpr (std::is_same_v<cmd_cb_t,
-                                              EmbossCommandCompleteCallback>) {
-            if (cmd_cb_arg) {
-              cmd_cb_arg(*emboss_packet);
-            }
-          }
-        },
-        cmd_cb);
+    if (cmd_cb) {
+      cmd_cb(*emboss_packet);
+    }
 
     // The callback could have destroyed, canceled, or restarted the command
     // runner.  While this check looks redundant to the above check, the state

diff --git a/pw_bluetooth_sapphire/host/hci/sequential_command_runner_test.cc b/pw_bluetooth_sapphire/host/hci/sequential_command_runner_test.cc
@@ -114,7 +114,7 @@ TEST_F(SequentialCommandRunnerTest, SequentialCommandRunner) {
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) { cb_called++; };
+  auto cb = [&](const EmbossEventPacket& event) { cb_called++; };
 
   // Sequence 1 (test)
   SequentialCommandRunner cmd_runner(cmd_channel()->AsWeakPtr());
@@ -283,7 +283,7 @@ TEST_F(SequentialCommandRunnerTest, SequentialCommandRunnerCancel) {
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) { cb_called++; };
+  auto cb = [&](const EmbossEventPacket& event) { cb_called++; };
 
   // Sequence 1: Sequence will be cancelled after the first command.
   SequentialCommandRunner cmd_runner(cmd_channel()->AsWeakPtr());
@@ -321,7 +321,7 @@ TEST_F(SequentialCommandRunnerTest, SequentialCommandRunnerCancel) {
   cmd_runner.QueueCommand(
       EmbossCommandPacket::New<pw::bluetooth::emboss::CommandHeaderView>(
           kTestOpCode),
-      [&](const EventPacket& event) {
+      [&](const EmbossEventPacket& event) {
         bt_log(TRACE, "hci-test", "callback called");
         cmd_runner.Cancel();
         EXPECT_TRUE(cmd_runner.IsReady());
@@ -355,7 +355,7 @@ TEST_F(SequentialCommandRunnerTest, SequentialCommandRunnerCancel) {
   cmd_runner.QueueCommand(
       EmbossCommandPacket::New<pw::bluetooth::emboss::CommandHeaderView>(
           kTestOpCode),
-      [&](const EventPacket& event) {
+      [&](const EmbossEventPacket& event) {
         cmd_runner.Cancel();
         EXPECT_TRUE(cmd_runner.IsReady());
         EXPECT_FALSE(cmd_runner.HasQueuedCommands());
@@ -599,7 +599,7 @@ TEST_F(SequentialCommandRunnerTest, CommandCompletesOnStatusEvent) {
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) { cb_called++; };
+  auto cb = [&](const EmbossEventPacket& event) { cb_called++; };
 
   SequentialCommandRunner cmd_runner(cmd_channel()->AsWeakPtr());
   EXPECT_FALSE(cmd_runner.HasQueuedCommands());
@@ -663,7 +663,7 @@ TEST_F(SequentialCommandRunnerTest, AsyncCommands) {
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) { cb_called++; };
+  auto cb = [&](const EmbossEventPacket& event) { cb_called++; };
 
   SequentialCommandRunner cmd_runner(cmd_channel()->AsWeakPtr());
   EXPECT_FALSE(cmd_runner.HasQueuedCommands());
@@ -738,7 +738,7 @@ TEST_F(SequentialCommandRunnerTest, ExclusiveAsyncCommands) {
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) { cb_called++; };
+  auto cb = [&](const EmbossEventPacket& event) { cb_called++; };
 
   SequentialCommandRunner cmd_runner(cmd_channel()->AsWeakPtr());
   EXPECT_FALSE(cmd_runner.HasQueuedCommands());
@@ -809,7 +809,7 @@ TEST_F(SequentialCommandRunnerTest,
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) {
+  auto cb = [&](const EmbossEventPacket& event) {
     if (cb_called == 0) {
       cmd_runner.reset();
     }
@@ -861,7 +861,7 @@ TEST_F(SequentialCommandRunnerTest,
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) { cb_called++; };
+  auto cb = [&](const EmbossEventPacket& event) { cb_called++; };
 
   auto command = bt::testing::EmptyCommandPacket(kTestOpCode);
   cmd_runner->QueueCommand(
@@ -913,10 +913,10 @@ TEST_F(SequentialCommandRunnerTest, QueueCommandsWhileAlreadyRunning) {
   };
 
   int cb_called = 0;
-  auto cb = [&](const EventPacket& event) { cb_called++; };
+  auto cb = [&](const EmbossEventPacket& event) { cb_called++; };
 
   int name_cb_called = 0;
-  auto name_request_callback = [&](const EventPacket& event) {
+  auto name_request_callback = [&](const EmbossEventPacket& event) {
     name_cb_called++;
 
     EXPECT_FALSE(cmd_runner.IsReady());

diff --git a/...tooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/sequential_command_runner.h b/...tooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/sequential_command_runner.h
@@ -67,11 +67,9 @@ class SequentialCommandRunner final {
   using CommandCompleteCallback = fit::function<void(const EventPacket& event)>;
   using EmbossCommandCompleteCallback =
       fit::function<void(const EmbossEventPacket& event_packet)>;
-  using CommandCompleteCallbackVariant =
-      std::variant<CommandCompleteCallback, EmbossCommandCompleteCallback>;
   void QueueCommand(
       EmbossCommandPacket command_packet,
-      CommandCompleteCallbackVariant callback = CommandCompleteCallback(),
+      EmbossCommandCompleteCallback callback = EmbossCommandCompleteCallback(),
       bool wait = true,
       hci_spec::EventCode complete_event_code =
           hci_spec::kCommandCompleteEventCode,
@@ -82,7 +80,7 @@ class SequentialCommandRunner final {
   void QueueLeAsyncCommand(
       EmbossCommandPacket command_packet,
       hci_spec::EventCode le_meta_subevent_code,
-      CommandCompleteCallbackVariant callback = CommandCompleteCallback(),
+      EmbossCommandCompleteCallback callback = EmbossCommandCompleteCallback(),
       bool wait = true);
 
   // Runs all the queued commands. This method will return before queued
@@ -129,7 +127,7 @@ class SequentialCommandRunner final {
     EmbossCommandPacket packet;
     hci_spec::EventCode complete_event_code;
     bool is_le_async_command;
-    CommandCompleteCallbackVariant callback;
+    EmbossCommandCompleteCallback callback;
     bool wait;
     std::unordered_set<hci_spec::OpCode> exclusions;
   };