From 622451436b650671179fbb46d92a70134c5eb693 Mon Sep 17 00:00:00 2001 From: Vibhav Pant Date: Thu, 3 Aug 2023 17:14:03 +0530 Subject: [PATCH] Refactor --- .../implementation/linux/bluetooth_adapter.cc | 20 +++++++++---------- .../implementation/linux/bluetooth_adapter.h | 9 +++------ .../linux/bluetooth_classic_device.cc | 8 ++++++++ .../linux/bluetooth_classic_device.h | 1 + .../linux/bluetooth_classic_medium.cc | 17 +++++++--------- .../linux/bluetooth_classic_medium.h | 4 ++-- .../implementation/linux/bluetooth_pairing.cc | 5 +++-- .../implementation/linux/bluetooth_pairing.h | 1 - 8 files changed, 34 insertions(+), 31 deletions(-) diff --git a/internal/platform/implementation/linux/bluetooth_adapter.cc b/internal/platform/implementation/linux/bluetooth_adapter.cc index cc734974..b24c5395 100644 --- a/internal/platform/implementation/linux/bluetooth_adapter.cc +++ b/internal/platform/implementation/linux/bluetooth_adapter.cc @@ -15,8 +15,8 @@ bool BluetoothAdapter::SetStatus(Status status) { SD_BUS_ERROR_NULL; if (sd_bus_set_property( - system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, - "Powered", &err, "b", + system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", + BLUEZ_ADAPTER_INTERFACE, "Powered", &err, "b", status == api::BluetoothAdapter::Status::kEnabled ? 1 : 0) < 0) { NEARBY_LOGS(ERROR) << __func__ << ": Error setting adaptor status: " << err.message; @@ -30,7 +30,7 @@ bool BluetoothAdapter::IsEnabled() const { SD_BUS_ERROR_NULL; int enabled = 0; - if (sd_bus_get_property_trivial(system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", + if (sd_bus_get_property_trivial(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "Powered", &err, 'b', &enabled) < 0) { NEARBY_LOGS(ERROR) << __func__ @@ -45,7 +45,7 @@ BluetoothAdapter::ScanMode BluetoothAdapter::GetScanMode() const { int powered = 0; int discoverable = 0; - if (sd_bus_get_property_trivial(system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", + if (sd_bus_get_property_trivial(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "Powered", &err, 'b', &powered) < 0) { NEARBY_LOGS(ERROR) << __func__ @@ -56,7 +56,7 @@ BluetoothAdapter::ScanMode BluetoothAdapter::GetScanMode() const { return ScanMode::kNone; } - if (sd_bus_get_property_trivial(system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", + if (sd_bus_get_property_trivial(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "Discoverable", &err, 'b', &powered) < 0) { NEARBY_LOGS(ERROR) << __func__ @@ -78,7 +78,7 @@ bool BluetoothAdapter::SetScanMode(ScanMode scan_mode) { } __attribute__((cleanup(sd_bus_error_free))) sd_bus_error err = SD_BUS_ERROR_NULL; - if (sd_bus_set_property(system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", + if (sd_bus_set_property(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "Discoverable", &err, "b", 1) < 0) { NEARBY_LOGS(ERROR) << __func__ @@ -99,7 +99,7 @@ std::string BluetoothAdapter::GetName() const { __attribute__((cleanup(sd_bus_error_free))) sd_bus_error err = SD_BUS_ERROR_NULL; char *cname = nullptr; - if (sd_bus_get_property_string(system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", + if (sd_bus_get_property_string(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "Alias", &err, &cname) < 0) { NEARBY_LOGS(ERROR) << __func__ @@ -116,7 +116,7 @@ bool BluetoothAdapter::SetName(absl::string_view name, bool persist) { __attribute__((cleanup(sd_bus_error_free))) sd_bus_error err = SD_BUS_ERROR_NULL; std::string pretty_hostname(name); - if (sd_bus_set_property(system_bus, "org.freedesktop.hostname1", + if (sd_bus_set_property(system_bus_, "org.freedesktop.hostname1", "/org/freedesktop/hostname1", "org.freedesktop.hostname1", "PrettyHostname", &err, "s", pretty_hostname.c_str()) < 0) { @@ -131,7 +131,7 @@ bool BluetoothAdapter::SetName(absl::string_view name) { std::string alias(name); __attribute__((cleanup(sd_bus_error_free))) sd_bus_error err = SD_BUS_ERROR_NULL; - if (sd_bus_set_property(system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", + if (sd_bus_set_property(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "Alias", &err, "s", alias.c_str()) < 0) { NEARBY_LOGS(ERROR) << __func__ @@ -145,7 +145,7 @@ std::string BluetoothAdapter::GetMacAddress() const { __attribute__((cleanup(sd_bus_error_free))) sd_bus_error err = SD_BUS_ERROR_NULL; char *caddr = nullptr; - if (sd_bus_get_property_string(system_bus, BLUEZ_SERVICE, "/org/bluez/hci0", + if (sd_bus_get_property_string(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "Address", &err, &caddr) < 0) { NEARBY_LOGS(ERROR) << __func__ diff --git a/internal/platform/implementation/linux/bluetooth_adapter.h b/internal/platform/implementation/linux/bluetooth_adapter.h index 0ca0facf..b9c4b2c2 100644 --- a/internal/platform/implementation/linux/bluetooth_adapter.h +++ b/internal/platform/implementation/linux/bluetooth_adapter.h @@ -11,11 +11,8 @@ namespace nearby { namespace linux { class BluetoothAdapter : public api::BluetoothAdapter { public: - ~BluetoothAdapter() override { - if (system_bus) { - sd_bus_unrefp(&system_bus); - } - }; + BluetoothAdapter(sd_bus *bus) { system_bus_ = bus; } + ~BluetoothAdapter() override { sd_bus_unref(system_bus_); }; bool SetStatus(Status status) override; bool IsEnabled() const override; @@ -30,7 +27,7 @@ public: std::string GetMacAddress() const override; private: - sd_bus *system_bus; + sd_bus *system_bus_; }; } // namespace linux } // namespace nearby diff --git a/internal/platform/implementation/linux/bluetooth_classic_device.cc b/internal/platform/implementation/linux/bluetooth_classic_device.cc index cda7e74a..ad325e71 100644 --- a/internal/platform/implementation/linux/bluetooth_classic_device.cc +++ b/internal/platform/implementation/linux/bluetooth_classic_device.cc @@ -24,6 +24,14 @@ BluetoothDevice::BluetoothDevice(sd_bus *system_bus, object_path_ = device_object_path; } +BluetoothDevice::BluetoothDevice(const BluetoothDevice &device) { + if (!device.mac_addr_.empty()) { + mac_addr_ = device.mac_addr_; + } + object_path_ = device.object_path_; + system_bus_ = sd_bus_ref(device.system_bus_); +} + std::string BluetoothDevice::GetName() const { __attribute__((cleanup(sd_bus_error_free))) sd_bus_error err = SD_BUS_ERROR_NULL; diff --git a/internal/platform/implementation/linux/bluetooth_classic_device.h b/internal/platform/implementation/linux/bluetooth_classic_device.h index 098abc03..18e93795 100644 --- a/internal/platform/implementation/linux/bluetooth_classic_device.h +++ b/internal/platform/implementation/linux/bluetooth_classic_device.h @@ -15,6 +15,7 @@ public: BluetoothDevice(sd_bus *system_bus, absl::string_view adapter, absl::string_view address); BluetoothDevice(sd_bus *system_bus, absl::string_view device_object_path); + BluetoothDevice(const BluetoothDevice &device); ~BluetoothDevice() override { sd_bus_unref(system_bus_); }; diff --git a/internal/platform/implementation/linux/bluetooth_classic_medium.cc b/internal/platform/implementation/linux/bluetooth_classic_medium.cc index e53fa850..e298d6c0 100644 --- a/internal/platform/implementation/linux/bluetooth_classic_medium.cc +++ b/internal/platform/implementation/linux/bluetooth_classic_medium.cc @@ -55,7 +55,7 @@ int bluez_interfaces_added_signal_handler(sd_bus_message *m, void *userdata, ret = sd_bus_message_enter_container(m, 'a', "{sa{sv}}"); if (ret < 0) { NEARBY_LOGS(ERROR) << __func__ << "Error entering container: " << ret; - return 0; + return ret; } while (true) { @@ -63,6 +63,7 @@ int bluez_interfaces_added_signal_handler(sd_bus_message *m, void *userdata, ret = sd_bus_message_read(m, "s", &interface_name); if (ret < 0) { NEARBY_LOGS(ERROR) << __func__ << "Error reading dict entry: " << ret; + return ret; } if (ret == 0) break; @@ -149,13 +150,6 @@ bool BluetoothClassicMedium::StopDiscovery() { << adapter_object_path_ << ": " << err.message; return false; } - const sd_bus_error *m_err = sd_bus_message_get_error(reply); - - if (m_err) { - NEARBY_LOGS(ERROR) << __func__ << "Error calling StopDiscovery on " - << adapter_object_path_ << ": " << err.message; - return false; - } return true; } @@ -214,8 +208,11 @@ BluetoothClassicMedium::ListenForService(const std::string &service_name, api::BluetoothDevice * BluetoothClassicMedium::GetRemoteDevice(const std::string &mac_address) { - return new BluetoothDevice(sd_bus_ref(system_bus_), - GetDeviceObjectPath(mac_address)); + if (devices_by_path_.count(mac_address) == 1) { + return devices_by_path_[mac_address].get(); + } + + return nullptr; } std::unique_ptr diff --git a/internal/platform/implementation/linux/bluetooth_classic_medium.h b/internal/platform/implementation/linux/bluetooth_classic_medium.h index 1fd6886c..c4d5217d 100644 --- a/internal/platform/implementation/linux/bluetooth_classic_medium.h +++ b/internal/platform/implementation/linux/bluetooth_classic_medium.h @@ -95,10 +95,10 @@ private: sd_bus *system_bus_ = nullptr; sd_bus_slot *system_bus_slot_ = nullptr; std::string adapter_object_path_ = std::string(); - std::map> devices_by_id_; + std::map> devices_by_path_; ObserverList observers_; - DiscoveryParams discovery_params_ = {adapter_object_path_, devices_by_id_, + DiscoveryParams discovery_params_ = {adapter_object_path_, devices_by_path_, observers_}; }; } // namespace linux diff --git a/internal/platform/implementation/linux/bluetooth_pairing.cc b/internal/platform/implementation/linux/bluetooth_pairing.cc index 814a0162..3d82511f 100644 --- a/internal/platform/implementation/linux/bluetooth_pairing.cc +++ b/internal/platform/implementation/linux/bluetooth_pairing.cc @@ -5,6 +5,7 @@ #include "internal/platform/implementation/linux/bluetooth_classic_device.h" #include "internal/platform/implementation/linux/bluetooth_pairing.h" #include "internal/platform/implementation/linux/bluez.h" +#include "internal/platform/logging.h" namespace nearby { namespace linux { @@ -79,7 +80,7 @@ bool BluetoothPairing::CancelPairing() { SD_BUS_ERROR_NULL; if (sd_bus_call_method(system_bus_, BLUEZ_SERVICE, device_object_path_.c_str(), BLUEZ_DEVICE_INTERFACE, - "CancelPairing", &err, nullptr, nullptr)) { + "CancelPairing", &err, nullptr, nullptr) < 0) { NEARBY_LOGS(ERROR) << __func__ << "Error calling method CancelPairing on device " << device_object_path_ << ": " << err.message; @@ -96,7 +97,7 @@ bool BluetoothPairing::Unpair() { SD_BUS_ERROR_NULL; if (sd_bus_call_method(system_bus_, BLUEZ_SERVICE, "/org/bluez/hci0", BLUEZ_ADAPTER_INTERFACE, "RemoveDevice", &err, nullptr, - "o", device_object_path_.c_str())) { + "o", device_object_path_.c_str()) < 0) { NEARBY_LOGS(ERROR) << __func__ << "Error calling method CancelPairing on device " << device_object_path_ << ": " << err.message; diff --git a/internal/platform/implementation/linux/bluetooth_pairing.h b/internal/platform/implementation/linux/bluetooth_pairing.h index 1b1084f9..fa0f321d 100644 --- a/internal/platform/implementation/linux/bluetooth_pairing.h +++ b/internal/platform/implementation/linux/bluetooth_pairing.h @@ -7,7 +7,6 @@ #include "absl/strings/string_view.h" #include "internal/platform/implementation/bluetooth_classic.h" -#include "internal/platform/logging.h" namespace nearby { namespace linux {