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

4 files changed, 24 insertions, 14 deletions

diff --git a/wifi/1.0/default/wifi.cpp b/wifi/1.0/default/wifi.cpp
index 30adcc04b..332363de6 100644
--- a/wifi/1.0/default/wifi.cpp
+++ b/wifi/1.0/default/wifi.cpp
@@ -165,7 +165,6 @@ std::pair<WifiStatus, sp<IWifiChip>> 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 bf61d835d..92343824e 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<void()>& 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 b68fa507b..27ffa719c 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 42dd9ad17..7e82d4caf 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;
   }