diff --git a/pw_bluetooth_sapphire/host/hci/BUILD.bazel b/pw_bluetooth_sapphire/host/hci/BUILD.bazel index e99ce7d9d1..040ae4d95b 100644 --- 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 index 48c74e4603..f642b47e6d 100644 --- 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 index 531f2ca6ab..e360cd0f87 100644 --- 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 index 9f0227adf9..c12615694b 100644 --- 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 index ccea788a68..1dfbaed66a 100644 --- 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 index c1a436864e..59ae7d9ed0 100644 --- 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 index 2550e59a69..a987112bc4 100644 --- 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 index 3349a84f7e..acb87c26ee 100644 --- 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 index 5204c1e1fd..7e94f9e706 100644 --- 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,8 +311,8 @@ 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, @@ -322,11 +320,10 @@ bool LowEnergyAdvertiser::StartAdvertisingInternalStep2( "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 index e1c00f1f53..606f26197b 100644 --- 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(<k()->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 index afa76d9a68..c366236299 100644 --- 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, diff --git a/pw_bluetooth_sapphire/host/hci/low_energy_scanner.cc b/pw_bluetooth_sapphire/host/hci/low_energy_scanner.cc index 5102f22374..de8f0e8dea 100644 --- a/pw_bluetooth_sapphire/host/hci/low_energy_scanner.cc +++ b/pw_bluetooth_sapphire/host/hci/low_energy_scanner.cc @@ -69,10 +69,10 @@ LowEnergyScanner::PendingScanResult::PendingScanResult( fit::closure timeout_handler) : result_(result), timeout_(timeout), timeout_task_(dispatcher) { timeout_task_.set_function( - [timeout_handler = std::move(timeout_handler)](pw::async::Context /*ctx*/, - pw::Status status) { + [timeout_handler_cb = std::move(timeout_handler)]( + pw::async::Context /*ctx*/, pw::Status status) { if (status.ok()) { - timeout_handler(); + timeout_handler_cb(); } }); StartTimer(); @@ -141,9 +141,8 @@ bool LowEnergyScanner::StartScan(const ScanOptions& options, // Obtain the local address type. local_addr_delegate_->EnsureLocalAddress( - [this, options, callback = std::move(callback)]( - const auto& address) mutable { - StartScanInternal(address, options, std::move(callback)); + [this, options, cb = std::move(callback)](const auto& address) mutable { + StartScanInternal(address, options, std::move(cb)); }); return true; diff --git a/pw_bluetooth_sapphire/host/hci/sequential_command_runner.cc b/pw_bluetooth_sapphire/host/hci/sequential_command_runner.cc index 4db93eaff0..eade4786e7 100644 --- a/pw_bluetooth_sapphire/host/hci/sequential_command_runner.cc +++ b/pw_bluetooth_sapphire/host/hci/sequential_command_runner.cc @@ -111,16 +111,16 @@ void SequentialCommandRunner::TryRunNextQueuedCommand(Result<> status) { seq_no = sequence_number_]( auto, const EventPacket& event_packet) { std::optional emboss_packet; - hci::Result<> status = + hci::Result<> event_result = bt::ToResult(pw::bluetooth::emboss::StatusCode::SUCCESS); using T = std::decay_t; if constexpr (std::is_same_v) { - status = event_packet.ToResult(); + 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); - status = emboss_packet->ToResult(); + event_result = emboss_packet->ToResult(); } if (self.is_alive() && seq_no != self->sequence_number_) { @@ -129,7 +129,7 @@ void SequentialCommandRunner::TryRunNextQueuedCommand(Result<> status) { "Ignoring event for previous sequence (event code: %#.2x, status: " "%s)", event_packet.event_code(), - bt_str(status)); + bt_str(event_result)); } // The sequence could have failed or been canceled, and a new sequence could @@ -140,23 +140,23 @@ void SequentialCommandRunner::TryRunNextQueuedCommand(Result<> status) { return; } - if (status.is_ok() && + if (event_result.is_ok() && event_packet.event_code() == hci_spec::kCommandStatusEventCode && complete_event_code != hci_spec::kCommandStatusEventCode) { return; } std::visit( - [&event_packet, &emboss_packet](auto& cmd_cb) { - using cmd_cb_t = std::decay_t; + [&event_packet, &emboss_packet](auto& cmd_cb_arg) { + using cmd_cb_t = std::decay_t; if constexpr (std::is_same_v) { - if (cmd_cb) { - cmd_cb(event_packet); + if (cmd_cb_arg) { + cmd_cb_arg(event_packet); } } else if constexpr (std::is_same_v) { - if (cmd_cb) { - cmd_cb(*emboss_packet); + if (cmd_cb_arg) { + cmd_cb_arg(*emboss_packet); } } }, @@ -172,7 +172,7 @@ void SequentialCommandRunner::TryRunNextQueuedCommand(Result<> status) { BT_DEBUG_ASSERT(self->running_commands_ > 0); self->running_commands_--; - self->TryRunNextQueuedCommand(status); + self->TryRunNextQueuedCommand(event_result); }; running_commands_++; diff --git a/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/bredr_connection_request.h b/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/bredr_connection_request.h index de65172f9d..83f2e11121 100644 --- a/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/bredr_connection_request.h +++ b/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/bredr_connection_request.h @@ -58,7 +58,7 @@ class BrEdrConnectionRequest final { BrEdrConnectionRequest(PeerId id, DeviceAddress addr, - fit::closure timeout_cb, + fit::closure timeout_callback, pw::async::Dispatcher& dispatcher) : state_(RequestState::kPending), peer_id_(id), @@ -66,8 +66,8 @@ class BrEdrConnectionRequest final { timeout_task_(dispatcher), weak_self_(this) { timeout_task_.set_function( - [timeout_cb = std::move(timeout_cb)](pw::async::Context /*ctx*/, - pw::Status status) { + [timeout_cb = std::move(timeout_callback)](pw::async::Context /*ctx*/, + pw::Status status) { if (status.ok()) { timeout_cb(); } diff --git a/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/low_energy_advertiser.h b/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/low_energy_advertiser.h index ee61606391..d11e5b8047 100644 --- a/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/low_energy_advertiser.h +++ b/pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/hci/low_energy_advertiser.h @@ -107,16 +107,16 @@ class LowEnergyAdvertiser : public LocalAddressClient { // HostError::kAdvertisingDataTooLong or HostError::kScanResponseTooLong error // will be generated. struct AdvertisingOptions { - AdvertisingOptions(AdvertisingIntervalRange interval, - AdvFlags flags, - bool extended_pdu, - bool anonymous, - bool include_tx_power_level) - : interval(interval), - flags(flags), - extended_pdu(extended_pdu), - include_tx_power_level(include_tx_power_level), - anonymous(anonymous) {} + AdvertisingOptions(AdvertisingIntervalRange init_interval, + AdvFlags init_flags, + bool init_extended_pdu, + bool init_anonymous, + bool init_include_tx_power_level) + : interval(init_interval), + flags(init_flags), + extended_pdu(init_extended_pdu), + include_tx_power_level(init_include_tx_power_level), + anonymous(init_anonymous) {} AdvertisingIntervalRange interval; AdvFlags flags;