From b1ad3a7bf1973c04989fa91a728fced27cd12f62 Mon Sep 17 00:00:00 2001 From: Ningyuan Wang Date: Tue, 18 Apr 2017 14:20:41 -0700 Subject: [PATCH 1/2] Return ERROR_NOT_SUPPORTED when configureChip() is used for reconfiguration Bug: 36562856 Bug: 37446050 Test: compile, VTS test fail as expected Change-Id: I895dd0d6e96b0d0a2b429c3a68be1f7c7e32a7e3 --- current.txt | 2 +- wifi/1.0/IWifiChip.hal | 8 +++++++- wifi/1.0/default/wifi_chip.cpp | 12 +++++------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/current.txt b/current.txt index 575e8faccb..750ea6f62c 100644 --- a/current.txt +++ b/current.txt @@ -161,7 +161,7 @@ f7e6e747910a3cd0a35846141e3b990a6a612a297b2b70ccd5740b646a450a8c android.hardwar 4b962968a7df4ab104d1315d66a0a7348a713fecbb5d2c1b23688494458f37ce android.hardware.vr@1.0::IVr b9be36719a8ad534000a51ea07be91be94c405bf1e038ae825acf65087ffd378 android.hardware.wifi@1.0::IWifi ee0224ee18813506d9d6f13d8c8e4679f053c290a443a52a7c52a5d3c852262b android.hardware.wifi@1.0::IWifiApIface -50390d78cb83c9912235e17f25255a19429b5e7094c8980bea28623873ce5e3d android.hardware.wifi@1.0::IWifiChip +f3eecc489deb4c74892f59eb7adb769063bd5c354ac132b626a5f42b363d36bc android.hardware.wifi@1.0::IWifiChip a1b988377645a58e5e2542ca2bad4e17c21a4a389213d05de2f0e32d57b7d339 android.hardware.wifi@1.0::IWifiChipEventCallback 5ed6760ce77e84bc6c49d1acb3f7d8117c9176b3f06514bc44ad3af84c80dcfe android.hardware.wifi@1.0::IWifiEventCallback 6b9ad43a5efbe6ca214f751e22ce43cf5cd4d5d5f2cba80f24ccd3755a72401c android.hardware.wifi@1.0::IWifiIface diff --git a/wifi/1.0/IWifiChip.hal b/wifi/1.0/IWifiChip.hal index 611c44978f..b97327c84c 100644 --- a/wifi/1.0/IWifiChip.hal +++ b/wifi/1.0/IWifiChip.hal @@ -220,7 +220,12 @@ interface IWifiChip { getAvailableModes() generates (WifiStatus status, vec modes); /** - * Reconfigure the Chip. + * Configure the Chip. + * This may NOT be called to reconfigure a chip due to an internal + * limitation. Calling this when chip is already configured in a different + * mode must trigger an ERROR_NOT_SUPPORTED failure. + * If you want to do reconfiguration, please call IWifi.stop() and IWifi.start() + * to restart Wifi HAL before calling this. * Any existing |IWifiIface| objects must be marked invalid after this call. * If this fails then the chips is now in an undefined state and * configureChip must be called again. @@ -234,6 +239,7 @@ interface IWifiChip { * |WifiStatusCode.SUCCESS|, * |WifiStatusCode.ERROR_WIFI_CHIP_INVALID|, * |WifiStatusCode.ERROR_NOT_AVAILABLE|, + * |WifiStatusCode.ERROR_NOT_SUPPORTED|, * |WifiStatusCode.ERROR_UNKNOWN| */ configureChip(ChipModeId modeId) generates (WifiStatus status); diff --git a/wifi/1.0/default/wifi_chip.cpp b/wifi/1.0/default/wifi_chip.cpp index 319e12651d..770c83f181 100644 --- a/wifi/1.0/default/wifi_chip.cpp +++ b/wifi/1.0/default/wifi_chip.cpp @@ -804,14 +804,12 @@ WifiStatus WifiChip::enableDebugErrorAlertsInternal(bool enable) { WifiStatus WifiChip::handleChipConfiguration(ChipModeId mode_id) { // If the chip is already configured in a different mode, stop // the legacy HAL and then start it after firmware mode change. + // Currently the underlying implementation has a deadlock issue. + // We should return ERROR_NOT_SUPPORTED if chip is already configured in + // a different mode. if (current_mode_id_ != kInvalidModeId) { - invalidateAndRemoveAllIfaces(); - legacy_hal::wifi_error legacy_status = legacy_hal_.lock()->stop([]() {}); - if (legacy_status != legacy_hal::WIFI_SUCCESS) { - LOG(ERROR) << "Failed to stop legacy HAL: " - << legacyErrorToString(legacy_status); - return createWifiStatusFromLegacyError(legacy_status); - } + // TODO(b/37446050): Fix the deadlock. + return createWifiStatus(WifiStatusCode::ERROR_NOT_SUPPORTED); } bool success; if (mode_id == kStaChipModeId) { From f8bb8d7fc779dcf16eb203642900d222bf9f1f7e Mon Sep 17 00:00:00 2001 From: Ningyuan Wang Date: Fri, 21 Apr 2017 09:54:07 -0700 Subject: [PATCH 2/2] Fix configureChip() VTS test Bug: 36562856 Bug: 37446050 Test: compile, vts test pass Change-Id: I99ee0336d9cc11329de560daf26216ac15e5a0a9 --- wifi/1.0/vts/functional/wifi_chip_hidl_test.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/wifi/1.0/vts/functional/wifi_chip_hidl_test.cpp b/wifi/1.0/vts/functional/wifi_chip_hidl_test.cpp index 0627a99a7e..6c2372f80e 100644 --- a/wifi/1.0/vts/functional/wifi_chip_hidl_test.cpp +++ b/wifi/1.0/vts/functional/wifi_chip_hidl_test.cpp @@ -173,8 +173,12 @@ TEST_F(WifiChipHidlTest, ConfigureChip) { EXPECT_EQ(WifiStatusCode::SUCCESS, status_and_modes.first.code); EXPECT_LT(0u, status_and_modes.second.size()); for (const auto& mode : status_and_modes.second) { + // configureChip() requires to be called with a fresh IWifiChip object. + wifi_chip_ = getWifiChip(); + ASSERT_NE(nullptr, wifi_chip_.get()); EXPECT_EQ(WifiStatusCode::SUCCESS, HIDL_INVOKE(wifi_chip_, configureChip, mode.id).code); + stopWifi(); } }