service: Fix IBluetoothLowEnergy signatures to return bool
Changed the IBluetoothLowEnergy API signatures that can
synchronously fail to return bool to report synchronous errors. Also
fixed a bug with lambda capture by value in advertising API
implementation.
Bug: 24245347
Change-Id: I9ce4bb44d155d74ba38916e2834d7e93bf49301c
diff --git a/service/client/main.cpp b/service/client/main.cpp
index 52dd181..617d8f9 100644
--- a/service/client/main.cpp
+++ b/service/client/main.cpp
@@ -17,6 +17,8 @@
#include <iostream>
#include <string>
+#include <base/at_exit.h>
+#include <base/command_line.h>
#include <base/logging.h>
#include <base/macros.h>
#include <base/strings/string_split.h>
@@ -256,9 +258,9 @@
return;
}
- ble_iface->RegisterClient(new CLIBluetoothLowEnergyCallback());
- ble_registering = true;
- PrintCommandStatus(true);
+ bool status = ble_iface->RegisterClient(new CLIBluetoothLowEnergyCallback());
+ ble_registering = status;
+ PrintCommandStatus(status);
}
void HandleUnregisterBLE(IBluetooth* bt_iface, const vector<string>& args) {
@@ -361,8 +363,9 @@
bluetooth::AdvertiseData scan_rsp;
- ble_iface->StartMultiAdvertising(ble_client_if.load(),
- adv_data, scan_rsp, settings);
+ bool status = ble_iface->StartMultiAdvertising(ble_client_if.load(),
+ adv_data, scan_rsp, settings);
+ PrintCommandStatus(status);
}
void HandleStopAdv(IBluetooth* bt_iface, const vector<string>& args) {
@@ -377,7 +380,8 @@
return;
}
- ble_iface->StopMultiAdvertising(ble_client_if.load());
+ bool status = ble_iface->StopMultiAdvertising(ble_client_if.load());
+ PrintCommandStatus(status);
}
void HandleHelp(IBluetooth* bt_iface, const vector<string>& args);
@@ -441,7 +445,16 @@
DISALLOW_COPY_AND_ASSIGN(BluetoothDeathRecipient);
};
-int main() {
+int main(int argc, char* argv[]) {
+ base::AtExitManager exit_manager;
+ base::CommandLine::Init(argc, argv);
+ logging::LoggingSettings log_settings;
+
+ if (!logging::InitLogging(log_settings)) {
+ LOG(ERROR) << "Failed to set up logging";
+ return EXIT_FAILURE;
+ }
+
sp<IBluetooth> bt_iface = IBluetooth::getClientInterface();
if (!bt_iface.get()) {
LOG(ERROR) << "Failed to obtain handle on IBluetooth";
diff --git a/service/ipc/binder/IBluetoothLowEnergy.cpp b/service/ipc/binder/IBluetoothLowEnergy.cpp
index e9c0a43..1060170 100644
--- a/service/ipc/binder/IBluetoothLowEnergy.cpp
+++ b/service/ipc/binder/IBluetoothLowEnergy.cpp
@@ -49,7 +49,11 @@
switch (code) {
case REGISTER_CLIENT_TRANSACTION: {
sp<IBinder> callback = data.readStrongBinder();
- RegisterClient(interface_cast<IBluetoothLowEnergyCallback>(callback));
+ bool result = RegisterClient(
+ interface_cast<IBluetoothLowEnergyCallback>(callback));
+
+ reply->writeInt32(result);
+
return android::NO_ERROR;
}
case UNREGISTER_CLIENT_TRANSACTION: {
@@ -70,14 +74,18 @@
std::unique_ptr<AdvertiseSettings> adv_settings =
CreateAdvertiseSettingsFromParcel(data);
- StartMultiAdvertising(client_if, *adv_data, *scan_rsp, *adv_settings);
+ bool result = StartMultiAdvertising(
+ client_if, *adv_data, *scan_rsp, *adv_settings);
+
+ reply->writeInt32(result);
return android::NO_ERROR;
}
case STOP_MULTI_ADVERTISING_TRANSACTION: {
int client_if = data.readInt32();
+ bool result = StopMultiAdvertising(client_if);
- StopMultiAdvertising(client_if);
+ reply->writeInt32(result);
return android::NO_ERROR;
}
@@ -93,7 +101,7 @@
: BpInterface<IBluetoothLowEnergy>(impl) {
}
-void BpBluetoothLowEnergy::RegisterClient(
+bool BpBluetoothLowEnergy::RegisterClient(
const sp<IBluetoothLowEnergyCallback>& callback) {
Parcel data, reply;
@@ -102,6 +110,8 @@
remote()->transact(IBluetoothLowEnergy::REGISTER_CLIENT_TRANSACTION,
data, &reply);
+
+ return reply.readInt32();
}
void BpBluetoothLowEnergy::UnregisterClient(int client_if) {
@@ -123,7 +133,7 @@
data, &reply);
}
-void BpBluetoothLowEnergy::StartMultiAdvertising(
+bool BpBluetoothLowEnergy::StartMultiAdvertising(
int client_if,
const AdvertiseData& advertise_data,
const AdvertiseData& scan_response,
@@ -138,9 +148,11 @@
remote()->transact(IBluetoothLowEnergy::START_MULTI_ADVERTISING_TRANSACTION,
data, &reply);
+
+ return reply.readInt32();
}
-void BpBluetoothLowEnergy::StopMultiAdvertising(int client_if) {
+bool BpBluetoothLowEnergy::StopMultiAdvertising(int client_if) {
Parcel data, reply;
data.writeInterfaceToken(IBluetoothLowEnergy::getInterfaceDescriptor());
@@ -148,6 +160,8 @@
remote()->transact(IBluetoothLowEnergy::STOP_MULTI_ADVERTISING_TRANSACTION,
data, &reply);
+
+ return reply.readInt32();
}
IMPLEMENT_META_INTERFACE(BluetoothLowEnergy, IBluetoothLowEnergy::kServiceName);
diff --git a/service/ipc/binder/IBluetoothLowEnergy.h b/service/ipc/binder/IBluetoothLowEnergy.h
index 8ba8564..cd9d302 100644
--- a/service/ipc/binder/IBluetoothLowEnergy.h
+++ b/service/ipc/binder/IBluetoothLowEnergy.h
@@ -65,17 +65,17 @@
NUM_HW_TRACK_FILTERS_AVAILABLE,
};
- virtual void RegisterClient(
+ virtual bool RegisterClient(
const android::sp<IBluetoothLowEnergyCallback>& callback) = 0;
virtual void UnregisterClient(int client_if) = 0;
virtual void UnregisterAll() = 0;
- virtual void StartMultiAdvertising(
+ virtual bool StartMultiAdvertising(
int client_if,
const bluetooth::AdvertiseData& advertise_data,
const bluetooth::AdvertiseData& scan_response,
const bluetooth::AdvertiseSettings& settings) = 0;
- virtual void StopMultiAdvertising(int client_if) = 0;
+ virtual bool StopMultiAdvertising(int client_if) = 0;
// TODO(armansito): Complete the API definition.
@@ -107,16 +107,16 @@
virtual ~BpBluetoothLowEnergy() = default;
// IBluetoothLowEnergy overrides:
- void RegisterClient(
+ bool RegisterClient(
const android::sp<IBluetoothLowEnergyCallback>& callback) override;
void UnregisterClient(int client_if) override;
void UnregisterAll() override;
- void StartMultiAdvertising(
+ bool StartMultiAdvertising(
int client_if,
const bluetooth::AdvertiseData& advertise_data,
const bluetooth::AdvertiseData& scan_response,
const bluetooth::AdvertiseSettings& settings) override;
- void StopMultiAdvertising(int client_if) override;
+ bool StopMultiAdvertising(int client_if) override;
private:
DISALLOW_COPY_AND_ASSIGN(BpBluetoothLowEnergy);
diff --git a/service/ipc/binder/bluetooth_low_energy_binder_server.cpp b/service/ipc/binder/bluetooth_low_energy_binder_server.cpp
index 95f2712..6cc8d10 100644
--- a/service/ipc/binder/bluetooth_low_energy_binder_server.cpp
+++ b/service/ipc/binder/bluetooth_low_energy_binder_server.cpp
@@ -31,13 +31,13 @@
BluetoothLowEnergyBinderServer::~BluetoothLowEnergyBinderServer() {
}
-void BluetoothLowEnergyBinderServer::RegisterClient(
+bool BluetoothLowEnergyBinderServer::RegisterClient(
const android::sp<IBluetoothLowEnergyCallback>& callback) {
VLOG(2) << __func__;
bluetooth::LowEnergyClientFactory* ble_factory =
adapter_->GetLowEnergyClientFactory();
- RegisterClientBase(callback, ble_factory);
+ return RegisterClientBase(callback, ble_factory);
}
void BluetoothLowEnergyBinderServer::UnregisterClient(int client_if) {
@@ -50,31 +50,25 @@
UnregisterAllBase();
}
-void BluetoothLowEnergyBinderServer::StartMultiAdvertising(
+bool BluetoothLowEnergyBinderServer::StartMultiAdvertising(
int client_if,
const bluetooth::AdvertiseData& advertise_data,
const bluetooth::AdvertiseData& scan_response,
const bluetooth::AdvertiseSettings& settings) {
- VLOG(2) << __func__;
+ VLOG(2) << __func__ << " client_if: " << client_if;
std::lock_guard<std::mutex> lock(*maps_lock());
auto client = GetLEClient(client_if);
if (!client) {
LOG(ERROR) << "Unknown client_if: " << client_if;
- return;
- }
-
- auto cb = GetLECallback(client_if);
- if (!cb.get()) {
- LOG(ERROR) << "Client was unregistered - client_if: " << client_if;
- return;
+ return false;
}
// Create a weak pointer and pass that to the callback to prevent a potential
// use after free.
- bluetooth::AdvertiseSettings settings_copy = settings;
android::wp<BluetoothLowEnergyBinderServer> weak_ptr_to_this(this);
- auto callback = [&](bluetooth::BLEStatus status) {
+ auto settings_copy = settings;
+ auto callback = [=](bluetooth::BLEStatus status) {
auto sp_to_this = weak_ptr_to_this.promote();
if (!sp_to_this.get()) {
VLOG(2) << "BluetoothLowEnergyBinderServer was deleted";
@@ -83,60 +77,62 @@
std::lock_guard<std::mutex> lock(*maps_lock());
+ auto cb = GetLECallback(client_if);
+ if (!cb.get()) {
+ VLOG(1) << "Client was removed before callback: " << client_if;
+ return;
+ }
+
cb->OnMultiAdvertiseCallback(status, true /* is_start */, settings_copy);
};
- if (client->StartAdvertising(
- settings, advertise_data, scan_response, callback))
- return;
+ if (!client->StartAdvertising(
+ settings, advertise_data, scan_response, callback)) {
+ LOG(ERROR) << "Failed to initiate call to start advertising";
+ return false;
+ }
- LOG(ERROR) << "Failed to initiate call to start advertising";
-
- // Notify error in oneway callback.
- cb->OnMultiAdvertiseCallback(
- bluetooth::BLE_STATUS_FAILURE, true /* is_start */, settings_copy);
+ return true;
}
-void BluetoothLowEnergyBinderServer::StopMultiAdvertising(int client_if) {
+bool BluetoothLowEnergyBinderServer::StopMultiAdvertising(int client_if) {
VLOG(2) << __func__;
std::lock_guard<std::mutex> lock(*maps_lock());
auto client = GetLEClient(client_if);
if (!client) {
LOG(ERROR) << "Unknown client_if: " << client_if;
- return;
- }
-
- auto cb = GetLECallback(client_if);
- if (!cb.get()) {
- LOG(ERROR) << "Client was unregistered - client_if: " << client_if;
- return;
+ return false;
}
// Create a weak pointer and pass that to the callback to prevent a potential
// use after free.
android::wp<BluetoothLowEnergyBinderServer> weak_ptr_to_this(this);
- bluetooth::AdvertiseSettings settings_copy = client->settings();
- auto callback = [&](bluetooth::BLEStatus status) {
+ auto settings_copy = client->settings();
+ auto callback = [=](bluetooth::BLEStatus status) {
auto sp_to_this = weak_ptr_to_this.promote();
if (!sp_to_this.get()) {
VLOG(2) << "BluetoothLowEnergyBinderServer was deleted";
return;
}
+ auto cb = GetLECallback(client_if);
+ if (!cb.get()) {
+ VLOG(2) << "Client was unregistered - client_if: " << client_if;
+ return;
+ }
+
std::lock_guard<std::mutex> lock(*maps_lock());
cb->OnMultiAdvertiseCallback(status, false /* is_start */, settings_copy);
};
- if (client->StopAdvertising(callback))
- return;
+ if (!client->StopAdvertising(callback)) {
+ LOG(ERROR) << "Failed to initiate call to start advertising";
+ return false;
+ }
- LOG(ERROR) << "Failed to initiate call to start advertising";
-
- // Notify error in oneway callback.
- cb->OnMultiAdvertiseCallback(
- bluetooth::BLE_STATUS_FAILURE, false/* is_start */, settings_copy);
+ return true;
}
android::sp<IBluetoothLowEnergyCallback>
diff --git a/service/ipc/binder/bluetooth_low_energy_binder_server.h b/service/ipc/binder/bluetooth_low_energy_binder_server.h
index e491a9d..7cb7fb2 100644
--- a/service/ipc/binder/bluetooth_low_energy_binder_server.h
+++ b/service/ipc/binder/bluetooth_low_energy_binder_server.h
@@ -41,16 +41,16 @@
~BluetoothLowEnergyBinderServer() override;
// IBluetoothLowEnergy overrides:
- void RegisterClient(
+ bool RegisterClient(
const android::sp<IBluetoothLowEnergyCallback>& callback) override;
void UnregisterClient(int client_if) override;
void UnregisterAll() override;
- void StartMultiAdvertising(
+ bool StartMultiAdvertising(
int client_if,
const bluetooth::AdvertiseData& advertise_data,
const bluetooth::AdvertiseData& scan_response,
const bluetooth::AdvertiseSettings& settings) override;
- void StopMultiAdvertising(int client_if) override;
+ bool StopMultiAdvertising(int client_if) override;
private:
// Returns a pointer to the IBluetoothLowEnergyCallback instance associated
diff --git a/service/main.cpp b/service/main.cpp
index 93eb25f..90b7c9a 100644
--- a/service/main.cpp
+++ b/service/main.cpp
@@ -13,6 +13,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//
+
#include <base/at_exit.h>
#include <base/command_line.h>
#include <base/files/scoped_file.h>