Skip to content

Commit

Permalink
pw_bluetooth_sapphire: Use PW_CHECK directly for most asserts
Browse files Browse the repository at this point in the history
Use PW_ASSERT only for the few asserts in constexpr functions.

Change-Id: I55896095e9e50ef6eaf9748479e409d3761d4f75
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/244533
Docs-Not-Needed: Ben Lawson <benlawson@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Ben Lawson <benlawson@google.com>
Lint: Lint 🤖 <android-build-ayeaye@system.gserviceaccount.com>
  • Loading branch information
BenjaminLawson authored and CQ Bot Account committed Oct 28, 2024
1 parent 7ddb65a commit 8153a8a
Show file tree
Hide file tree
Showing 213 changed files with 1,770 additions and 1,797 deletions.
2 changes: 1 addition & 1 deletion pw_bluetooth_sapphire/fuchsia/bt_hci_virtual/controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void VirtualController::CreateLoopbackDevice(

loopback_device_ = std::make_unique<LoopbackDevice>();

BT_ASSERT(request->has_uart_channel());
PW_CHECK(request->has_uart_channel());
zx_status_t status = loopback_device_->Initialize(
std::move(request->uart_channel()),
std::string_view(name),
Expand Down
6 changes: 3 additions & 3 deletions pw_bluetooth_sapphire/fuchsia/bt_host/host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,15 @@ void BtHostComponent::BindToHostInterface(
return;
}

BT_DEBUG_ASSERT(gap_);
BT_DEBUG_ASSERT(gatt_);
PW_DCHECK(gap_);
PW_DCHECK(gatt_);

zx::channel channel = host_client.TakeChannel();

host_server_ = std::make_unique<HostServer>(
std::move(channel), gap_->AsWeakPtr(), gatt_->GetWeakPtr());
host_server_->set_error_handler([this](zx_status_t status) {
BT_DEBUG_ASSERT(host_server_);
PW_DCHECK(host_server_);
bt_log(WARN, "bt-host", "Host interface disconnected");
host_server_ = nullptr;
});
Expand Down
2 changes: 1 addition & 1 deletion pw_bluetooth_sapphire/fuchsia/bt_host/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ int main() {
LifecycleHandler lifecycle_handler(&loop, host->GetWeakPtr());

auto init_cb = [&host, &lifecycle_handler, &loop](bool success) {
BT_DEBUG_ASSERT(host);
PW_DCHECK(host);
if (!success) {
bt_log(
ERROR, "bt-host", "Failed to initialize bt-host; shutting down...");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ FidlController::FidlController(fidl::ClientEnd<fhbt::Vendor> vendor_client_end,
[this](zx_status_t status) { OnScoUnbind(status); },
[this](fhbt::ScoPacket packet) { OnReceiveSco(std::move(packet)); }),
dispatcher_(dispatcher) {
BT_ASSERT(vendor_client_end.is_valid());
PW_CHECK(vendor_client_end.is_valid());
vendor_client_end_ = std::move(vendor_client_end);
}

Expand Down Expand Up @@ -373,7 +373,7 @@ void FidlController::GetFeatures(
void FidlController::EncodeVendorCommand(
pw::bluetooth::VendorCommandParameters parameters,
pw::Callback<void(pw::Result<pw::span<const std::byte>>)> callback) {
BT_ASSERT(vendor_);
PW_CHECK(vendor_);

if (!std::holds_alternative<pw::bluetooth::SetAclPriorityCommandParameters>(
parameters)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ void BrEdrConnectionServer::Receive(ReceiveCallback callback) {

void BrEdrConnectionServer::WatchChannelParameters(
WatchChannelParametersCallback callback) {
BT_ASSERT_MSG(
PW_CHECK(
!pending_watch_channel_parameters_.has_value(),
"WatchChannelParameters called while there was already a pending call.");
pending_watch_channel_parameters_ = std::move(callback);
Expand All @@ -99,7 +99,7 @@ void BrEdrConnectionServer::handle_unknown_method(uint64_t ordinal,
}

bool BrEdrConnectionServer::Activate() {
BT_ASSERT(state_ == State::kActivating);
PW_CHECK(state_ == State::kActivating);

WeakPtr self = weak_self_.GetWeakPtr();
bt::l2cap::ChannelId channel_id = channel_->id();
Expand Down Expand Up @@ -136,7 +136,7 @@ bool BrEdrConnectionServer::Activate() {
}

void BrEdrConnectionServer::Deactivate() {
BT_ASSERT(state_ != State::kDeactivated);
PW_CHECK(state_ != State::kDeactivated);
state_ = State::kDeactivating;

if (!receive_queue_.empty()) {
Expand All @@ -156,7 +156,7 @@ void BrEdrConnectionServer::Deactivate() {
void BrEdrConnectionServer::OnChannelDataReceived(bt::ByteBufferPtr rx_data) {
// Note: kActivating is deliberately permitted, as ChannelImpl::Activate()
// will synchronously deliver any queued frames.
BT_ASSERT(state_ != State::kDeactivated);
PW_CHECK(state_ != State::kDeactivated);
if (state_ == State::kDeactivating) {
bt_log(DEBUG,
"fidl",
Expand All @@ -166,14 +166,14 @@ void BrEdrConnectionServer::OnChannelDataReceived(bt::ByteBufferPtr rx_data) {
return;
}

BT_ASSERT(rx_data);
PW_CHECK(rx_data);
if (rx_data->size() == 0) {
bt_log(
DEBUG, "fidl", "Ignoring empty rx_data for channel %u", channel_->id());
return;
}

BT_ASSERT(receive_queue_.size() <= receive_queue_max_frames_);
PW_CHECK(receive_queue_.size() <= receive_queue_max_frames_);
// On a full queue, we drop the oldest element, on the theory that newer data
// is more useful. This should be true, e.g., for real-time applications such
// as voice calls. In the future, we may want to make the drop-head vs.
Expand All @@ -196,7 +196,7 @@ void BrEdrConnectionServer::OnChannelClosed() {
channel_->id());
return;
}
BT_ASSERT(state_ == State::kActivated);
PW_CHECK(state_ == State::kActivated);
DeactivateAndRequestDestruction();
}

Expand Down
20 changes: 10 additions & 10 deletions pw_bluetooth_sapphire/fuchsia/host/fidl/gatt2_client_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ Gatt2ClientServer::Gatt2ClientServer(
}

Gatt2ClientServer::~Gatt2ClientServer() {
BT_ASSERT(gatt()->UnregisterRemoteServiceWatcher(service_watcher_id_));
PW_CHECK(gatt()->UnregisterRemoteServiceWatcher(service_watcher_id_));
}

void Gatt2ClientServer::OnWatchServicesResult(
Expand Down Expand Up @@ -233,7 +233,7 @@ void Gatt2ClientServer::WatchServices(std::vector<fb::Uuid> fidl_uuids,
"WatchServices: ListServices complete (peer: %s)",
bt_str(self->peer_id_));

BT_ASSERT(self->watch_services_request_);
PW_CHECK(self->watch_services_request_);
self->list_services_complete_ = true;
self->OnWatchServicesResult(
/*removed=*/{}, /*added=*/services, /*modified=*/{});
Expand Down Expand Up @@ -280,7 +280,7 @@ void Gatt2ClientServer::ConnectToService(
request.Close(ZX_ERR_NOT_FOUND);
return;
}
BT_ASSERT(service_handle == service->handle());
PW_CHECK(service_handle == service->handle());

// This removed handler may be called long after the service is removed from
// the service map or this server is destroyed, since removed handlers are not
Expand Down Expand Up @@ -316,12 +316,12 @@ void Gatt2ClientServer::ConnectToService(
// service is already shut down, but that should not be possible in this
// synchronous callback (the service would not have been returned in the first
// place).
BT_ASSERT_MSG(service->AddRemovedHandler(std::move(removed_handler)),
"adding service removed handler failed (service may be shut "
"down) (peer: %s, "
"handle: %#.4x)",
bt_str(peer_id_),
service_handle);
PW_CHECK(service->AddRemovedHandler(std::move(removed_handler)),
"adding service removed handler failed (service may be shut "
"down) (peer: %s, "
"handle: %#.4x)",
bt_str(peer_id_),
service_handle);

std::unique_ptr<Gatt2RemoteServiceServer> remote_service_server =
std::make_unique<Gatt2RemoteServiceServer>(
Expand All @@ -340,7 +340,7 @@ void Gatt2ClientServer::ConnectToService(
});

// Error handler should not have been called yet.
BT_ASSERT(services_.count(service_handle) == 1);
PW_CHECK(services_.count(service_handle) == 1);
services_[service_handle] = std::move(remote_service_server);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ void Gatt2RemoteServiceServer::ReadByType(::fuchsia::bluetooth::Uuid uuid,
}

measure_fbg::Size result_size = measure_fbg::Measure(fidl_result);
BT_ASSERT(result_size.num_handles == 0);
PW_CHECK(result_size.num_handles == 0);
bytes_used += result_size.num_bytes;

if (bytes_used > kMaxBytes) {
Expand Down Expand Up @@ -441,7 +441,7 @@ void Gatt2RemoteServiceServer::RegisterCharacteristicNotifier(
// `DisableNotifications` completion callback in
// `OnCharacteristicNotifierError`, so no notifications should be received
// after removing a notifier.
BT_ASSERT_MSG(
PW_CHECK(
notifier_iter != self->characteristic_notifiers_.end(),
"characteristic notification value received after notifier unregistered"
"(peer: %s, characteristic: 0x%lX) ",
Expand Down Expand Up @@ -509,7 +509,7 @@ void Gatt2RemoteServiceServer::RegisterCharacteristicNotifier(
.notifier = notifier_handle.Bind()};
auto [notifier_iter, emplaced] = self->characteristic_notifiers_.emplace(
notifier_id, std::move(notifier));
BT_ASSERT(emplaced);
PW_CHECK(emplaced);

// When the client closes the protocol, unregister the notifier.
notifier_iter->second.notifier.set_error_handler(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class Gatt2RemoteServiceServerTest : public bt::fidl::testing::FakeGattFixture {

protected:
const bt::gatt::testing::FakeClient::WeakPtr& fake_client() const {
BT_ASSERT(fake_client_.is_alive());
PW_CHECK(fake_client_.is_alive());
return fake_client_;
}

Expand Down
10 changes: 5 additions & 5 deletions pw_bluetooth_sapphire/fuchsia/host/fidl/gatt2_server_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ void Gatt2ServerServer::OnReadRequest(bt::PeerId peer_id,
btg::ReadResponder responder) {
auto svc_iter = services_.find(InternalServiceId(service_id));
// GATT must only send read requests for registered services.
BT_ASSERT(svc_iter != services_.end());
PW_CHECK(svc_iter != services_.end());

auto cb = [responder = std::move(responder)](
LocalService_ReadValue_Result res) mutable {
Expand All @@ -209,7 +209,7 @@ void Gatt2ServerServer::OnWriteRequest(bt::PeerId peer_id,
btg::WriteResponder responder) {
auto svc_iter = services_.find(InternalServiceId(service_id));
// GATT must only send write requests for registered services.
BT_ASSERT(svc_iter != services_.end());
PW_CHECK(svc_iter != services_.end());

auto cb = [responder = std::move(responder)](
LocalService_WriteValue_Result result) mutable {
Expand Down Expand Up @@ -240,7 +240,7 @@ void Gatt2ServerServer::OnClientCharacteristicConfiguration(
bool indicate) {
auto svc_iter = services_.find(InternalServiceId(service_id));
// GATT must only send CCC updates for registered services.
BT_ASSERT(svc_iter != services_.end());
PW_CHECK(svc_iter != services_.end());

auto cb = []() {
bt_log(TRACE, "fidl", "characteristic configuration acknowledged");
Expand All @@ -263,7 +263,7 @@ bool Gatt2ServerServer::ValidateValueChangedEvent(
const char* update_type) {
auto iter = services_.find(service_id);
// It is impossible for clients to send events to a closed service.
BT_ASSERT(iter != services_.end());
PW_CHECK(iter != services_.end());
// Subtract credit before validating parameters so that credits aren't
// permanently lost from the client's perspective.
SubtractCredit(iter->second);
Expand Down Expand Up @@ -363,7 +363,7 @@ void Gatt2ServerServer::SubtractCredit(Service& svc) {
// perspective because new credits are granted before the count reaches 0
// (excessive events will fill the FIDL channel and eventually crash the
// client).
BT_ASSERT(svc.credits > 0);
PW_CHECK(svc.credits > 0);
svc.credits--;
if (svc.credits <= REFRESH_CREDITS_AT) {
// Static cast OK because current_credits > 0 and we never add more than
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ GattRemoteServiceServer::GattRemoteServiceServer(
service_(std::move(service)),
peer_id_(peer_id),
weak_self_(this) {
BT_DEBUG_ASSERT(service_.is_alive());
PW_DCHECK(service_.is_alive());
}

GattRemoteServiceServer::~GattRemoteServiceServer() {
Expand Down Expand Up @@ -397,10 +397,9 @@ void GattRemoteServiceServer::NotifyCharacteristic(
}

if (status.is_ok()) {
BT_DEBUG_ASSERT(handler_id != bt::gatt::kInvalidId);
BT_DEBUG_ASSERT(self->notify_handlers_.count(handle) == 1u);
BT_DEBUG_ASSERT(self->notify_handlers_[handle] ==
bt::gatt::kInvalidId);
PW_DCHECK(handler_id != bt::gatt::kInvalidId);
PW_DCHECK(self->notify_handlers_.count(handle) == 1u);
PW_DCHECK(self->notify_handlers_[handle] == bt::gatt::kInvalidId);
self->notify_handlers_[handle] = handler_id;
} else {
// Remove our handle holder.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class GattRemoteServiceServerTest : public bt::fidl::testing::FakeGattFixture {

protected:
const bt::gatt::testing::FakeClient::WeakPtr& fake_client() const {
BT_ASSERT(fake_client_.is_alive());
PW_CHECK(fake_client_.is_alive());
return fake_client_;
}

Expand Down
12 changes: 6 additions & 6 deletions pw_bluetooth_sapphire/fuchsia/host/fidl/gatt_server_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ class GattServerServer::LocalServiceImpl
owner_(owner),
id_(id),
delegate_(std::move(delegate)) {
BT_DEBUG_ASSERT(owner_);
BT_DEBUG_ASSERT(delegate_);
PW_DCHECK(owner_);
PW_DCHECK(delegate_);
}

// The destructor removes the GATT service
Expand Down Expand Up @@ -336,7 +336,7 @@ void GattServerServer::PublishService(
return;
}

BT_DEBUG_ASSERT(self->services_.find(id) == self->services_.end());
PW_DCHECK(self->services_.find(id) == self->services_.end());

// This will be called if either the delegate or the service connection
// closes.
Expand Down Expand Up @@ -388,7 +388,7 @@ void GattServerServer::OnReadRequest(bt::gatt::IdType service_id,
};

auto* delegate = iter->second->delegate();
BT_DEBUG_ASSERT(delegate);
PW_DCHECK(delegate);
delegate->OnReadValue(id, offset, std::move(cb));
}

Expand All @@ -407,7 +407,7 @@ void GattServerServer::OnWriteRequest(bt::gatt::IdType service_id,

auto fidl_value = fidl::To<std::vector<uint8_t>>(value);
auto* delegate = iter->second->delegate();
BT_DEBUG_ASSERT(delegate);
PW_DCHECK(delegate);

if (!responder) {
delegate->OnWriteWithoutResponse(id, offset, std::move(fidl_value));
Expand All @@ -434,7 +434,7 @@ void GattServerServer::OnCharacteristicConfig(bt::gatt::IdType service_id,
}

auto* delegate = iter->second->delegate();
BT_DEBUG_ASSERT(delegate);
PW_DCHECK(delegate);
delegate->OnCharacteristicConfiguration(
chrc_id, peer_id.ToString(), notify, indicate);
}
Expand Down
10 changes: 5 additions & 5 deletions pw_bluetooth_sapphire/fuchsia/host/fidl/helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ bool IsScanFilterValid(const fble::ScanFilter& fidl_filter) {

bool PopulateDiscoveryFilter(const fble::ScanFilter& fidl_filter,
bt::gap::DiscoveryFilter* out_filter) {
BT_DEBUG_ASSERT(out_filter);
PW_DCHECK(out_filter);

if (fidl_filter.service_uuids) {
std::vector<bt::UUID> uuids;
Expand Down Expand Up @@ -1742,7 +1742,7 @@ fuchsia::bluetooth::le::ScanData AdvertisingDataToFidlScanData(
}

fble::Peer PeerToFidlLe(const bt::gap::Peer& peer) {
BT_ASSERT(peer.le());
PW_CHECK(peer.le());

fble::Peer output;
output.set_id(fbt::PeerId{peer.identifier().value()});
Expand Down Expand Up @@ -1848,7 +1848,7 @@ ServiceDefinitionToServiceRecord(
// ProtocolListId = uint8_t, and std::numeric_limits<uint8_t>::max() == 255
// == the MAX_SEQUENCE_LENGTH vector limit from
// fuchsia.bluetooth.bredr/ServiceDefinition.additional_protocol_descriptor_lists.
BT_ASSERT(
PW_CHECK(
definition.additional_protocol_descriptor_lists()->size() <=
std::numeric_limits<bt::sdp::ServiceRecord::ProtocolListId>::max());
bt::sdp::ServiceRecord::ProtocolListId protocol_list_id = 1;
Expand Down Expand Up @@ -1969,7 +1969,7 @@ ServiceDefinitionToServiceRecord(
// ProtocolListId = uint8_t, and std::numeric_limits<uint8_t>::max() == 255
// == the MAX_SEQUENCE_LENGTH vector limit from
// fuchsia.bluetooth.bredr/ServiceDefinition.additional_protocol_descriptor_lists.
BT_ASSERT(
PW_CHECK(
definition.additional_protocol_descriptor_lists().size() <=
std::numeric_limits<bt::sdp::ServiceRecord::ProtocolListId>::max());
bt::sdp::ServiceRecord::ProtocolListId protocol_list_id = 1;
Expand Down Expand Up @@ -2654,7 +2654,7 @@ bt::StaticPacket<pw::bluetooth::emboss::CodecIdWriter> CodecIdFromFidl(
CodingFormatFromFidl(fidl_codec_id.assigned_format());
result_view.coding_format().Write(out_coding_format);
} else {
BT_ASSERT(fidl_codec_id.is_vendor_format());
PW_CHECK(fidl_codec_id.is_vendor_format());
result_view.coding_format().Write(
pw::bluetooth::emboss::CodingFormat::VENDOR_SPECIFIC);
result_view.company_id().Write(fidl_codec_id.vendor_format().company_id());
Expand Down
Loading

0 comments on commit 8153a8a

Please sign in to comment.