pw_bluetooth_sapphire: Fix disabled flags in HCI layer

Change-Id: I6a19ef34f96c56f6d420d23c48bbce74159306d5 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/243265 Reviewed-by: Faraaz Sareshwala <[email protected]> Commit-Queue: Ben Lawson <[email protected]> Docs-Not-Needed: Ben Lawson <[email protected]> Lint: Lint 🤖 <[email protected]>
google · Oct 21, 2024 · e9391cf · e9391cf
1 parent e4d8db4
commit e9391cf
Show file tree

Hide file tree

Showing 15 changed files with 105 additions and 158 deletions.
diff --git a/pw_bluetooth_sapphire/host/hci/BUILD.bazel b/pw_bluetooth_sapphire/host/hci/BUILD.bazel
@@ -13,7 +13,6 @@
 # the License.
 
 load("@pigweed//pw_unit_test:pw_cc_test.bzl", "pw_cc_test")
-load("//pw_bluetooth_sapphire/host:variables.bzl", "COPTS")
 
 package(default_visibility = ["//visibility:public"])
 
@@ -39,12 +38,6 @@ cc_library(
         "sco_connection.cc",
         "sequential_command_runner.cc",
     ],
-    copts = [
-        "-Wno-unused-parameter",
-        "-Wno-deprecated-this-capture",
-        "-Wswitch-enum",
-        "-Wno-shadow",
-    ] + COPTS,
     deps = [
         "//pw_bluetooth_sapphire:public",
         "//pw_bluetooth_sapphire/host/transport",
@@ -62,7 +55,6 @@ cc_library(
         "fake_low_energy_connection.cc",
         "fake_sco_connection.cc",
     ],
-    copts = COPTS,
     deps = [
         ":hci",
         "//pw_bluetooth_sapphire:public",
@@ -71,37 +63,8 @@ cc_library(
     ],
 )
 
-cc_library(
-    name = "hci_test.lib",
-    testonly = True,
-    srcs = [
-        "advertising_handle_map_test.cc",
-        "connection_test.cc",
-        "extended_low_energy_advertiser_test.cc",
-        "extended_low_energy_scanner_test.cc",
-        "legacy_low_energy_advertiser_test.cc",
-        "legacy_low_energy_scanner_test.cc",
-        "low_energy_advertiser_test.cc",
-        "low_energy_connector_test.cc",
-        "low_energy_multiple_advertising_test.cc",
-        "low_energy_scanner_test.cc",
-        "sequential_command_runner_test.cc",
-    ],
-    copts = COPTS,
-    tags = ["manual"],
-    visibility = ["//visibility:public"],
-    deps = [
-        ":hci",
-        ":testing",
-        "//pw_bluetooth_sapphire/host/testing",
-        "//pw_bluetooth_sapphire/host/testing:gtest_main",
-        "@pigweed//pw_bluetooth:emboss_hci_test",
-    ],
-    alwayslink = 1,
-)
-
 pw_cc_test(
-    name = "hci-spec_test",
+    name = "hci_test",
     srcs = [
         "advertising_handle_map_test.cc",
         "connection_test.cc",
@@ -115,7 +78,6 @@ pw_cc_test(
         "low_energy_scanner_test.cc",
         "sequential_command_runner_test.cc",
     ],
-    copts = COPTS,
     test_main = "//pw_bluetooth_sapphire/host/testing:gtest_main",
     deps = [
         ":hci",

diff --git a/pw_bluetooth_sapphire/host/hci/BUILD.gn b/pw_bluetooth_sapphire/host/hci/BUILD.gn
@@ -17,10 +17,6 @@ import("$dir_pw_unit_test/test.gni")
 
 dir_public_hci = "../../public/pw_bluetooth_sapphire/internal/host/hci"
 
-config("copts") {
-  cflags_cc = [ "-Wno-shadow" ]
-}
-
 pw_source_set("hci") {
   public = [
     "$dir_public_hci/acl_connection.h",
@@ -69,13 +65,6 @@ pw_source_set("hci") {
 
   public_configs = [ "$dir_pw_bluetooth_sapphire:public_include_path" ]
 
-  remove_configs = [ "$dir_pw_build:internal_strict_warnings" ]
-  configs = [
-    ":copts",
-    "$dir_pw_build:internal_strict_warnings_core",
-    "$dir_pw_bluetooth_sapphire/host:copts",
-  ]
-
   deps = [ "$dir_pw_preprocessor" ]
 }
 
@@ -97,12 +86,6 @@ pw_source_set("testing") {
   deps = [ "$dir_pw_async:heap_dispatcher" ]
 
   public_deps = [ ":hci" ]
-
-  remove_configs = [ "$dir_pw_build:internal_strict_warnings" ]
-  configs = [
-    "$dir_pw_build:internal_strict_warnings_core",
-    "$dir_pw_bluetooth_sapphire/host:copts",
-  ]
 }
 
 pw_test("tests") {
@@ -120,8 +103,6 @@ pw_test("tests") {
     "sequential_command_runner_test.cc",
   ]
 
-  configs = [ "$dir_pw_bluetooth_sapphire/host:copts" ]
-
   deps = [
     ":testing",
     "$dir_pw_bluetooth:emboss_hci_test",

diff --git a/pw_bluetooth_sapphire/host/hci/android_extended_low_energy_advertiser.cc b/pw_bluetooth_sapphire/host/hci/android_extended_low_energy_advertiser.cc
@@ -299,15 +299,15 @@ void AndroidExtendedLowEnergyAdvertiser::StartAdvertising(
 
     op_queue_.push([this,
                     address,
-                    data = std::move(copied_data),
-                    scan_rsp = std::move(copied_scan_rsp),
-                    options,
+                    data_copy = std::move(copied_data),
+                    scan_rsp_copy = std::move(copied_scan_rsp),
+                    options_copy = options,
                     conn_cb = std::move(connect_callback),
                     result_cb = std::move(result_callback)]() mutable {
       StartAdvertising(address,
-                       data,
-                       scan_rsp,
-                       options,
+                       data_copy,
+                       scan_rsp_copy,
+                       options_copy,
                        std::move(conn_cb),
                        std::move(result_cb));
     });

diff --git a/pw_bluetooth_sapphire/host/hci/bredr_connection_request.cc b/pw_bluetooth_sapphire/host/hci/bredr_connection_request.cc
@@ -73,7 +73,7 @@ void BrEdrConnectionRequest::CreateConnection(
   auto complete_cb = [self,
                       timeout,
                       peer_id = peer_id_,
-                      on_command_fail = std::move(on_command_fail)](
+                      on_command_fail_cb = std::move(on_command_fail)](
                          auto, const EventPacket& event) {
     BT_DEBUG_ASSERT(event.event_code() == hci_spec::kCommandStatusEventCode);
 
@@ -82,7 +82,7 @@ void BrEdrConnectionRequest::CreateConnection(
 
     Result<> status = event.ToResult();
     if (status.is_error()) {
-      on_command_fail(status, peer_id);
+      on_command_fail_cb(status, peer_id);
     } else {
       // Both CommandChannel and the controller perform some scheduling, so log
       // when the controller finally acknowledges Create Connection to observe

diff --git a/pw_bluetooth_sapphire/host/hci/connection.cc b/pw_bluetooth_sapphire/host/hci/connection.cc
@@ -45,12 +45,11 @@ Connection::Connection(hci_spec::ConnectionHandle handle,
 
   auto disconn_complete_handler = [self = weak_self_.GetWeakPtr(),
                                    handle,
-                                   hci = hci_,
-                                   on_disconnection_complete =
+                                   on_disconnection_complete_cb =
                                        std::move(on_disconnection_complete)](
                                       const EmbossEventPacket& event) mutable {
     return Connection::OnDisconnectionComplete(
-        self, handle, event, std::move(on_disconnection_complete));
+        self, handle, event, std::move(on_disconnection_complete_cb));
   };
   hci_->command_channel()->AddEventHandler(
       hci_spec::kDisconnectionCompleteEventCode,

diff --git a/pw_bluetooth_sapphire/host/hci/extended_low_energy_advertiser.cc b/pw_bluetooth_sapphire/host/hci/extended_low_energy_advertiser.cc
@@ -497,16 +497,16 @@ void ExtendedLowEnergyAdvertiser::StartAdvertising(
     scan_rsp.Copy(&copied_scan_rsp);
 
     op_queue_.push([this,
-                    address,
-                    data = std::move(copied_data),
-                    scan_rsp = std::move(copied_scan_rsp),
-                    options,
+                    address_copy = address,
+                    data_copy = std::move(copied_data),
+                    scan_rsp_copy = std::move(copied_scan_rsp),
+                    options_copy = options,
                     conn_cb = std::move(connect_callback),
                     result_cb = std::move(result_callback)]() mutable {
-      StartAdvertising(address,
-                       data,
-                       scan_rsp,
-                       options,
+      StartAdvertising(address_copy,
+                       data_copy,
+                       scan_rsp_copy,
+                       options_copy,
                        std::move(conn_cb),
                        std::move(result_cb));
     });

diff --git a/pw_bluetooth_sapphire/host/hci/fake_local_address_delegate.cc b/pw_bluetooth_sapphire/host/hci/fake_local_address_delegate.cc
@@ -23,10 +23,10 @@ void FakeLocalAddressDelegate::EnsureLocalAddress(AddressCallback callback) {
     return;
   }
   (void)heap_dispatcher_.Post(
-      [callback = std::move(callback), addr = local_address_](
+      [cb = std::move(callback), addr = local_address_](
           pw::async::Context /*ctx*/, pw::Status status) {
         if (status.ok()) {
-          callback(addr);
+          cb(addr);
         }
       });
 }

diff --git a/pw_bluetooth_sapphire/host/hci/legacy_low_energy_advertiser.cc b/pw_bluetooth_sapphire/host/hci/legacy_low_energy_advertiser.cc
@@ -279,12 +279,12 @@ void LegacyLowEnergyAdvertiser::StartAdvertising(
           staged_params.options,
           std::move(staged_params.connect_callback),
           [this,
-           address = staged_params.address,
-           result_callback = std::move(staged_params.result_callback)](
-              const Result<>& result) {
+           address_copy = staged_params.address,
+           result_cb = std::move(staged_params.result_callback)](
+              const Result<>& start_result) {
             starting_ = false;
-            local_address_ = address;
-            result_callback(result);
+            local_address_ = address_copy;
+            result_cb(start_result);
           });
     };
 
@@ -299,11 +299,11 @@ void LegacyLowEnergyAdvertiser::StartAdvertising(
       scan_rsp,
       options,
       std::move(connect_callback),
-      [this, address, result_callback = std::move(result_callback)](
-          const Result<>& result) {
+      [this, address_copy = address, result_cb = std::move(result_callback)](
+          const Result<>& start_result) {
         starting_ = false;
-        local_address_ = address;
-        result_callback(result);
+        local_address_ = address_copy;
+        result_cb(start_result);
       });
 }
 

diff --git a/pw_bluetooth_sapphire/host/hci/low_energy_advertiser.cc b/pw_bluetooth_sapphire/host/hci/low_energy_advertiser.cc
@@ -260,29 +260,27 @@ void LowEnergyAdvertiser::StartAdvertisingInternal(
   // of the SetAdvertisingParams HCI command, we place the remaining advertising
   // setup HCI commands in the result callback here. SequentialCommandRunner
   // doesn't allow enqueuing commands within a callback (during a run).
-  hci_cmd_runner_->RunCommands([this,
-                                address,
-                                options,
-                                result_callback = std::move(result_callback),
-                                connect_callback = std::move(connect_callback)](
-                                   hci::Result<> result) mutable {
-    if (bt_is_error(result,
-                    WARN,
-                    "hci-le",
-                    "failed to start advertising for %s",
-                    bt_str(address))) {
-      result_callback(result);
-      return;
-    }
-
-    bool success = StartAdvertisingInternalStep2(address,
-                                                 options,
-                                                 std::move(connect_callback),
-                                                 std::move(result_callback));
-    if (!success) {
-      result_callback(ToResult(HostError::kCanceled));
-    }
-  });
+  hci_cmd_runner_->RunCommands(
+      [this,
+       address,
+       options,
+       result_cb = std::move(result_callback),
+       connect_cb = std::move(connect_callback)](hci::Result<> result) mutable {
+        if (bt_is_error(result,
+                        WARN,
+                        "hci-le",
+                        "failed to start advertising for %s",
+                        bt_str(address))) {
+          result_cb(result);
+          return;
+        }
+
+        bool success = StartAdvertisingInternalStep2(
+            address, options, std::move(connect_cb), std::move(result_cb));
+        if (!success) {
+          result_cb(ToResult(HostError::kCanceled));
+        }
+      });
 }
 
 bool LowEnergyAdvertiser::StartAdvertisingInternalStep2(
@@ -313,20 +311,19 @@ bool LowEnergyAdvertiser::StartAdvertisingInternalStep2(
   hci_cmd_runner_->RunCommands([this,
                                 address,
                                 extended_pdu = options.extended_pdu,
-                                result_callback = std::move(result_callback),
-                                connect_callback = std::move(connect_callback)](
+                                result_cb = std::move(result_callback),
+                                connect_cb = std::move(connect_callback)](
                                    Result<> result) mutable {
     if (!bt_is_error(result,
                      WARN,
                      "hci-le",
                      "failed to start advertising for %s",
                      bt_str(address))) {
       bt_log(INFO, "hci-le", "advertising enabled for %s", bt_str(address));
-      connection_callbacks_[{address, extended_pdu}] =
-          std::move(connect_callback);
+      connection_callbacks_[{address, extended_pdu}] = std::move(connect_cb);
     }
 
-    result_callback(result);
+    result_cb(result);
     OnCurrentOperationComplete();
   });
 

diff --git a/pw_bluetooth_sapphire/host/hci/low_energy_connection.cc b/pw_bluetooth_sapphire/host/hci/low_energy_connection.cc
@@ -148,17 +148,18 @@ LowEnergyConnection::OnLELongTermKeyRequestEvent(
     return CommandChannel::EventCallbackResult::kRemove;
   }
 
-  auto status_cb = [](auto id, const EventPacket& event) {
-    hci_is_error(event, TRACE, "hci-le", "failed to reply to LTK request");
+  auto status_cb = [](auto id, const EventPacket& status_event) {
+    hci_is_error(
+        status_event, TRACE, "hci-le", "failed to reply to LTK request");
   };
 
   if (ltk() && ltk()->rand() == rand && ltk()->ediv() == ediv) {
     auto cmd = EmbossCommandPacket::New<
         pw::bluetooth::emboss::LELongTermKeyRequestReplyCommandWriter>(
         hci_spec::kLELongTermKeyRequestReply);
-    auto view = cmd.view_t();
-    view.connection_handle().Write(handle);
-    view.long_term_key().CopyFrom(
+    auto cmd_view = cmd.view_t();
+    cmd_view.connection_handle().Write(handle);
+    cmd_view.long_term_key().CopyFrom(
         pw::bluetooth::emboss::LinkKeyView(&ltk()->value()));
     hci()->command_channel()->SendCommand(cmd, std::move(status_cb));
   } else {
@@ -167,8 +168,8 @@ LowEnergyConnection::OnLELongTermKeyRequestEvent(
     auto cmd = EmbossCommandPacket::New<
         pw::bluetooth::emboss::LELongTermKeyRequestNegativeReplyCommandWriter>(
         hci_spec::kLELongTermKeyRequestNegativeReply);
-    auto view = cmd.view_t();
-    view.connection_handle().Write(handle);
+    auto cmd_view = cmd.view_t();
+    cmd_view.connection_handle().Write(handle);
     hci()->command_channel()->SendCommand(cmd, std::move(status_cb));
   }
 

diff --git a/pw_bluetooth_sapphire/host/hci/low_energy_connector.cc b/pw_bluetooth_sapphire/host/hci/low_energy_connector.cc
@@ -37,8 +37,9 @@ using pw::bluetooth::emboss::LEPeerAddressType;
 using pw::bluetooth::emboss::StatusCode;
 
 LowEnergyConnector::PendingRequest::PendingRequest(
-    const DeviceAddress& peer_address, StatusCallback status_callback)
-    : peer_address(peer_address), status_callback(std::move(status_callback)) {}
+    const DeviceAddress& init_peer_address, StatusCallback init_status_callback)
+    : peer_address(init_peer_address),
+      status_callback(std::move(init_status_callback)) {}
 
 LowEnergyConnector::LowEnergyConnector(
     Transport::WeakPtr hci,
@@ -123,8 +124,15 @@ bool LowEnergyConnector::CreateConnection(
   }
 
   local_addr_delegate_->EnsureLocalAddress(
-      [=, callback = std::move(status_callback)](
-          const DeviceAddress& address) mutable {
+      [this,
+       use_accept_list,
+       peer_address,
+       scan_interval,
+       scan_window,
+       initial_parameters,
+       timeout,
+       callback =
+           std::move(status_callback)](const DeviceAddress& address) mutable {
         CreateConnectionInternal(address,
                                  use_accept_list,
                                  peer_address,