From da4ac95e82436be46b78880cb5c7f4f9d7ca865d Mon Sep 17 00:00:00 2001 From: Guogang Li Date: Fri, 2 Dec 2022 15:08:18 -0800 Subject: [PATCH] Check connectivity before report Wi-Fi discovery PiperOrigin-RevId: 492570605 --- .../implementation/windows/wifi_lan.h | 26 +++- .../implementation/windows/wifi_lan_medium.cc | 128 +++++++++++++++++- 2 files changed, 148 insertions(+), 6 deletions(-) diff --git a/internal/platform/implementation/windows/wifi_lan.h b/internal/platform/implementation/windows/wifi_lan.h index aa3f474e..a819fd78 100644 --- a/internal/platform/implementation/windows/wifi_lan.h +++ b/internal/platform/implementation/windows/wifi_lan.h @@ -16,8 +16,10 @@ #define PLATFORM_IMPL_WINDOWS_WIFI_LAN_H_ // Windows headers +// clang-format off #include // NOLINT #include // NOLINT +// clang-format on // Standard C/C++ headers #include @@ -28,14 +30,17 @@ // Nearby connections headers #include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_map.h" -#include "absl/container/flat_hash_set.h" +#include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" +#include "absl/time/time.h" #include "absl/types/optional.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/exception.h" #include "internal/platform/implementation/wifi_lan.h" +#include "internal/platform/implementation/windows/scheduled_executor.h" #include "internal/platform/input_stream.h" #include "internal/platform/mutex.h" +#include "internal/platform/nsd_service_info.h" #include "internal/platform/output_stream.h" // WinRT headers @@ -57,6 +62,7 @@ using winrt::Windows::Devices::Enumeration::DeviceInformation; using winrt::Windows::Devices::Enumeration::DeviceInformationKind; using winrt::Windows::Devices::Enumeration::DeviceInformationUpdate; using winrt::Windows::Devices::Enumeration::DeviceWatcher; +using winrt::Windows::Devices::Enumeration::DeviceWatcherStatus; using winrt::Windows::Foundation::IInspectable; using winrt::Windows::Foundation::Collections::IMapView; using winrt::Windows::Networking::HostName; @@ -276,6 +282,15 @@ class WifiLanMedium : public api::WifiLanMedium { return (medium_status_ & kMediumStatusDiscovering) != 0; } + // Checks whether the IP address is connectable in the given timeout. + // Parameters: + // ip - IP string in format as 192.168.1.1 + // port - The IP port to connect. + // timeout - IP is not connectable if cannot connect in the duration. + // Result - return true if the IP is connectable, otherwise return false. + bool IsConnectableIpAddress(absl::string_view ip, int port, + absl::Duration timeout = absl::Seconds(1)); + // From mDNS device information, to build NsdServiceInfo. // the properties are from DeviceInformation and DeviceInformationUpdate. // The API gets IP addresses, service name and text attributes of mDNS @@ -296,6 +311,8 @@ class WifiLanMedium : public api::WifiLanMedium { // Gets error message from exception pointer std::string GetErrorMessage(std::exception_ptr eptr); + void RestartScanning(); + // // Dns-sd related properties // @@ -330,6 +347,13 @@ class WifiLanMedium : public api::WifiLanMedium { absl::flat_hash_map port_to_server_socket_map_; + + // Used to protect the access to mDNS instances and scanning related data. + absl::Mutex mutex_; + + // Keeps the discovered services in this time scanning. + absl::flat_hash_map discovered_services_map_ + ABSL_GUARDED_BY(mutex_); }; } // namespace windows diff --git a/internal/platform/implementation/windows/wifi_lan_medium.cc b/internal/platform/implementation/windows/wifi_lan_medium.cc index bfea872e..4a66092f 100644 --- a/internal/platform/implementation/windows/wifi_lan_medium.cc +++ b/internal/platform/implementation/windows/wifi_lan_medium.cc @@ -16,6 +16,7 @@ // Windows headers #include +#include // Standard C/C++ headers #include @@ -30,10 +31,14 @@ #include "absl/strings/string_view.h" // Nearby connections headers +#include "absl/synchronization/mutex.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" #include "internal/platform/cancellation_flag_listener.h" #include "internal/platform/exception.h" #include "internal/platform/implementation/windows/utils.h" #include "internal/platform/logging.h" +#include "internal/platform/runnable.h" namespace location { namespace nearby { @@ -52,6 +57,7 @@ constexpr absl::string_view kMdnsDeviceSelectorFormat = "AND System.Devices.Dnssd.ServiceName:=\"%s\" AND " "System.Devices.Dnssd.Domain:=\"local\""; +constexpr absl::Duration kConnectTimeout = absl::Seconds(1); } // namespace bool WifiLanMedium::IsNetworkConnected() const { @@ -277,6 +283,12 @@ bool WifiLanMedium::StartDiscovery(const std::string& service_type, device_watcher_removed_event_token = device_watcher_.Removed({this, &WifiLanMedium::Watcher_DeviceRemoved}); + // clear discovered mDNS instances. + { + absl::MutexLock lock(&mutex_); + discovered_services_map_.clear(); + } + device_watcher_.Start(); discovered_service_callback_ = std::move(callback); medium_status_ |= kMediumStatusDiscovering; @@ -386,7 +398,9 @@ std::unique_ptr WifiLanMedium::ListenForService( if (server_socket->listen()) { int port = server_socket_ptr->GetPort(); NEARBY_LOGS(INFO) << "started to listen serive on IP:port " - << server_socket_ptr->GetIPAddress() << ":" << port; + << ipaddr_4bytes_to_dotdecimal_string( + server_socket_ptr->GetIPAddress()) + << ":" << port; port_to_server_socket_map_.insert({port, server_socket_ptr}); server_socket->SetCloseNotifier([this, server_socket_ptr, port]() { @@ -530,7 +544,7 @@ fire_and_forget WifiLanMedium::Watcher_DeviceAdded( if (!nsd_service_info_except.ok()) { NEARBY_LOGS(WARNING) << "NSD information is incompleted or has error! " - "Don't add WIFI_LAN Medium"; + "Don't add WIFI_LAN device."; return fire_and_forget{}; } @@ -539,13 +553,13 @@ fire_and_forget WifiLanMedium::Watcher_DeviceAdded( nsd_service_info.GetTxtRecord(kDeviceEndpointInfo.data()); if (endpoint.empty()) { NEARBY_LOGS(WARNING) << "No endpoint information! " - "Don't add WIFI_LAN Medium"; + "Don't add WIFI_LAN device."; return fire_and_forget{}; } // Don't discover itself if (nsd_service_info.GetServiceName() == service_name_) { - NEARBY_LOGS(WARNING) << "Don't add WIFI_LAN Medium for itself"; + NEARBY_LOGS(WARNING) << "Don't add WIFI_LAN device for itself"; return fire_and_forget{}; } @@ -555,6 +569,20 @@ fire_and_forget WifiLanMedium::Watcher_DeviceAdded( nsd_service_info.GetIPAddress()) << ":" << nsd_service_info.GetPort(); + if (!IsConnectableIpAddress( + ipaddr_4bytes_to_dotdecimal_string(nsd_service_info.GetIPAddress()), + nsd_service_info.GetPort(), kConnectTimeout)) { + NEARBY_LOGS(WARNING) + << "Don't add WIFI_LAN device due to it is not reachable."; + return fire_and_forget{}; + } + + { + absl::MutexLock lock(&mutex_); + discovered_services_map_.emplace(nsd_service_info.GetServiceName(), + nsd_service_info); + } + discovered_service_callback_.service_discovered_cb(nsd_service_info); return fire_and_forget(); @@ -577,10 +605,43 @@ fire_and_forget WifiLanMedium::Watcher_DeviceUpdated( // Don't discover itself if (nsd_service_info.GetServiceName() == service_name_) { - NEARBY_LOGS(WARNING) << "Don't update WIFI_LAN Medium for itself"; + NEARBY_LOGS(WARNING) << "Don't update WIFI_LAN device for itself."; return fire_and_forget{}; } + // check having any changes + { + absl::MutexLock lock(&mutex_); + const auto& it = + discovered_services_map_.find(nsd_service_info.GetServiceName()); + + if (it == discovered_services_map_.end()) { + NEARBY_LOGS(WARNING) + << "Don't update WIFI_LAN device due to it is not in device list."; + return fire_and_forget{}; + } + + if (it->second.GetTxtRecord(std::string(kDeviceEndpointInfo)) == + nsd_service_info.GetTxtRecord(std::string(kDeviceEndpointInfo))) { + NEARBY_LOGS(INFO) + << "Don't update WIFI_LAN device due to no endpoint change."; + return fire_and_forget{}; + } + + NEARBY_LOGS(WARNING) + << "Endpoint is changed from " + << it->second.GetTxtRecord(std::string(kDeviceEndpointInfo)) << " to " + << nsd_service_info.GetTxtRecord(std::string(kDeviceEndpointInfo)); + + // Report device lost first. + discovered_service_callback_.service_lost_cb(it->second); + discovered_services_map_[nsd_service_info.GetServiceName()] = + nsd_service_info; + + // Report the updated device discovered. + discovered_service_callback_.service_discovered_cb(nsd_service_info); + } + NEARBY_LOGS(INFO) << "device updated for service name: " << nsd_service_info.GetServiceName() << ", address: " << ipaddr_4bytes_to_dotdecimal_string( @@ -612,11 +673,68 @@ fire_and_forget WifiLanMedium::Watcher_DeviceRemoved( return fire_and_forget{}; } + { + absl::MutexLock lock(&mutex_); + const auto& it = + discovered_services_map_.find(nsd_service_info.GetServiceName()); + + if (it == discovered_services_map_.end()) { + NEARBY_LOGS(WARNING) << "Cannot remove the WIFI_LAN device due to it is " + "not in device list."; + return fire_and_forget{}; + } + + discovered_services_map_.erase(it); + } + discovered_service_callback_.service_lost_cb(nsd_service_info); return fire_and_forget(); } +bool WifiLanMedium::IsConnectableIpAddress(absl::string_view ip, int port, + absl::Duration timeout) { + bool result = false; + int error = -1; + int size = sizeof(int); + timeval tm; + fd_set set; + unsigned long non_blocking = 1; // NOLINT + struct sockaddr_in serv_addr; + SOCKET sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); + serv_addr.sin_family = AF_INET; + serv_addr.sin_port = htons(port); + serv_addr.sin_addr.S_un.S_addr = inet_addr(std::string(ip).c_str()); + + ioctlsocket(sock, /*cmd=*/FIONBIO, /*argp=*/&non_blocking); + if (connect(sock, (struct sockaddr*)&serv_addr, sizeof(serv_addr)) == + SOCKET_ERROR) { + tm.tv_sec = timeout / absl::Seconds(1); + tm.tv_usec = 0; + FD_ZERO(&set); + FD_SET(sock, &set); + + if (select(sock + 1, nullptr, &set, nullptr, &tm) > 0) { + getsockopt(sock, SOL_SOCKET, SO_ERROR, (char*)&error, + /*(socklen_t *)*/ &size); + result = error == 0; + } else { + result = false; + } + } else { + result = true; + } + + non_blocking = 0; + ioctlsocket(sock, /*cmd=*/FIONBIO, /*argp=*/&non_blocking); + + if (result) { + closesocket(sock); + } + + return result; +} + std::string WifiLanMedium::GetErrorMessage(std::exception_ptr eptr) { try { if (eptr) {