From 6b037b1b81325c18459a6d5edca63ea3122653ea Mon Sep 17 00:00:00 2001 From: Vibhav Pant Date: Wed, 6 Sep 2023 23:08:06 +0530 Subject: [PATCH] Refactor --- .../platform/implementation/linux/avahi.h | 12 ++-- .../linux/bluetooth_bluez_profile.cc | 70 +++++++++---------- .../linux/bluetooth_bluez_profile.h | 3 +- .../linux/bluetooth_classic_medium.cc | 2 +- .../linux/bluetooth_classic_medium.h | 19 +++-- .../linux/bluetooth_classic_server_socket.cc | 7 +- .../platform/implementation/linux/bluez.cc | 18 ++++- .../platform/implementation/linux/bluez.h | 9 ++- 8 files changed, 79 insertions(+), 61 deletions(-) diff --git a/internal/platform/implementation/linux/avahi.h b/internal/platform/implementation/linux/avahi.h index 5ba38b4e..e7ed6e20 100644 --- a/internal/platform/implementation/linux/avahi.h +++ b/internal/platform/implementation/linux/avahi.h @@ -15,9 +15,10 @@ #ifndef PLATFORM_IMPL_LINUX_AVAHI_H_ #define PLATFORM_IMPL_LINUX_AVAHI_H_ +#include + #include #include -#include #include "internal/platform/implementation/linux/dbus.h" #include "internal/platform/implementation/linux/generated/dbus/avahi/entrygroup_client.h" @@ -28,7 +29,7 @@ namespace nearby { namespace linux { namespace avahi { -class Server +class Server final : public sdbus::ProxyInterfaces { public: Server(sdbus::IConnection &system_bus) @@ -42,7 +43,7 @@ class Server } }; -class EntryGroup +class EntryGroup final : public sdbus::ProxyInterfaces { public: EntryGroup(sdbus::IConnection &system_bus, @@ -69,8 +70,9 @@ class EntryGroup } }; -class ServiceBrowser : public sdbus::ProxyInterfaces< - org::freedesktop::Avahi::ServiceBrowser_proxy> { +class ServiceBrowser final + : public sdbus::ProxyInterfaces< + org::freedesktop::Avahi::ServiceBrowser_proxy> { public: ServiceBrowser(sdbus::IConnection &system_bus, const sdbus::ObjectPath &service_browser_object_path, diff --git a/internal/platform/implementation/linux/bluetooth_bluez_profile.cc b/internal/platform/implementation/linux/bluetooth_bluez_profile.cc index f28128ae..2f0e1441 100644 --- a/internal/platform/implementation/linux/bluetooth_bluez_profile.cc +++ b/internal/platform/implementation/linux/bluetooth_bluez_profile.cc @@ -46,7 +46,7 @@ bool ProfileManager::ProfileRegistered(absl::string_view service_uuid) { void Profile::Release() { released_ = true; - NEARBY_LOGS(VERBOSE) << __func__ << "Profile object " << getObjectPath() + NEARBY_LOGS(VERBOSE) << __func__ << ": Profile object " << getObjectPath() << " has been released"; } @@ -54,7 +54,8 @@ void Profile::NewConnection( const sdbus::ObjectPath &device_object_path, const sdbus::UnixFd &fd, const std::map &fd_props) { if (released_) { - NEARBY_LOGS(ERROR) << __func__ << "NewConnection called on released object " + NEARBY_LOGS(ERROR) << __func__ + << ": NewConnection called on released object " << getObjectPath(); throw sdbus::Error("org.bluez.Error.Rejected", "NewConnection called on released object"); @@ -65,7 +66,7 @@ void Profile::NewConnection( if (device == nullptr) { NEARBY_LOGS(ERROR) << __func__ - << "NewConection called with a device object we don't know about: " + << ": NewConection called with a device object we don't know about: " << device_object_path; throw sdbus::Error("org.bluez.Error.Rejected", "Unknown object"); } @@ -78,11 +79,7 @@ void Profile::NewConnection( FDProperties props(fd_props); absl::MutexLock l(&connections_lock_); - if (connections_.count(mac_addr) != 0) { - connections_[mac_addr].push_back(std::pair(fd, props)); - } else { - connections_[mac_addr] = std::vector{std::pair(fd, props)}; - } + connections_[mac_addr].push_back(std::pair(fd, props)); } void Profile::RequestDisconnection( @@ -104,7 +101,7 @@ void Profile::RequestDisconnection( if (connections_.count(mac_addr) == 0) { NEARBY_LOGS(ERROR) << __func__ - << "Disconnection requested, but we are not connected to this device"; + << ": Disconnection requested, but we are not connected to this device"; return; } @@ -113,7 +110,8 @@ void Profile::RequestDisconnection( bool ProfileManager::Register(std::optional name, absl::string_view service_uuid) { - if (ProfileRegistered(service_uuid)) { + absl::MutexLock l(®istered_service_uuids_mutex_); + if (registered_services_.count(std::string(service_uuid)) == 1) { NEARBY_LOGS(WARNING) << __func__ << ": Trying to register profile " << service_uuid << " which was already registered."; return true; @@ -139,10 +137,7 @@ bool ProfileManager::Register(std::optional name, return false; } - { - absl::MutexLock l(®istered_service_uuids_mutex_); - registered_services_.emplace(service_uuid, profile); - } + registered_services_.emplace(service_uuid, profile); NEARBY_LOGS(INFO) << __func__ << ": Registered profile instancefor service uuid " @@ -152,7 +147,8 @@ bool ProfileManager::Register(std::optional name, } void ProfileManager::Unregister(absl::string_view service_uuid) { - if (!ProfileRegistered(service_uuid)) { + absl::MutexLock l(®istered_service_uuids_mutex_); + if (registered_services_.count(std::string(service_uuid)) == 0) { NEARBY_LOGS(WARNING) << __func__ << ": attempted to unregister a profile that is not registered"; @@ -169,10 +165,7 @@ void ProfileManager::Unregister(absl::string_view service_uuid) { BLUEZ_LOG_METHOD_CALL_ERROR(&getProxy(), "UnregisterProfile", e); } - { - absl::MutexLock l(®istered_service_uuids_mutex_); - registered_services_.erase(std::string(service_uuid)); - } + registered_services_.erase(std::string(service_uuid)); } // Get a service record FD for a connected profile (identified by service_uuid) @@ -180,18 +173,18 @@ void ProfileManager::Unregister(absl::string_view service_uuid) { std::optional ProfileManager::GetServiceRecordFD( api::BluetoothDevice &remote_device, absl::string_view service_uuid, CancellationFlag *cancellation_flag) { - if (!ProfileRegistered(service_uuid)) { - NEARBY_LOGS(ERROR) << __func__ << ": Service " << service_uuid - << " is not registered"; - return std::nullopt; + std::shared_ptr profile; + { + absl::ReaderMutexLock lock(®istered_service_uuids_mutex_); + if (registered_services_.count(std::string(service_uuid)) == 0) { + NEARBY_LOGS(ERROR) << __func__ << ": Service " << service_uuid + << " is not registered"; + return std::nullopt; + } + profile = registered_services_[std::string(service_uuid)]; } - auto mac_addr = remote_device.GetMacAddress(); - registered_service_uuids_mutex_.ReaderLock(); - auto profile = registered_services_[std::string(service_uuid)]; - registered_service_uuids_mutex_.ReaderUnlock(); - std::unique_ptr cancel_listener; if (cancellation_flag != nullptr) cancel_listener = std::make_unique( @@ -235,13 +228,16 @@ std::optional ProfileManager::GetServiceRecordFD( std::optional, sdbus::UnixFd>> ProfileManager::GetServiceRecordFD(absl::string_view service_uuid, CancellationFlag *cancellation_flag) { - if (!ProfileRegistered(service_uuid)) { - return std::nullopt; - } + std::shared_ptr profile; - registered_service_uuids_mutex_.ReaderLock(); - auto profile = registered_services_[std::string(service_uuid)]; - registered_service_uuids_mutex_.ReaderUnlock(); + { + absl::ReaderMutexLock lock(®istered_service_uuids_mutex_); + if (registered_services_.count(std::string(service_uuid)) == 0) { + return std::nullopt; + } + + profile = registered_services_[std::string(service_uuid)]; + } NEARBY_LOGS(VERBOSE) << __func__ << ": " << profile->getObjectPath() << ": Attempting to get a FD for service " @@ -264,9 +260,9 @@ ProfileManager::GetServiceRecordFD(absl::string_view service_uuid, profile->connections_lock_.Await(absl::Condition(&cond)); if (cancellation_flag != nullptr && cancellation_flag->Cancelled()) { - NEARBY_LOGS(VERBOSE) << __func__ - << "Cancelled waiting for new connections on profile " - << profile->getObjectPath(); + NEARBY_LOGS(VERBOSE) + << __func__ << ": Cancelled waiting for new connections on profile " + << profile->getObjectPath(); profile->connections_lock_.Unlock(); return std::nullopt; } diff --git a/internal/platform/implementation/linux/bluetooth_bluez_profile.h b/internal/platform/implementation/linux/bluetooth_bluez_profile.h index 5e025b11..31cc1bf3 100644 --- a/internal/platform/implementation/linux/bluetooth_bluez_profile.h +++ b/internal/platform/implementation/linux/bluetooth_bluez_profile.h @@ -47,7 +47,8 @@ namespace linux { class ProfileManager; class Profile final - : public sdbus::AdaptorInterfaces { + : public sdbus::AdaptorInterfaces { public: Profile(const Profile &) = delete; Profile(Profile &&) = delete; diff --git a/internal/platform/implementation/linux/bluetooth_classic_medium.cc b/internal/platform/implementation/linux/bluetooth_classic_medium.cc index 3d3daa95..c1001391 100644 --- a/internal/platform/implementation/linux/bluetooth_classic_medium.cc +++ b/internal/platform/implementation/linux/bluetooth_classic_medium.cc @@ -38,7 +38,7 @@ BluetoothClassicMedium::BluetoothClassicMedium(sdbus::IConnection &system_bus, BluetoothAdapter &adapter) : ProxyInterfaces(system_bus, "org.bluez", "/"), adapter_(adapter), - devices_(std::make_unique( + devices_(std::make_shared( system_bus, adapter.GetObjectPath(), observers_)), profile_manager_( std::make_unique(system_bus, *devices_)) { diff --git a/internal/platform/implementation/linux/bluetooth_classic_medium.h b/internal/platform/implementation/linux/bluetooth_classic_medium.h index 40d369a7..9a3b120a 100644 --- a/internal/platform/implementation/linux/bluetooth_classic_medium.h +++ b/internal/platform/implementation/linux/bluetooth_classic_medium.h @@ -25,23 +25,20 @@ #include #include #include -#include -#include "absl/synchronization/mutex.h" #include "internal/base/observer_list.h" #include "internal/platform/implementation/bluetooth_classic.h" #include "internal/platform/implementation/linux/bluetooth_adapter.h" #include "internal/platform/implementation/linux/bluetooth_bluez_profile.h" -#include "internal/platform/implementation/linux/bluetooth_classic_device.h" #include "internal/platform/implementation/linux/bluetooth_devices.h" namespace nearby { namespace linux { // Container of operations that can be performed over the Bluetooth Classic // medium. -class BluetoothClassicMedium final +class BluetoothClassicMedium : public api::BluetoothClassicMedium, - sdbus::ProxyInterfaces { + protected sdbus::ProxyInterfaces { public: BluetoothClassicMedium(const BluetoothClassicMedium &) = delete; BluetoothClassicMedium(BluetoothClassicMedium &&) = delete; @@ -120,12 +117,14 @@ class BluetoothClassicMedium final private: BluetoothAdapter adapter_; - std::unique_ptr devices_; - - std::shared_ptr discovery_cb_; - - std::unique_ptr profile_manager_; ObserverList observers_; + + protected: + std::shared_ptr devices_; + + private: + std::shared_ptr discovery_cb_; + std::unique_ptr profile_manager_; }; } // namespace linux diff --git a/internal/platform/implementation/linux/bluetooth_classic_server_socket.cc b/internal/platform/implementation/linux/bluetooth_classic_server_socket.cc index 489423cc..f70162c8 100644 --- a/internal/platform/implementation/linux/bluetooth_classic_server_socket.cc +++ b/internal/platform/implementation/linux/bluetooth_classic_server_socket.cc @@ -33,9 +33,10 @@ std::unique_ptr BluetoothServerSocket::Accept() { auto pair = profile_manager_.GetServiceRecordFD(service_uuid_, &stopped_); if (!pair.has_value()) { - NEARBY_LOGS(ERROR) << __func__ - << "Failed to get a new connection for profile " - << service_uuid_; + if (!stopped_.Cancelled()) + NEARBY_LOGS(ERROR) << __func__ + << ": Failed to get a new connection for profile " + << service_uuid_; return nullptr; } diff --git a/internal/platform/implementation/linux/bluez.cc b/internal/platform/implementation/linux/bluez.cc index 61bf0a02..ddd0eb55 100644 --- a/internal/platform/implementation/linux/bluez.cc +++ b/internal/platform/implementation/linux/bluez.cc @@ -28,14 +28,28 @@ std::string device_object_path(const sdbus::ObjectPath &adapter_object_path, } sdbus::ObjectPath profile_object_path(absl::string_view service_uuid) { - return absl::Substitute("/com/google/nearby/profiles/$0", - absl::StrReplaceAll(service_uuid, {{"-", "_"}})); + return absl::Substitute( + "/com/google/nearby/medium/bluetooth_classic/profiles/$0", + absl::StrReplaceAll(service_uuid, {{"-", "_"}})); } sdbus::ObjectPath adapter_object_path(absl::string_view name) { return absl::Substitute("/org/bluez/$0", name); } +sdbus::ObjectPath gatt_service_path(size_t num) { + return absl::Substitute("$0/service$1", NEARBY_BLE_GATT_PATH_ROOT, num); +} + +sdbus::ObjectPath gatt_characteristic_path( + const sdbus::ObjectPath &service_path, size_t num) { + return absl::Substitute("$0/char$1", service_path, num); +} + +sdbus::ObjectPath ble_advertisement_path(absl::string_view uuid) { + return absl::Substitute("/com/google/nearby/medium/ble/advertisement/$0", uuid); +} + } // namespace bluez } // namespace linux } // namespace nearby diff --git a/internal/platform/implementation/linux/bluez.h b/internal/platform/implementation/linux/bluez.h index 049503ae..12dbcb33 100644 --- a/internal/platform/implementation/linux/bluez.h +++ b/internal/platform/implementation/linux/bluez.h @@ -45,12 +45,17 @@ static constexpr const char *DEVICE_PROP_PAIRED = "Paired"; static constexpr const char *DEVICE_PROP_CONNECTED = "Connected"; static constexpr const char *DEVICE_NAME = "Name"; +static constexpr const char *NEARBY_BLE_GATT_PATH_ROOT = + "/com/google/nearby/medium/ble/gatt"; + std::string device_object_path(const sdbus::ObjectPath &adapter_object_path, absl::string_view mac_address); - sdbus::ObjectPath profile_object_path(absl::string_view service_uuid); - sdbus::ObjectPath adapter_object_path(absl::string_view name); +sdbus::ObjectPath gatt_service_path(size_t num); +sdbus::ObjectPath gatt_characteristic_path( + const sdbus::ObjectPath &service_path, size_t num); +sdbus::ObjectPath ble_advertisement_path(absl::string_view uuid); class BluezObjectManager : public sdbus::ProxyInterfaces {