From 9ebf66aef39bdc867dd87980b592f313699f306e Mon Sep 17 00:00:00 2001 From: Edwin Wu Date: Tue, 10 Dec 2024 19:23:34 -0800 Subject: [PATCH] analytics: Add exact OperationResultCode for Wifi_Lan - Connect/AttemptConnect PiperOrigin-RevId: 704931772 --- .../implementation/mediums/wifi_lan.cc | 34 ++++++----- connections/implementation/mediums/wifi_lan.h | 12 ++-- .../implementation/mediums/wifi_lan_test.cc | 59 ++++++++++--------- .../implementation/p2p_cluster_pcp_handler.cc | 12 ++-- .../implementation/wifi_lan_bwu_handler.cc | 19 +++--- 5 files changed, 76 insertions(+), 60 deletions(-) diff --git a/connections/implementation/mediums/wifi_lan.cc b/connections/implementation/mediums/wifi_lan.cc index 888b1be8..e36de309 100644 --- a/connections/implementation/mediums/wifi_lan.cc +++ b/connections/implementation/mediums/wifi_lan.cc @@ -428,9 +428,9 @@ bool WifiLan::IsAcceptingConnectionsLocked(const std::string& service_id) { return server_sockets_.find(service_id) != server_sockets_.end(); } -WifiLanSocket WifiLan::Connect(const std::string& service_id, - const NsdServiceInfo& service_info, - CancellationFlag* cancellation_flag) { +ErrorOr WifiLan::Connect(const std::string& service_id, + const NsdServiceInfo& service_info, + CancellationFlag* cancellation_flag) { MutexLock lock(&mutex_); // Socket to return. To allow for NRVO to work, it has to be a single object. WifiLanSocket socket; @@ -438,18 +438,20 @@ WifiLanSocket WifiLan::Connect(const std::string& service_id, if (service_id.empty()) { NEARBY_LOGS(INFO) << "Refusing to create client WifiLan socket because " "service_id is empty."; - return socket; + // TODO(edwinwu): Modify new OperationResultCode + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } if (!IsAvailableLocked()) { NEARBY_LOGS(INFO) << "Can't create client WifiLan socket [service_id=" << service_id << "]; WifiLan isn't available."; - return socket; + return {Error(OperationResultCode::MEDIUM_UNAVAILABLE_LAN_NOT_AVAILABLE)}; } if (cancellation_flag->Cancelled()) { NEARBY_LOGS(INFO) << "Can't create client WifiLan socket due to cancel."; - return socket; + return {Error(OperationResultCode:: + CLIENT_CANCELLATION_CANCEL_LAN_OUTGOING_CONNECTION)}; } ExceptionOr virtual_socket = @@ -462,7 +464,8 @@ WifiLanSocket WifiLan::Connect(const std::string& service_id, if (!socket.IsValid()) { NEARBY_LOGS(INFO) << "Failed to Connect via WifiLan [service_id=" << service_id << "]"; - return socket; + return {Error( + OperationResultCode::CONNECTIVITY_LAN_CLIENT_SOCKET_CREATION_FAILURE)}; } else { ExceptionOr virtual_socket = CreateOutgoingMultiplexSocketLocked(socket, service_id, @@ -480,9 +483,9 @@ WifiLanSocket WifiLan::Connect(const std::string& service_id, return socket; } -WifiLanSocket WifiLan::Connect(const std::string& service_id, - const std::string& ip_address, int port, - CancellationFlag* cancellation_flag) { +ErrorOr WifiLan::Connect(const std::string& service_id, + const std::string& ip_address, int port, + CancellationFlag* cancellation_flag) { MutexLock lock(&mutex_); // Socket to return. To allow for NRVO to work, it has to be a single object. WifiLanSocket socket; @@ -490,18 +493,20 @@ WifiLanSocket WifiLan::Connect(const std::string& service_id, if (service_id.empty()) { NEARBY_LOGS(INFO) << "Refusing to create client WifiLan socket because " "service_id is empty."; - return socket; + // TODO(edwinwu): Modify new OperationResultCode + return {Error(OperationResultCode::DETAIL_UNKNOWN)}; } if (!IsAvailableLocked()) { NEARBY_LOGS(INFO) << "Can't create client WifiLan socket [service_id=" << service_id << "]; WifiLan isn't available."; - return socket; + return {Error(OperationResultCode::MEDIUM_UNAVAILABLE_LAN_NOT_AVAILABLE)}; } if (cancellation_flag->Cancelled()) { NEARBY_LOGS(INFO) << "Can't create client WifiLan socket due to cancel."; - return socket; + return {Error(OperationResultCode:: + CLIENT_CANCELLATION_CANCEL_LAN_OUTGOING_CONNECTION)}; } ExceptionOr virtual_socket = @@ -514,7 +519,8 @@ WifiLanSocket WifiLan::Connect(const std::string& service_id, if (!socket.IsValid()) { NEARBY_LOGS(INFO) << "Failed to Connect via WifiLan [service_id=" << service_id << "]"; - return socket; + return {Error( + OperationResultCode::CONNECTIVITY_LAN_CLIENT_SOCKET_CREATION_FAILURE)}; } else { ExceptionOr virtual_socket = CreateOutgoingMultiplexSocketLocked(socket, service_id, ip_address); diff --git a/connections/implementation/mediums/wifi_lan.h b/connections/implementation/mediums/wifi_lan.h index e75cdde9..32345f28 100644 --- a/connections/implementation/mediums/wifi_lan.h +++ b/connections/implementation/mediums/wifi_lan.h @@ -95,17 +95,17 @@ class WifiLan { // another service with StartAcceptingConnections() using the same service_id. // Blocks until connection is established, or server-side is terminated. // Returns socket instance. On success, WifiLanSocket.IsValid() return true. - WifiLanSocket Connect(const std::string& service_id, - const NsdServiceInfo& service_info, - CancellationFlag* cancellation_flag) + ErrorOr Connect(const std::string& service_id, + const NsdServiceInfo& service_info, + CancellationFlag* cancellation_flag) ABSL_LOCKS_EXCLUDED(mutex_); // Establishes connection to WifiLan service by ip address and port for // bandwidth upgradation. // Returns socket instance. On success, WifiLanSocket.IsValid() return true. - WifiLanSocket Connect(const std::string& service_id, - const std::string& ip_address, int port, - CancellationFlag* cancellation_flag) + ErrorOr Connect(const std::string& service_id, + const std::string& ip_address, int port, + CancellationFlag* cancellation_flag) ABSL_LOCKS_EXCLUDED(mutex_); // Gets ip address + port for remote services on the network to identify and diff --git a/connections/implementation/mediums/wifi_lan_test.cc b/connections/implementation/mediums/wifi_lan_test.cc index fa6649d1..77b8f59e 100644 --- a/connections/implementation/mediums/wifi_lan_test.cc +++ b/connections/implementation/mediums/wifi_lan_test.cc @@ -17,20 +17,21 @@ #include #include -#include "gmock/gmock.h" -#include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" #include "absl/strings/string_view.h" +#include "absl/time/time.h" #include "connections/implementation/flags/nearby_connections_feature_flags.h" #include "internal/flags/nearby_flags.h" +#include "internal/platform/base64_utils.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" #include "internal/platform/nsd_service_info.h" #include "internal/platform/single_thread_executor.h" #include "internal/platform/wifi_lan.h" -#include "internal/platform/base64_utils.h" namespace nearby { namespace connections { @@ -102,13 +103,14 @@ TEST_P(WifiLanTest, CanConnect) { ASSERT_TRUE(discovered_service_info.IsValid()); CancellationFlag flag; - WifiLanSocket socket_for_client = + ErrorOr socket_for_client_result = wifi_lan_client.Connect(service_id, discovered_service_info, &flag); EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); EXPECT_TRUE(wifi_lan_server.StopAcceptingConnections(service_id)); EXPECT_TRUE(wifi_lan_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()); env_.Stop(); } @@ -145,26 +147,28 @@ TEST_P(WifiLanTest, CanConnectWithMultiplex) { WifiLanSocket socket_for_client; SingleThreadExecutor client_executor; client_executor.Execute([&]() { - NsdServiceInfo discovered_service_info; - wifi_lan_client.StartDiscovery( - service_id, - { - .service_discovered_cb = - [&discovered_latch, &discovered_service_info]( - NsdServiceInfo service_info, const std::string& service_id) { - NEARBY_LOGS(INFO) - << "Discovered service_info=" << &service_info; - discovered_service_info = service_info; - discovered_latch.CountDown(); - }, - }); - discovered_latch.Await(kWaitDuration).result(); - ASSERT_TRUE(discovered_service_info.IsValid()); + NsdServiceInfo discovered_service_info; + wifi_lan_client.StartDiscovery( + service_id, { + .service_discovered_cb = + [&discovered_latch, &discovered_service_info]( + NsdServiceInfo service_info, + const std::string& service_id) { + NEARBY_LOGS(INFO) << "Discovered service_info=" + << &service_info; + discovered_service_info = service_info; + discovered_latch.CountDown(); + }, + }); + discovered_latch.Await(kWaitDuration).result(); + ASSERT_TRUE(discovered_service_info.IsValid()); - CancellationFlag flag; - socket_for_client = - wifi_lan_client.Connect(service_id, discovered_service_info, &flag); - Base64Utils::WriteInt(&socket_for_client.GetOutputStream(), 4); + CancellationFlag flag; + ErrorOr socket_for_client_result = + wifi_lan_client.Connect(service_id, discovered_service_info, &flag); + socket_for_client = std::move(socket_for_client_result.value()); + Base64Utils::WriteInt(&socket_for_client_result.value().GetOutputStream(), + 4); }); EXPECT_TRUE(accept_latch.Await(kWaitDuration).result()); EXPECT_TRUE(wifi_lan_server.StopAcceptingConnections(service_id)); @@ -219,7 +223,7 @@ TEST_P(WifiLanTest, CanCancelConnect) { ASSERT_TRUE(discovered_service_info.IsValid()); CancellationFlag flag(true); - WifiLanSocket socket_for_client = + ErrorOr socket_for_client_result = wifi_lan_client.Connect(service_id, discovered_service_info, &flag); // If FeatureFlag is disabled, Cancelled is false as no-op. if (!feature_flags.enable_cancellation_flag) { @@ -227,13 +231,14 @@ TEST_P(WifiLanTest, CanCancelConnect) { EXPECT_TRUE(wifi_lan_server.StopAcceptingConnections(service_id)); EXPECT_TRUE(wifi_lan_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()); } else { EXPECT_FALSE(accept_latch.Await(kWaitDuration).result()); EXPECT_TRUE(wifi_lan_server.StopAcceptingConnections(service_id)); EXPECT_TRUE(wifi_lan_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 2c873034..db2d67be 100644 --- a/connections/implementation/p2p_cluster_pcp_handler.cc +++ b/connections/implementation/p2p_cluster_pcp_handler.cc @@ -2848,30 +2848,34 @@ BasePcpHandler::ConnectImplResult P2pClusterPcpHandler::WifiLanConnectImpl( NEARBY_LOGS(INFO) << "Client " << client->GetClientId() << " is attempting to connect to endpoint(id=" << endpoint->endpoint_id << ") over WifiLan."; - WifiLanSocket socket = wifi_lan_medium_.Connect( + ErrorOr socket_result = wifi_lan_medium_.Connect( endpoint->service_id, endpoint->service_info, client->GetCancellationFlag(endpoint->endpoint_id)); - if (!socket.IsValid()) { + if (socket_result.has_error()) { NEARBY_LOGS(ERROR) << "In WifiLanConnectImpl(), failed to connect to service " << endpoint->service_info.GetServiceName() << " for endpoint(id=" << endpoint->endpoint_id << ")."; return BasePcpHandler::ConnectImplResult{ .status = {Status::kWifiLanError}, + .operation_result_code = + socket_result.error().operation_result_code().value(), }; } NEARBY_LOGS(INFO) << "In WifiLanConnectImpl(), connect to service " - << " socket=" << &socket.GetImpl() + << " socket=" << &socket_result.value().GetImpl() << " for endpoint(id=" << endpoint->endpoint_id << ")."; auto channel = std::make_unique( - endpoint->service_id, /*channel_name=*/endpoint->endpoint_id, socket); + endpoint->service_id, /*channel_name=*/endpoint->endpoint_id, + socket_result.value()); NEARBY_LOGS(INFO) << "Client " << client->GetClientId() << " created WifiLan endpoint channel to endpoint(id=" << endpoint->endpoint_id << ")."; return BasePcpHandler::ConnectImplResult{ .medium = WIFI_LAN, .status = {Status::kSuccess}, + .operation_result_code = OperationResultCode::DETAIL_SUCCESS, .endpoint_channel = std::move(channel), }; } diff --git a/connections/implementation/wifi_lan_bwu_handler.cc b/connections/implementation/wifi_lan_bwu_handler.cc index 04c751f6..22a7d624 100644 --- a/connections/implementation/wifi_lan_bwu_handler.cc +++ b/connections/implementation/wifi_lan_bwu_handler.cc @@ -39,7 +39,6 @@ namespace { using ::location::nearby::proto::connections::OperationResultCode; } // namespace -// TODO(edwinwu): Add exact OperationResultCode for WifiLanBwuHandler. WifiLanBwuHandler::WifiLanBwuHandler( Mediums& mediums, IncomingConnectionCallback incoming_connection_callback) : BaseBwuHandler(std::move(incoming_connection_callback)), @@ -52,14 +51,15 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( ClientProxy* client, const std::string& service_id, const std::string& endpoint_id, const UpgradePathInfo& upgrade_path_info) { if (!upgrade_path_info.has_wifi_lan_socket()) { - return {Error(OperationResultCode::DETAIL_UNKNOWN)}; + return { + Error(OperationResultCode::CONNECTIVITY_WIFI_LAN_INVALID_CREDENTIAL)}; } const UpgradePathInfo::WifiLanSocket& upgrade_path_info_socket = upgrade_path_info.wifi_lan_socket(); if (!upgrade_path_info_socket.has_ip_address() || !upgrade_path_info_socket.has_wifi_port()) { NEARBY_LOGS(ERROR) << "WifiLanBwuHandler failed to parse UpgradePathInfo."; - return {Error(OperationResultCode::DETAIL_UNKNOWN)}; + return {Error(OperationResultCode::CONNECTIVITY_WIFI_LAN_IP_ADDRESS_ERROR)}; } const std::string& ip_address = upgrade_path_info_socket.ip_address(); @@ -69,14 +69,14 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( << "available WifiLan service (" << ip_address << ":" << port << ") for endpoint " << endpoint_id; - WifiLanSocket socket = wifi_lan_medium_.Connect( + ErrorOr socket_result = wifi_lan_medium_.Connect( service_id, ip_address, port, client->GetCancellationFlag(endpoint_id)); - if (!socket.IsValid()) { + if (socket_result.has_error()) { NEARBY_LOGS(ERROR) << "WifiLanBwuHandler failed to connect to the WifiLan service (" << WifiUtils::GetHumanReadableIpAddress(ip_address) << ":" << port << ") for endpoint " << endpoint_id; - return {Error(OperationResultCode::DETAIL_UNKNOWN)}; + return {Error(socket_result.error().operation_result_code().value())}; } NEARBY_VLOG(1) @@ -86,13 +86,14 @@ WifiLanBwuHandler::CreateUpgradedEndpointChannel( // Create a new WifiLanEndpointChannel. auto channel = std::make_unique( - service_id, /*channel_name=*/service_id, socket); + service_id, /*channel_name=*/service_id, socket_result.value()); if (channel == nullptr) { NEARBY_LOGS(ERROR) << "WifiLanBwuHandler failed to create WifiLan endpoint " << "channel to the WifiLan service (" << ip_address << ":" << port << ") for endpoint " << endpoint_id; - socket.Close(); - return {Error(OperationResultCode::DETAIL_UNKNOWN)}; + socket_result.value().Close(); + return {Error( + OperationResultCode::NEARBY_LAN_ENDPOINT_CHANNEL_CREATION_FAILURE)}; } return {std::move(channel)};