Refactor bluetooth code to use shared pointers instead of references.

This commit is contained in:
Vibhav Pant
2023-09-05 19:45:10 +05:30
parent df5800fc70
commit 4bf11c8db0
11 changed files with 85 additions and 94 deletions
@@ -17,7 +17,6 @@
#include "internal/platform/implementation/bluetooth_adapter.h"
#include "internal/platform/implementation/linux/bluetooth_adapter.h"
#include "internal/platform/implementation/linux/bluez.h"
#include "internal/platform/implementation/linux/dbus.h"
#include "internal/platform/implementation/linux/generated/dbus/bluez/adapter_client.h"
#include "internal/platform/logging.h"
@@ -95,8 +94,7 @@ std::string BluetoothAdapter::GetName() const {
}
}
bool BluetoothAdapter::SetName(absl::string_view name, bool persist) {
persist_name_ = persist;
bool BluetoothAdapter::SetName(absl::string_view name, bool /*persist*/) {
return SetName(name);
}
@@ -37,21 +37,17 @@ class BluezAdapter : public sdbus::ProxyInterfaces<org::bluez::Adapter1_proxy> {
class BluetoothAdapter : public api::BluetoothAdapter {
public:
BluetoothAdapter(const BluetoothAdapter &) = default;
BluetoothAdapter(BluetoothAdapter &&) = delete;
BluetoothAdapter &operator=(const BluetoothAdapter &) = default;
BluetoothAdapter &operator=(BluetoothAdapter &&) = delete;
BluetoothAdapter(sdbus::IConnection &system_bus,
const sdbus::ObjectPath &adapter_object_path)
: bluez_adapter_(
std::make_unique<BluezAdapter>(system_bus, adapter_object_path)) {}
std::make_shared<BluezAdapter>(system_bus, adapter_object_path)) {}
~BluetoothAdapter() override {
if (!persist_name_) {
NEARBY_LOGS(INFO) << __func__ << "Resetting adapter Alias";
try {
bluez_adapter_->Alias("");
} catch (const sdbus::Error &e) {
DBUS_LOG_PROPERTY_SET_ERROR(bluez_adapter_, "Alias", e);
}
}
}
~BluetoothAdapter() override = default;
bool SetStatus(Status status) override;
bool IsEnabled() const override;
@@ -82,8 +78,7 @@ class BluetoothAdapter : public api::BluetoothAdapter {
BluezAdapter &GetBluezAdapterObject() { return *bluez_adapter_; }
private:
std::unique_ptr<BluezAdapter> bluez_adapter_;
bool persist_name_;
std::shared_ptr<BluezAdapter> bluez_adapter_;
};
} // namespace linux
} // namespace nearby
@@ -62,7 +62,7 @@ void Profile::NewConnection(
auto device = devices_.get_device_by_path(device_object_path);
if (!device.has_value()) {
if (device == nullptr) {
NEARBY_LOGS(ERROR)
<< __func__
<< "NewConection called with a device object we don't know about: "
@@ -70,10 +70,10 @@ void Profile::NewConnection(
throw sdbus::Error("org.bluez.Error.Rejected", "Unknown object");
}
auto alias = device->get().Alias();
auto mac_addr = device->get().Address();
auto alias = device->Alias();
auto mac_addr = device->Address();
NEARBY_LOGS(VERBOSE) << __func__ << ": " << getObjectPath()
<< ": Connected to " << device->get().getObjectPath();
<< ": Connected to " << device->getObjectPath();
FDProperties props(fd_props);
@@ -88,7 +88,7 @@ void Profile::NewConnection(
void Profile::RequestDisconnection(
const sdbus::ObjectPath &device_object_path) {
auto device = devices_.get_device_by_path(device_object_path);
if (!device.has_value()) {
if (device == nullptr) {
NEARBY_LOGS(ERROR) << __func__ << ": " << getObjectPath()
<< ": RequestDisconnection called with a device object "
"we don't know about: "
@@ -96,7 +96,7 @@ void Profile::RequestDisconnection(
throw sdbus::Error("org.bluez.Error.Rejected", "Unknown object");
}
auto mac_addr = device->get().Address();
auto mac_addr = device->Address();
NEARBY_LOGS(VERBOSE) << __func__ << ": Disconnection requested for device "
<< device_object_path;
@@ -232,7 +232,7 @@ std::optional<sdbus::UnixFd> ProfileManager::GetServiceRecordFD(
// Listen for a connected profile on any device, returning the connected device
// with its FD.
std::optional<std::pair<std::reference_wrapper<BluetoothDevice>, sdbus::UnixFd>>
std::optional<std::pair<std::shared_ptr<BluetoothDevice>, sdbus::UnixFd>>
ProfileManager::GetServiceRecordFD(absl::string_view service_uuid,
CancellationFlag *cancellation_flag) {
if (!ProfileRegistered(service_uuid)) {
@@ -278,14 +278,14 @@ ProfileManager::GetServiceRecordFD(absl::string_view service_uuid,
if (it->second.empty()) profile->connections_.erase(it);
profile->connections_lock_.Unlock();
auto maybe_device = devices_.get_device_by_address(mac_addr);
if (!maybe_device.has_value()) {
auto device = devices_.get_device_by_address(mac_addr);
if (device == nullptr) {
NEARBY_LOGS(ERROR) << __func__ << ": Device " << mac_addr
<< " is no longer available";
return std::nullopt;
}
return std::pair(*maybe_device, std::move(fd));
return std::pair(device, std::move(fd));
}
} // namespace linux
@@ -128,8 +128,7 @@ class ProfileManager final
api::BluetoothDevice &remote_device, absl::string_view service_uuid,
CancellationFlag *cancellation_flag)
ABSL_LOCKS_EXCLUDED(registered_service_uuids_mutex_);
std::optional<
std::pair<std::reference_wrapper<BluetoothDevice>, sdbus::UnixFd>>
std::optional<std::pair<std::shared_ptr<BluetoothDevice>, sdbus::UnixFd>>
GetServiceRecordFD(absl::string_view service_uuid,
CancellationFlag *cancellation_flag)
ABSL_LOCKS_EXCLUDED(registered_service_uuids_mutex_);
@@ -34,14 +34,12 @@
namespace nearby {
namespace linux {
BluetoothClassicMedium::BluetoothClassicMedium(
sdbus::IConnection &system_bus,
const sdbus::ObjectPath &adapter_object_path)
BluetoothClassicMedium::BluetoothClassicMedium(sdbus::IConnection &system_bus,
BluetoothAdapter &adapter)
: ProxyInterfaces(system_bus, "org.bluez", "/"),
adapter_(
std::make_unique<BluetoothAdapter>(system_bus, adapter_object_path)),
adapter_(adapter),
devices_(std::make_unique<BluetoothDevices>(
system_bus, adapter_object_path, observers_)),
system_bus, adapter.GetObjectPath(), observers_)),
profile_manager_(
std::make_unique<ProfileManager>(system_bus, *devices_)) {
registerProxy();
@@ -52,12 +50,12 @@ void BluetoothClassicMedium::onInterfacesAdded(
const std::map<std::string, std::map<std::string, sdbus::Variant>>
&interfaces) {
auto path_prefix = absl::Substitute(
"$0/dev_", adapter_->GetBluezAdapterObject().getObjectPath());
"$0/dev_", adapter_.GetBluezAdapterObject().getObjectPath());
if (object.find(path_prefix) != 0) {
return;
}
if (devices_->get_device_by_path(object).has_value()) {
if (devices_->get_device_by_path(object) != nullptr) {
// Device already exists.
return;
}
@@ -65,15 +63,15 @@ void BluetoothClassicMedium::onInterfacesAdded(
if (interfaces.count(org::bluez::Device1_proxy::INTERFACE_NAME) == 1) {
NEARBY_LOGS(INFO) << __func__ << ": Encountered new device at " << object;
auto &device = devices_->add_new_device(object);
auto device = devices_->add_new_device(object);
if (discovery_cb_.has_value() &&
discovery_cb_->device_discovered_cb != nullptr) {
discovery_cb_->device_discovered_cb(device);
discovery_cb_->device_discovered_cb(*device);
}
for (const auto &observer : observers_.GetObservers()) {
observer->DeviceAdded(device);
observer->DeviceAdded(*device);
}
}
}
@@ -81,7 +79,7 @@ void BluetoothClassicMedium::onInterfacesAdded(
void BluetoothClassicMedium::onInterfacesRemoved(
const sdbus::ObjectPath &object,
const std::vector<std::string> &interfaces) {
auto path_prefix = absl::Substitute("$0/dev_", adapter_->GetObjectPath());
auto path_prefix = absl::Substitute("$0/dev_", adapter_.GetObjectPath());
if (object.find(path_prefix) != 0) {
return;
}
@@ -90,7 +88,7 @@ void BluetoothClassicMedium::onInterfacesRemoved(
if (interface == org::bluez::Device1_proxy::INTERFACE_NAME) {
{
auto device = devices_->get_device_by_path(object);
if (!device.has_value()) {
if (device == nullptr) {
NEARBY_LOGS(WARNING) << __func__
<< ": received InterfacesRemoved for a device "
"we don't know about: "
@@ -120,10 +118,10 @@ bool BluetoothClassicMedium::StartDiscovery(
try {
NEARBY_LOGS(INFO) << __func__ << ": Starting discovery on "
<< adapter_->GetObjectPath();
adapter_->GetBluezAdapterObject().StartDiscovery();
<< adapter_.GetObjectPath();
adapter_.GetBluezAdapterObject().StartDiscovery();
} catch (const sdbus::Error &e) {
DBUS_LOG_METHOD_CALL_ERROR(&adapter_->GetBluezAdapterObject(),
DBUS_LOG_METHOD_CALL_ERROR(&adapter_.GetBluezAdapterObject(),
"StartDiscovery", e);
discovery_cb_.reset();
return false;
@@ -133,7 +131,7 @@ bool BluetoothClassicMedium::StartDiscovery(
}
bool BluetoothClassicMedium::StopDiscovery() {
auto &adapter = adapter_->GetBluezAdapterObject();
auto &adapter = adapter_.GetBluezAdapterObject();
try {
NEARBY_LOGS(INFO) << __func__ << "Stopping discovery on "
@@ -153,7 +151,7 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
api::BluetoothDevice &remote_device, const std::string &service_uuid,
CancellationFlag *cancellation_flag) {
auto device_object_path = bluez::device_object_path(
adapter_->GetObjectPath(), remote_device.GetMacAddress());
adapter_.GetObjectPath(), remote_device.GetMacAddress());
if (!profile_manager_->ProfileRegistered(service_uuid)) {
if (!profile_manager_->Register("", service_uuid)) {
NEARBY_LOGS(ERROR) << __func__ << ": Could not register profile "
@@ -162,11 +160,10 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
}
}
auto maybe_device = devices_->get_device_by_path(device_object_path);
if (!maybe_device.has_value()) return nullptr;
auto device = devices_->get_device_by_path(device_object_path);
if (device == nullptr) return nullptr;
auto &device = maybe_device->get();
device.ConnectToProfile(service_uuid);
device->ConnectToProfile(service_uuid);
auto fd = profile_manager_->GetServiceRecordFD(remote_device, service_uuid,
cancellation_flag);
@@ -179,7 +176,7 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
}
return std::unique_ptr<api::BluetoothSocket>(
new BluetoothSocket(remote_device, fd.value()));
new BluetoothSocket(device, fd.value()));
}
std::unique_ptr<api::BluetoothServerSocket>
@@ -201,18 +198,18 @@ BluetoothClassicMedium::ListenForService(const std::string &service_name,
api::BluetoothDevice *BluetoothClassicMedium::GetRemoteDevice(
const std::string &mac_address) {
auto device = devices_->get_device_by_address(mac_address);
if (!device.has_value()) return nullptr;
if (device == nullptr) return nullptr;
return &(device->get());
return device.get();
}
std::unique_ptr<api::BluetoothPairing> BluetoothClassicMedium::CreatePairing(
api::BluetoothDevice &remote_device) {
auto device = devices_->get_device_by_address(remote_device.GetMacAddress());
if (!device.has_value()) return nullptr;
if (device == nullptr) return nullptr;
return std::unique_ptr<api::BluetoothPairing>(
new BluetoothPairing(*adapter_, *device));
new BluetoothPairing(adapter_, device));
}
} // namespace linux
@@ -48,7 +48,7 @@ class BluetoothClassicMedium final
BluetoothClassicMedium &operator=(const BluetoothClassicMedium &) = delete;
BluetoothClassicMedium &operator=(BluetoothClassicMedium &&) = delete;
BluetoothClassicMedium(sdbus::IConnection &system_bus,
const sdbus::ObjectPath &adapter_object_path);
BluetoothAdapter &adapter);
~BluetoothClassicMedium() override { unregisterProxy(); };
// https://developer.android.com/reference/android/bluetooth/BluetoothAdapter.html#startDiscovery()
@@ -119,7 +119,7 @@ class BluetoothClassicMedium final
const std::vector<std::string> &interfaces) override;
private:
std::unique_ptr<BluetoothAdapter> adapter_;
BluetoothAdapter adapter_;
std::unique_ptr<BluetoothDevices> devices_;
std::optional<BluetoothClassicMedium::DiscoveryCallback> discovery_cb_;
@@ -24,6 +24,7 @@
#include "internal/platform/exception.h"
#include "internal/platform/implementation/bluetooth_classic.h"
#include "internal/platform/implementation/linux/bluetooth_classic_device.h"
#include "internal/platform/input_stream.h"
#include "internal/platform/output_stream.h"
@@ -69,7 +70,7 @@ class BluetoothOutputStream : public nearby::OutputStream {
explicit BluetoothOutputStream(sdbus::UnixFd fd) : fd_(std::move(fd)){};
Exception Write(const ByteArray &data) override;
Exception Flush() override {return {Exception::kSuccess};}
Exception Flush() override { return {Exception::kSuccess}; }
Exception Close() override;
private:
@@ -78,8 +79,9 @@ class BluetoothOutputStream : public nearby::OutputStream {
class BluetoothSocket final : public api::BluetoothSocket {
public:
BluetoothSocket(api::BluetoothDevice &device, const sdbus::UnixFd &fd)
: device_(device), output_stream_(fd), input_stream_(fd) {}
BluetoothSocket(std::shared_ptr<BluetoothDevice> device,
const sdbus::UnixFd &fd)
: device_(std::move(device)), output_stream_(fd), input_stream_(fd) {}
nearby::InputStream &GetInputStream() override { return input_stream_; }
nearby::OutputStream &GetOutputStream() override { return output_stream_; }
@@ -89,10 +91,10 @@ class BluetoothSocket final : public api::BluetoothSocket {
return Exception{Exception::kSuccess};
}
api::BluetoothDevice *GetRemoteDevice() override { return &device_; };
api::BluetoothDevice *GetRemoteDevice() override { return device_.get(); };
private:
api::BluetoothDevice &device_;
std::shared_ptr<BluetoothDevice> device_;
BluetoothOutputStream output_stream_;
BluetoothInputStream input_stream_;
};
@@ -24,21 +24,19 @@
namespace nearby {
namespace linux {
std::optional<std::reference_wrapper<BluetoothDevice>>
BluetoothDevices::get_device_by_path(
std::shared_ptr<BluetoothDevice> BluetoothDevices::get_device_by_path(
const sdbus::ObjectPath &device_object_path) {
absl::ReaderMutexLock l(&devices_by_path_lock_);
if (devices_by_path_.count(device_object_path) == 0) {
return std::nullopt;
return nullptr;
}
auto &device = devices_by_path_.at(device_object_path);
return *device;
return devices_by_path_[device_object_path];
}
std::optional<std::reference_wrapper<BluetoothDevice>>
BluetoothDevices::get_device_by_address(const std::string &addr) {
std::shared_ptr<BluetoothDevice> BluetoothDevices::get_device_by_address(
const std::string &addr) {
auto device_object_path =
bluez::device_object_path(adapter_object_path_, addr);
return get_device_by_path(device_object_path);
@@ -51,14 +49,14 @@ void BluetoothDevices::remove_device_by_path(
devices_by_path_.erase(device_object_path);
}
BluetoothDevice &BluetoothDevices::add_new_device(
std::shared_ptr<BluetoothDevice> BluetoothDevices::add_new_device(
sdbus::ObjectPath device_object_path) {
absl::MutexLock l(&devices_by_path_lock_);
auto pair = devices_by_path_.emplace(
std::string(device_object_path),
std::make_unique<MonitoredBluetoothDevice>(
system_bus_, std::move(device_object_path), observers_));
return *pair.first->second;
return pair.first->second;
}
} // namespace linux
} // namespace nearby
@@ -21,6 +21,7 @@
#include <sdbus-c++/IProxy.h>
#include <sdbus-c++/Types.h>
#include "absl/container/flat_hash_map.h"
#include "absl/synchronization/mutex.h"
#include "internal/base/observer_list.h"
#include "internal/platform/implementation/bluetooth_classic.h"
@@ -37,17 +38,18 @@ class BluetoothDevices final {
observers_(observers),
adapter_object_path_(std::move(adapter_object_path)) {}
std::optional<std::reference_wrapper<BluetoothDevice>> get_device_by_path(
const sdbus::ObjectPath &);
std::optional<std::reference_wrapper<BluetoothDevice>> get_device_by_address(
const std::string &);
void remove_device_by_path(const sdbus::ObjectPath &);
BluetoothDevice &add_new_device(sdbus::ObjectPath);
std::shared_ptr<BluetoothDevice> get_device_by_path(const sdbus::ObjectPath &)
ABSL_LOCKS_EXCLUDED(devices_by_path_lock_);
std::shared_ptr<BluetoothDevice> get_device_by_address(const std::string &);
void remove_device_by_path(const sdbus::ObjectPath &)
ABSL_LOCKS_EXCLUDED(devices_by_path_lock_);
std::shared_ptr<BluetoothDevice> add_new_device(sdbus::ObjectPath)
ABSL_LOCKS_EXCLUDED(devices_by_path_lock_);
private:
absl::Mutex devices_by_path_lock_;
std::map<std::string, std::unique_ptr<MonitoredBluetoothDevice>>
devices_by_path_;
absl::flat_hash_map<std::string, std::shared_ptr<MonitoredBluetoothDevice>>
devices_by_path_ ABSL_GUARDED_BY(devices_by_path_lock_);
sdbus::IConnection &system_bus_;
ObserverList<api::BluetoothClassicMedium::Observer> &observers_;
@@ -37,7 +37,7 @@ void BluetoothPairing::pairing_reply_handler(const sdbus::Error *error) {
<< "Got error '" << error->getName()
<< "' with message '" << error->getMessage()
<< "' while pairing with device "
<< device_.getObjectPath();
<< device_->getObjectPath();
if (name == "org.bluez.Error.AuthenticationCanceled") {
err = api::BluetoothPairingCallback::PairingError::kAuthCanceled;
@@ -60,9 +60,9 @@ void BluetoothPairing::pairing_reply_handler(const sdbus::Error *error) {
}
}
BluetoothPairing::BluetoothPairing(BluetoothAdapter &adapter,
BluetoothDevice &remote_device)
: device_(remote_device), adapter_(adapter) {}
BluetoothPairing::BluetoothPairing(
BluetoothAdapter &adapter, std::shared_ptr<BluetoothDevice> remote_device)
: device_(std::move(remote_device)), adapter_(adapter) {}
bool BluetoothPairing::InitiatePairing(
api::BluetoothPairingCallback pairing_cb) {
@@ -76,17 +76,17 @@ bool BluetoothPairing::InitiatePairing(
bool BluetoothPairing::FinishPairing(
std::optional<absl::string_view> pin_code) {
device_.set_pair_reply_callback([this](const sdbus::Error *error) {
device_->set_pair_reply_callback([this](const sdbus::Error *error) {
this->pairing_reply_handler(error);
});
try {
pair_async_call_ = device_.Pair();
pair_async_call_ = device_->Pair();
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to initiate pairing for device "
<< device_.getObjectPath();
<< device_->getObjectPath();
return false;
}
@@ -99,12 +99,12 @@ bool BluetoothPairing::CancelPairing() {
pair_async_call_.cancel();
}
device_.CancelPairing();
device_->CancelPairing();
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to cancel pairing for device "
<< device_.getObjectPath();
<< device_->getObjectPath();
return false;
}
@@ -113,13 +113,13 @@ bool BluetoothPairing::CancelPairing() {
bool BluetoothPairing::Unpair() {
try {
adapter_.RemoveDeviceByObjectPath(device_.getObjectPath());
adapter_.RemoveDeviceByObjectPath(device_->getObjectPath());
return true;
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to unpair device "
<< device_.getObjectPath() << " on adapter "
<< device_->getObjectPath() << " on adapter "
<< adapter_.GetObjectPath();
return false;
}
@@ -127,13 +127,13 @@ bool BluetoothPairing::Unpair() {
bool BluetoothPairing::IsPaired() {
try {
bool bonded = device_.Bonded();
bool bonded = device_->Bonded();
return bonded;
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to get Bonded state for device "
<< device_.getObjectPath();
<< device_->getObjectPath();
return false;
}
}
@@ -31,7 +31,7 @@ namespace nearby {
namespace linux {
class BluetoothPairing final : public api::BluetoothPairing {
public:
BluetoothPairing(BluetoothAdapter &adapter, BluetoothDevice &remote_device);
BluetoothPairing(BluetoothAdapter &adapter, std::shared_ptr<BluetoothDevice> remote_device);
bool InitiatePairing(api::BluetoothPairingCallback pairing_cb) override;
bool FinishPairing(std::optional<absl::string_view> pin_code) override;
@@ -44,7 +44,7 @@ class BluetoothPairing final : public api::BluetoothPairing {
sdbus::PendingAsyncCall pair_async_call_;
BluetoothDevice &device_;
std::shared_ptr<BluetoothDevice> device_;
linux::BluetoothAdapter &adapter_;
api::BluetoothPairingCallback pairing_cb_;