From 503582ed945f9d4da8696709566fb556df088099 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 11 Oct 2016 08:25:30 -0700 Subject: [PATCH] wifi: Make methods deliver status synchronously (1/3) Make the following |IWifi| HIDL interface methods return a synchronous status code: a) start() b) stop() The other methods in this interface do not have a failure case and hence not returning a status code. This changes the nature of event callbacks registered for each interface. Previously, every operation's status was sent to all the registered event callbacks. Now, only the caller is notified of the operation's status via the passed synchronous callbacks. The event callbacks are now used to broadcast only important state changes/events. Bug: 32056230 Bug: 32061909 Test: Compiles Change-Id: I95dc3fa139ac3ac7500d81e9e0dbc4f4de04e127 --- wifi/1.0/default/wifi.cpp | 73 ++++++++++++++++++++------------------- wifi/1.0/default/wifi.h | 12 +++---- 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/wifi/1.0/default/wifi.cpp b/wifi/1.0/default/wifi.cpp index ff2eb4cb2e..e10826bfbb 100644 --- a/wifi/1.0/default/wifi.cpp +++ b/wifi/1.0/default/wifi.cpp @@ -18,8 +18,8 @@ #include -#include "failure_reason_util.h" #include "wifi_chip.h" +#include "wifi_status_util.h" namespace { // Chip ID to use for the only supported chip. @@ -36,9 +36,9 @@ Wifi::Wifi() : legacy_hal_(new WifiLegacyHal()), run_state_(RunState::STOPPED) {} Return Wifi::registerEventCallback( - const sp& callback) { + const sp& event_callback) { // TODO(b/31632518): remove the callback when the client is destroyed - callbacks_.emplace_back(callback); + event_callbacks_.emplace_back(event_callback); return Void(); } @@ -46,47 +46,44 @@ Return Wifi::isStarted() { return run_state_ != RunState::STOPPED; } -Return Wifi::start() { +Return Wifi::start(start_cb hidl_status_cb) { if (run_state_ == RunState::STARTED) { - for (const auto& callback : callbacks_) { - callback->onStart(); - } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS)); return Void(); } else if (run_state_ == RunState::STOPPING) { - for (const auto& callback : callbacks_) { - callback->onStartFailure(CreateFailureReason( - CommandFailureReason::NOT_AVAILABLE, "HAL is stopping")); - } + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE, + "HAL is stopping")); return Void(); } LOG(INFO) << "Starting HAL"; - wifi_error status = legacy_hal_->start(); - if (status != WIFI_SUCCESS) { + wifi_error legacy_status = legacy_hal_->start(); + if (legacy_status != WIFI_SUCCESS) { LOG(ERROR) << "Failed to start Wifi HAL"; - for (auto& callback : callbacks_) { - callback->onStartFailure( - CreateFailureReasonLegacyError(status, "Failed to start HAL")); - } + hidl_status_cb( + createWifiStatusFromLegacyError(legacy_status, "Failed to start HAL")); return Void(); } // Create the chip instance once the HAL is started. chip_ = new WifiChip(kChipId, legacy_hal_); run_state_ = RunState::STARTED; - for (const auto& callback : callbacks_) { - callback->onStart(); + for (const auto& callback : event_callbacks_) { + if (!callback->onStart().getStatus().isOk()) { + LOG(ERROR) << "Failed to invoke onStart callback"; + }; } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS)); return Void(); } -Return Wifi::stop() { +Return Wifi::stop(stop_cb hidl_status_cb) { if (run_state_ == RunState::STOPPED) { - for (const auto& callback : callbacks_) { - callback->onStop(); - } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS)); return Void(); } else if (run_state_ == RunState::STOPPING) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_NOT_AVAILABLE, + "HAL is stopping")); return Void(); } @@ -98,37 +95,43 @@ Return Wifi::stop() { } chip_.clear(); run_state_ = RunState::STOPPED; - for (const auto& callback : callbacks_) { - callback->onStop(); + for (const auto& callback : event_callbacks_) { + if (!callback->onStop().getStatus().isOk()) { + LOG(ERROR) << "Failed to invoke onStop callback"; + }; } }; - wifi_error status = legacy_hal_->stop(on_complete_callback_); - if (status != WIFI_SUCCESS) { + wifi_error legacy_status = legacy_hal_->stop(on_complete_callback_); + if (legacy_status != WIFI_SUCCESS) { LOG(ERROR) << "Failed to stop Wifi HAL"; - for (const auto& callback : callbacks_) { - callback->onFailure( - CreateFailureReasonLegacyError(status, "Failed to stop HAL")); + WifiStatus wifi_status = + createWifiStatusFromLegacyError(legacy_status, "Failed to stop HAL"); + for (const auto& callback : event_callbacks_) { + callback->onFailure(wifi_status); } + hidl_status_cb(wifi_status); + return Void(); } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS)); return Void(); } -Return Wifi::getChipIds(getChipIds_cb cb) { +Return Wifi::getChipIds(getChipIds_cb hidl_status_cb) { std::vector chip_ids; if (chip_.get()) { chip_ids.emplace_back(kChipId); } hidl_vec hidl_data; hidl_data.setToExternal(chip_ids.data(), chip_ids.size()); - cb(hidl_data); + hidl_status_cb(hidl_data); return Void(); } -Return Wifi::getChip(ChipId chip_id, getChip_cb cb) { +Return Wifi::getChip(ChipId chip_id, getChip_cb hidl_status_cb) { if (chip_.get() && chip_id == kChipId) { - cb(chip_); + hidl_status_cb(chip_); } else { - cb(nullptr); + hidl_status_cb(nullptr); } return Void(); } diff --git a/wifi/1.0/default/wifi.h b/wifi/1.0/default/wifi.h index 55ba12b36b..989b63019b 100644 --- a/wifi/1.0/default/wifi.h +++ b/wifi/1.0/default/wifi.h @@ -41,12 +41,12 @@ class Wifi : public IWifi { // HIDL methods exposed. Return registerEventCallback( - const sp& callback) override; + const sp& event_callback) override; Return isStarted() override; - Return start() override; - Return stop() override; - Return getChipIds(getChipIds_cb cb) override; - Return getChip(ChipId chip_id, getChip_cb cb) override; + Return start(start_cb hidl_status_cb) override; + Return stop(stop_cb hidl_status_cb) override; + Return getChipIds(getChipIds_cb hidl_status_cb) override; + Return getChip(ChipId chip_id, getChip_cb hidl_status_cb) override; private: enum class RunState { STOPPED, STARTED, STOPPING }; @@ -55,7 +55,7 @@ class Wifi : public IWifi { // and shared with all the child HIDL interface objects. std::shared_ptr legacy_hal_; RunState run_state_; - std::vector> callbacks_; + std::vector> event_callbacks_; sp chip_; DISALLOW_COPY_AND_ASSIGN(Wifi);