From 65c2298f27489e8c61c94afdcf4d06783ff92ab0 Mon Sep 17 00:00:00 2001 From: Edwin Wu Date: Tue, 10 Dec 2024 18:55:22 -0800 Subject: [PATCH] analytics: Add exact OperationResultCode for BLE/BLEV2 - Connect/AttemptConnect PiperOrigin-RevId: 704926745 --- connections/implementation/mediums/ble.cc | 34 +++++++++++++------ connections/implementation/mediums/ble.h | 12 ++++--- .../implementation/mediums/ble_test.cc | 15 +++++--- connections/implementation/mediums/ble_v2.cc | 17 ++++++---- connections/implementation/mediums/ble_v2.h | 6 ++-- .../implementation/mediums/ble_v2_test.cc | 18 ++++++---- .../implementation/p2p_cluster_pcp_handler.cc | 20 +++++++---- 7 files changed, 79 insertions(+), 43 deletions(-) diff --git a/connections/implementation/mediums/ble.cc b/connections/implementation/mediums/ble.cc index 887d5ef6..a864bfb0 100644 --- a/connections/implementation/mediums/ble.cc +++ b/connections/implementation/mediums/ble.cc @@ -15,14 +15,19 @@ #include "connections/implementation/mediums/ble.h" #include -#include +#include #include #include #include "absl/strings/escaping.h" #include "connections/implementation/mediums/ble_v2/ble_advertisement.h" +#include "connections/implementation/mediums/bluetooth_radio.h" #include "connections/implementation/mediums/utils.h" +#include "internal/platform/ble.h" +#include "internal/platform/bluetooth_adapter.h" #include "internal/platform/byte_array.h" +#include "internal/platform/cancellation_flag.h" +#include "internal/platform/expected.h" #include "internal/platform/logging.h" #include "internal/platform/mutex_lock.h" #include "internal/platform/prng.h" @@ -30,6 +35,10 @@ namespace nearby { namespace connections { +namespace { +using location::nearby::proto::connections::OperationResultCode; +} // namespace + ByteArray Ble::GenerateHash(const std::string& source, size_t size) { return Utils::Sha256Hash(source, size); } @@ -144,8 +153,8 @@ bool Ble::StopAdvertising(const std::string& service_id) { bool Ble::StartLegacyAdvertising( const std::string& input_service_id, const std::string& local_endpoint_id, const std::string& fast_advertisement_service_uuid) { - NEARBY_LOGS(INFO) << "StartLegacyAdvertising: " << input_service_id.c_str() - << ", local_endpoint_id: " << local_endpoint_id.c_str(); + NEARBY_LOGS(INFO) << "StartLegacyAdvertising: " << input_service_id + << ", local_endpoint_id: " << local_endpoint_id; MutexLock lock(&mutex_); std::string service_id = input_service_id + "-Legacy"; @@ -195,7 +204,7 @@ bool Ble::StartLegacyAdvertising( } bool Ble::StopLegacyAdvertising(const std::string& input_service_id) { - NEARBY_LOGS(INFO) << "StopLegacyAdvertising:" << input_service_id.c_str(); + NEARBY_LOGS(INFO) << "StopLegacyAdvertising:" << input_service_id; MutexLock lock(&mutex_); std::string service_id = input_service_id + "-Legacy"; @@ -384,8 +393,9 @@ bool Ble::IsAcceptingConnectionsLocked(const std::string& service_id) { return accepting_connections_info_.Existed(service_id); } -BleSocket Ble::Connect(BlePeripheral& peripheral, const std::string& service_id, - CancellationFlag* cancellation_flag) { +ErrorOr Ble::Connect(BlePeripheral& peripheral, + const std::string& service_id, + CancellationFlag* cancellation_flag) { MutexLock lock(&mutex_); NEARBY_LOGS(INFO) << "BLE::Connect: service=" << &peripheral; // Socket to return. To allow for NRVO to work, it has to be a single object. @@ -393,24 +403,26 @@ BleSocket Ble::Connect(BlePeripheral& peripheral, const std::string& service_id, if (service_id.empty()) { NEARBY_LOGS(INFO) << "Refusing to create BLE socket with empty service_id."; - return socket; + // TODO(edwinwu): Modify new OperationResultCode + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } if (!radio_.IsEnabled()) { NEARBY_LOGS(INFO) << "Can't create client BLE socket to " << &peripheral << " because Bluetooth isn't enabled."; - return socket; + return {Error(OperationResultCode::MISCELLEANEOUS_BLE_SYSTEM_SERVICE_NULL)}; } if (!IsAvailableLocked()) { NEARBY_LOGS(INFO) << "Can't create client BLE socket [service_id=" << service_id << "]; BLE isn't available."; - return socket; + return {Error(OperationResultCode::MEDIUM_UNAVAILABLE_BLE_NOT_AVAILABLE)}; } if (cancellation_flag->Cancelled()) { NEARBY_LOGS(INFO) << "Can't create client BLE socket due to cancel."; - return socket; + return {Error(OperationResultCode:: + CLIENT_CANCELLATION_CANCEL_BLE_OUTGOING_CONNECTION)}; } socket = medium_.Connect(peripheral, service_id, cancellation_flag); @@ -428,7 +440,7 @@ ByteArray Ble::UnwrapAdvertisementBytes( mediums::BleAdvertisement::CreateBleAdvertisement( medium_advertisement_data); if (!medium_ble_advertisement_status_or.ok()) { - NEARBY_LOGS(INFO) << medium_ble_advertisement_status_or.status().ToString(); + NEARBY_LOGS(INFO) << medium_ble_advertisement_status_or.status(); return ByteArray(); } diff --git a/connections/implementation/mediums/ble.h b/connections/implementation/mediums/ble.h index 2502f92b..f21aa1b7 100644 --- a/connections/implementation/mediums/ble.h +++ b/connections/implementation/mediums/ble.h @@ -15,16 +15,17 @@ #ifndef CORE_INTERNAL_MEDIUMS_BLE_H_ #define CORE_INTERNAL_MEDIUMS_BLE_H_ -#include #include -#include "absl/container/flat_hash_map.h" +#include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_set.h" #include "connections/implementation/mediums/bluetooth_radio.h" #include "connections/listeners.h" +#include "internal/platform/ble.h" +#include "internal/platform/bluetooth_adapter.h" #include "internal/platform/byte_array.h" #include "internal/platform/cancellation_flag.h" -#include "internal/platform/ble.h" +#include "internal/platform/expected.h" #include "internal/platform/multi_thread_executor.h" #include "internal/platform/mutex.h" @@ -111,8 +112,9 @@ class Ble { // service_id. Blocks until connection is established, or server-side is // terminated. Returns socket instance. On success, BleSocket.IsValid() return // true. - BleSocket Connect(BlePeripheral& peripheral, const std::string& service_id, - CancellationFlag* cancellation_flag) + ErrorOr Connect(BlePeripheral& peripheral, + const std::string& service_id, + CancellationFlag* cancellation_flag) ABSL_LOCKS_EXCLUDED(mutex_); private: diff --git a/connections/implementation/mediums/ble_test.cc b/connections/implementation/mediums/ble_test.cc index c88cb51c..0382381b 100644 --- a/connections/implementation/mediums/ble_test.cc +++ b/connections/implementation/mediums/ble_test.cc @@ -26,6 +26,7 @@ #include "internal/platform/byte_array.h" #include "internal/platform/cancellation_flag.h" #include "internal/platform/count_down_latch.h" +#include "internal/platform/expected.h" #include "internal/platform/feature_flags.h" #include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" @@ -100,9 +101,11 @@ TEST_P(BleTest, CanStartAcceptingConnectionsAndConnect) { BlePeripheral discovered_peripheral = atomic_discovered_peripheral.load(); ASSERT_TRUE(discovered_peripheral.IsValid()); CancellationFlag flag; - BleSocket socket = ble_b.Connect(discovered_peripheral, service_id, &flag); + ErrorOr socket_result = + ble_b.Connect(discovered_peripheral, service_id, &flag); EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); - EXPECT_TRUE(socket.IsValid()); + EXPECT_TRUE(socket_result.has_value()); + EXPECT_TRUE(socket_result.value().IsValid()); ble_b.StopScanning(service_id); ble_a.StopAdvertising(service_id); env_.Stop(); @@ -151,14 +154,16 @@ TEST_P(BleTest, CanCancelConnect) { BlePeripheral discovered_peripheral = atomic_discovered_peripheral.load(); ASSERT_TRUE(discovered_peripheral.IsValid()); CancellationFlag flag(true); - BleSocket socket = ble_b.Connect(discovered_peripheral, service_id, &flag); + ErrorOr socket_result = + ble_b.Connect(discovered_peripheral, service_id, &flag); // If FeatureFlag is disabled, Cancelled is false as no-op. if (!feature_flags.enable_cancellation_flag) { EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); - EXPECT_TRUE(socket.IsValid()); + EXPECT_TRUE(socket_result.has_value()); + EXPECT_TRUE(socket_result.value().IsValid()); } else { EXPECT_FALSE(accept_latch.Await(kWaitDuration).result()); - EXPECT_FALSE(socket.IsValid()); + EXPECT_TRUE(socket_result.has_error()); } ble_b.StopScanning(service_id); ble_a.StopAdvertising(service_id); diff --git a/connections/implementation/mediums/ble_v2.cc b/connections/implementation/mediums/ble_v2.cc index 46d0091e..20d34b46 100644 --- a/connections/implementation/mediums/ble_v2.cc +++ b/connections/implementation/mediums/ble_v2.cc @@ -353,6 +353,7 @@ ErrorOr BleV2::StartScanning(const std::string& service_id, if (service_id.empty()) { LOG(INFO) << "Can not start BLE scanning with empty service id."; + // TODO(edwinwu): Modify new OperationResultCode return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } @@ -601,9 +602,9 @@ bool BleV2::IsAcceptingConnections(const std::string& service_id) { return IsAcceptingConnectionsLocked(service_id); } -BleV2Socket BleV2::Connect(const std::string& service_id, - const BleV2Peripheral& peripheral, - CancellationFlag* cancellation_flag) { +ErrorOr BleV2::Connect(const std::string& service_id, + const BleV2Peripheral& peripheral, + CancellationFlag* cancellation_flag) { MutexLock lock(&mutex_); // Socket to return. To allow for NRVO to work, it has to be a single object. BleV2Socket socket; @@ -611,18 +612,20 @@ BleV2Socket BleV2::Connect(const std::string& service_id, if (service_id.empty()) { LOG(INFO) << "Refusing to create client Ble socket because " "service_id is empty."; - return socket; + // TODO(edwinwu): Modify new OperationResultCode + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } if (!IsAvailableLocked()) { LOG(INFO) << "Can't create client Ble socket [service_id=" << service_id << "]; Ble isn't available."; - return socket; + return {Error(OperationResultCode::MEDIUM_UNAVAILABLE_BLE_NOT_AVAILABLE)}; } if (cancellation_flag->Cancelled()) { LOG(INFO) << "Can't create client Ble socket due to cancel."; - return socket; + return {Error(OperationResultCode:: + CLIENT_CANCELLATION_CANCEL_BLE_OUTGOING_CONNECTION)}; } socket = medium_.Connect(service_id, @@ -630,6 +633,8 @@ BleV2Socket BleV2::Connect(const std::string& service_id, peripheral, cancellation_flag); if (!socket.IsValid()) { LOG(INFO) << "Failed to Connect via Ble [service_id=" << service_id << "]"; + return {Error( + OperationResultCode::CONNECTIVITY_BLE_CLIENT_SOCKET_CREATION_FAILURE)}; } return socket; diff --git a/connections/implementation/mediums/ble_v2.h b/connections/implementation/mediums/ble_v2.h index 65b60a78..1216c107 100644 --- a/connections/implementation/mediums/ble_v2.h +++ b/connections/implementation/mediums/ble_v2.h @@ -139,9 +139,9 @@ class BleV2 final { // Establishes connection to Ble peripheral. // Returns socket instance. On success, BleSocket.IsValid() return true. - BleV2Socket Connect(const std::string& service_id, - const BleV2Peripheral& peripheral, - CancellationFlag* cancellation_flag) + ErrorOr Connect(const std::string& service_id, + const BleV2Peripheral& peripheral, + CancellationFlag* cancellation_flag) ABSL_LOCKS_EXCLUDED(mutex_); // Returns true if this object owns a valid platform implementation. diff --git a/connections/implementation/mediums/ble_v2_test.cc b/connections/implementation/mediums/ble_v2_test.cc index 39f22bf3..8c9d55c2 100644 --- a/connections/implementation/mediums/ble_v2_test.cc +++ b/connections/implementation/mediums/ble_v2_test.cc @@ -30,6 +30,7 @@ #include "internal/platform/byte_array.h" #include "internal/platform/cancellation_flag.h" #include "internal/platform/count_down_latch.h" +#include "internal/platform/expected.h" #include "internal/platform/feature_flags.h" #include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" @@ -120,15 +121,16 @@ TEST_P(BleV2Test, CanConnect) { ASSERT_TRUE(discovered_peripheral.IsValid()); CancellationFlag flag; - BleV2Socket socket_for_client = + ErrorOr socket_for_client_result = ble_client.Connect(service_id, discovered_peripheral, &flag); EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); EXPECT_TRUE(ble_server.StopAcceptingConnections(service_id)); EXPECT_TRUE(ble_server.StopAdvertising(service_id)); EXPECT_TRUE(socket_for_server.IsValid()); - EXPECT_TRUE(socket_for_client.IsValid()); + EXPECT_TRUE(socket_for_client_result.has_value()); + EXPECT_TRUE(socket_for_client_result.value().IsValid()); EXPECT_TRUE(socket_for_server.GetRemotePeripheral().IsValid()); - EXPECT_TRUE(socket_for_client.GetRemotePeripheral().IsValid()); + EXPECT_TRUE(socket_for_client_result.value().GetRemotePeripheral().IsValid()); env_.Stop(); } @@ -179,7 +181,7 @@ TEST_P(BleV2Test, CanCancelConnect) { ASSERT_TRUE(discovered_peripheral.IsValid()); CancellationFlag flag(true); - BleV2Socket socket_for_client = + ErrorOr socket_for_client_result = ble_client.Connect(service_id, discovered_peripheral, &flag); // If FeatureFlag is disabled, Cancelled is false as no-op. if (!feature_flags.enable_cancellation_flag) { @@ -187,15 +189,17 @@ TEST_P(BleV2Test, CanCancelConnect) { EXPECT_TRUE(ble_server.StopAcceptingConnections(service_id)); EXPECT_TRUE(ble_server.StopAdvertising(service_id)); EXPECT_TRUE(socket_for_server.IsValid()); - EXPECT_TRUE(socket_for_client.IsValid()); + EXPECT_TRUE(socket_for_client_result.has_value()); + EXPECT_TRUE(socket_for_client_result.value().IsValid()); EXPECT_TRUE(socket_for_server.GetRemotePeripheral().IsValid()); - EXPECT_TRUE(socket_for_client.GetRemotePeripheral().IsValid()); + EXPECT_TRUE( + socket_for_client_result.value().GetRemotePeripheral().IsValid()); } else { EXPECT_FALSE(accept_latch.Await(kWaitDuration).result()); EXPECT_TRUE(ble_server.StopAcceptingConnections(service_id)); EXPECT_TRUE(ble_server.StopAdvertising(service_id)); EXPECT_FALSE(socket_for_server.IsValid()); - EXPECT_FALSE(socket_for_client.IsValid()); + EXPECT_TRUE(socket_for_client_result.has_error()); } env_.Stop(); } diff --git a/connections/implementation/p2p_cluster_pcp_handler.cc b/connections/implementation/p2p_cluster_pcp_handler.cc index c6ec65b1..2c873034 100644 --- a/connections/implementation/p2p_cluster_pcp_handler.cc +++ b/connections/implementation/p2p_cluster_pcp_handler.cc @@ -2422,25 +2422,29 @@ BasePcpHandler::ConnectImplResult P2pClusterPcpHandler::BleConnectImpl( BlePeripheral& peripheral = endpoint->ble_peripheral; - BleSocket ble_socket = + ErrorOr ble_socket_result = ble_medium_.Connect(peripheral, endpoint->service_id, client->GetCancellationFlag(endpoint->endpoint_id)); - if (!ble_socket.IsValid()) { + if (ble_socket_result.has_error()) { NEARBY_LOGS(ERROR) << "In BleConnectImpl(), failed to connect to BLE device " << peripheral.GetName() << " for endpoint(id=" << endpoint->endpoint_id << ")."; return BasePcpHandler::ConnectImplResult{ .status = {Status::kBleError}, + .operation_result_code = + ble_socket_result.error().operation_result_code().value(), }; } auto channel = std::make_unique( - endpoint->service_id, /*channel_name=*/endpoint->endpoint_id, ble_socket); + endpoint->service_id, /*channel_name=*/endpoint->endpoint_id, + ble_socket_result.value()); return BasePcpHandler::ConnectImplResult{ .medium = BLE, .status = {Status::kSuccess}, + .operation_result_code = OperationResultCode::DETAIL_SUCCESS, .endpoint_channel = std::move(channel), }; } @@ -2674,25 +2678,29 @@ BasePcpHandler::ConnectImplResult P2pClusterPcpHandler::BleV2ConnectImpl( BleV2Peripheral& peripheral = endpoint->ble_peripheral; - BleV2Socket ble_socket = ble_v2_medium_.Connect( + ErrorOr ble_socket_result = ble_v2_medium_.Connect( endpoint->service_id, peripheral, client->GetCancellationFlag(endpoint->endpoint_id)); - if (!ble_socket.IsValid()) { + if (ble_socket_result.has_error()) { NEARBY_LOGS(ERROR) << "In BleV2ConnectImpl(), failed to connect to BLE device " << absl::BytesToHexString(peripheral.GetId().data()) << " for endpoint(id=" << endpoint->endpoint_id << ")."; return BasePcpHandler::ConnectImplResult{ .status = {Status::kBleError}, + .operation_result_code = + ble_socket_result.error().operation_result_code().value(), }; } auto channel = std::make_unique( - endpoint->service_id, /*channel_name=*/endpoint->endpoint_id, ble_socket); + endpoint->service_id, /*channel_name=*/endpoint->endpoint_id, + ble_socket_result.value()); return BasePcpHandler::ConnectImplResult{ .medium = BLE, .status = {Status::kSuccess}, + .operation_result_code = OperationResultCode::DETAIL_SUCCESS, .endpoint_channel = std::move(channel), }; }