From 11f930321ba8c3501ed4e24a5519f13d34f69b31 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Fri, 9 Dec 2016 10:26:17 -0800 Subject: [PATCH] wifi: Fixes in WifiLegacyHal Changes: 1. Add |is_started_| flag in WifiLegacyHal to ignore stop/start requests when they're already in the same state. 2. Detach the event loop thread and remove the class member storing the thread handle. While there, 1. Cleanup some logs to better help debug issues. 2. Also fixed the wrong method name used in WifiModeController. Bug: 33480898 Test: Gtests run successfully. Change-Id: I913f656206a2ca7a79fb316501c256fcdc32aed9 --- wifi/1.0/default/wifi.cpp | 2 -- wifi/1.0/default/wifi_legacy_hal.cpp | 29 ++++++++++++++++------- wifi/1.0/default/wifi_legacy_hal.h | 4 ++-- wifi/1.0/default/wifi_mode_controller.cpp | 3 +-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/wifi/1.0/default/wifi.cpp b/wifi/1.0/default/wifi.cpp index 30adcc04b0..332363de6a 100644 --- a/wifi/1.0/default/wifi.cpp +++ b/wifi/1.0/default/wifi.cpp @@ -165,7 +165,6 @@ std::pair> Wifi::getChipInternal(ChipId chip_id) { } WifiStatus Wifi::initializeLegacyHal() { - LOG(INFO) << "Initializing legacy HAL"; legacy_hal::wifi_error legacy_status = legacy_hal_->initialize(); if (legacy_status != legacy_hal::WIFI_SUCCESS) { LOG(ERROR) << "Failed to initialize legacy HAL: " @@ -176,7 +175,6 @@ WifiStatus Wifi::initializeLegacyHal() { } WifiStatus Wifi::stopLegacyHalAndDeinitializeModeController() { - LOG(INFO) << "Stopping legacy HAL"; run_state_ = RunState::STOPPING; const auto on_complete_callback_ = [&]() { if (chip_.get()) { diff --git a/wifi/1.0/default/wifi_legacy_hal.cpp b/wifi/1.0/default/wifi_legacy_hal.cpp index bf61d835d7..92343824e9 100644 --- a/wifi/1.0/default/wifi_legacy_hal.cpp +++ b/wifi/1.0/default/wifi_legacy_hal.cpp @@ -249,9 +249,11 @@ void onNanEventTransmitFollowUp(NanTransmitFollowupInd* event) { WifiLegacyHal::WifiLegacyHal() : global_handle_(nullptr), wlan_interface_handle_(nullptr), - awaiting_event_loop_termination_(false) {} + awaiting_event_loop_termination_(false), + is_started_(false) {} wifi_error WifiLegacyHal::initialize() { + LOG(DEBUG) << "Initialize legacy HAL"; // TODO: Add back the HAL Tool if we need to. All we need from the HAL tool // for now is this function call which we can directly call. wifi_error status = init_wifi_vendor_hal_func_table(&global_func_table_); @@ -266,29 +268,39 @@ wifi_error WifiLegacyHal::start() { // Ensure that we're starting in a good state. CHECK(global_func_table_.wifi_initialize && !global_handle_ && !wlan_interface_handle_ && !awaiting_event_loop_termination_); + if (is_started_) { + LOG(DEBUG) << "Legacy HAL already started"; + return WIFI_SUCCESS; + } + LOG(DEBUG) << "Starting legacy HAL"; if (!iface_tool_.SetWifiUpState(true)) { LOG(ERROR) << "Failed to set WiFi interface up"; return WIFI_ERROR_UNKNOWN; } - LOG(INFO) << "Starting legacy HAL"; wifi_error status = global_func_table_.wifi_initialize(&global_handle_); if (status != WIFI_SUCCESS || !global_handle_) { LOG(ERROR) << "Failed to retrieve global handle"; return status; } - event_loop_thread_ = std::thread(&WifiLegacyHal::runEventLoop, this); + std::thread(&WifiLegacyHal::runEventLoop, this).detach(); status = retrieveWlanInterfaceHandle(); if (status != WIFI_SUCCESS || !wlan_interface_handle_) { LOG(ERROR) << "Failed to retrieve wlan interface handle"; return status; } - LOG(VERBOSE) << "Legacy HAL start complete"; + LOG(DEBUG) << "Legacy HAL start complete"; + is_started_ = true; return WIFI_SUCCESS; } wifi_error WifiLegacyHal::stop( const std::function& on_stop_complete_user_callback) { - LOG(INFO) << "Stopping legacy HAL"; + if (!is_started_) { + LOG(DEBUG) << "Legacy HAL already stopped"; + on_stop_complete_user_callback(); + return WIFI_SUCCESS; + } + LOG(DEBUG) << "Stopping legacy HAL"; on_stop_complete_internal_callback = [&](wifi_handle handle) { CHECK_EQ(global_handle_, handle) << "Handle mismatch"; // Invalidate all the internal pointers now that the HAL is @@ -299,7 +311,8 @@ wifi_error WifiLegacyHal::stop( }; awaiting_event_loop_termination_ = true; global_func_table_.wifi_cleanup(global_handle_, onStopComplete); - LOG(VERBOSE) << "Legacy HAL stop initiated"; + LOG(DEBUG) << "Legacy HAL stop complete"; + is_started_ = false; return WIFI_SUCCESS; } @@ -1030,12 +1043,12 @@ wifi_error WifiLegacyHal::retrieveWlanInterfaceHandle() { } void WifiLegacyHal::runEventLoop() { - LOG(VERBOSE) << "Starting legacy HAL event loop"; + LOG(DEBUG) << "Starting legacy HAL event loop"; global_func_table_.wifi_event_loop(global_handle_); if (!awaiting_event_loop_termination_) { LOG(FATAL) << "Legacy HAL event loop terminated, but HAL was not stopping"; } - LOG(VERBOSE) << "Legacy HAL event loop terminated"; + LOG(DEBUG) << "Legacy HAL event loop terminated"; awaiting_event_loop_termination_ = false; } diff --git a/wifi/1.0/default/wifi_legacy_hal.h b/wifi/1.0/default/wifi_legacy_hal.h index b68fa507b8..27ffa719cf 100644 --- a/wifi/1.0/default/wifi_legacy_hal.h +++ b/wifi/1.0/default/wifi_legacy_hal.h @@ -250,8 +250,6 @@ class WifiLegacyHal { getGscanCachedResults(); void invalidate(); - // Event loop thread used by legacy HAL. - std::thread event_loop_thread_; // Global function table of legacy HAL. wifi_hal_fn global_func_table_; // Opaque handle to be used for all global operations. @@ -260,6 +258,8 @@ class WifiLegacyHal { wifi_interface_handle wlan_interface_handle_; // Flag to indicate if we have initiated the cleanup of legacy HAL. bool awaiting_event_loop_termination_; + // Flag to indicate if the legacy HAL has been started. + bool is_started_; wifi_system::InterfaceTool iface_tool_; }; diff --git a/wifi/1.0/default/wifi_mode_controller.cpp b/wifi/1.0/default/wifi_mode_controller.cpp index 42dd9ad174..7e82d4caf9 100644 --- a/wifi/1.0/default/wifi_mode_controller.cpp +++ b/wifi/1.0/default/wifi_mode_controller.cpp @@ -64,8 +64,7 @@ bool WifiModeController::changeFirmwareMode(IfaceType type) { LOG(ERROR) << "Failed to load WiFi driver"; return false; } - if (!driver_tool_->IsFirmwareModeChangeNeeded( - convertIfaceTypeToFirmwareMode(type))) { + if (!driver_tool_->ChangeFirmwareMode(convertIfaceTypeToFirmwareMode(type))) { LOG(ERROR) << "Failed to change firmware mode"; return false; }