From 5c05546fc9fdcb1dfa92b2c80474f88bba5a77a4 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 11 Oct 2016 08:27:27 -0700 Subject: [PATCH] wifi: Make methods deliver status synchronously (2/3) Make all the |IWifiChip| HIDL interface methods return a synchronous status code. Change from using the event callbacks to the synchronous callbacks for delivering status. While there, 1. Use std::tie to retrive values out of the legacy HAL functions returning a pair. 2. Use the std::vector to hidl_vec constructor for returning vector of ifnames. Bug: 32056230 Bug: 32061909 Test: Compiles Change-Id: Iac27521be17cd9852df04ad7d412e09160a08d33 --- wifi/1.0/default/wifi_chip.cpp | 323 +++++++++++++++++++-------------- wifi/1.0/default/wifi_chip.h | 53 +++--- 2 files changed, 212 insertions(+), 164 deletions(-) diff --git a/wifi/1.0/default/wifi_chip.cpp b/wifi/1.0/default/wifi_chip.cpp index 4dd020b592..4f8545ff48 100644 --- a/wifi/1.0/default/wifi_chip.cpp +++ b/wifi/1.0/default/wifi_chip.cpp @@ -18,23 +18,19 @@ #include -#include "failure_reason_util.h" +#include "wifi_status_util.h" namespace { using android::sp; using android::hardware::hidl_vec; using android::hardware::hidl_string; -hidl_vec createHidlVecOfIfaceNames(const std::string& ifname) { +std::vector createHidlVecOfIfaceNames(const std::string& ifname) { std::vector ifnames; if (!ifname.empty()) { - hidl_string hidl_ifname; - hidl_ifname = ifname.c_str(); - ifnames.emplace_back(hidl_ifname); + ifnames.emplace_back(ifname); } - hidl_vec hidl_ifnames; - hidl_ifnames.setToExternal(ifnames.data(), ifnames.size()); - return hidl_ifnames; + return ifnames; } template @@ -59,156 +55,181 @@ WifiChip::WifiChip(ChipId chip_id, void WifiChip::invalidate() { invalidateAndRemoveAllIfaces(); legacy_hal_.reset(); - callbacks_.clear(); + event_callbacks_.clear(); is_valid_ = false; } -Return WifiChip::getId() { - return chip_id_; +Return WifiChip::getId(getId_cb hidl_status_cb) { + if (!is_valid_) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + UINT32_MAX); + return Void(); + } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), chip_id_); + return Void(); } Return WifiChip::registerEventCallback( - const sp& callback) { - if (!is_valid_) + const sp& event_callback, + registerEventCallback_cb hidl_status_cb) { + if (!is_valid_) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID)); return Void(); + } // TODO(b/31632518): remove the callback when the client is destroyed - callbacks_.emplace_back(callback); + event_callbacks_.emplace_back(event_callback); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS)); return Void(); } -Return WifiChip::getAvailableModes(getAvailableModes_cb cb) { +Return WifiChip::getAvailableModes(getAvailableModes_cb hidl_status_cb) { if (!is_valid_) { - cb(hidl_vec()); - return Void(); - } else { - // TODO add implementation + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + hidl_vec()); return Void(); } + // TODO add implementation + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), + hidl_vec()); + return Void(); } -Return WifiChip::configureChip(uint32_t /*mode_id*/) { - if (!is_valid_) +Return WifiChip::configureChip(uint32_t /*mode_id*/, + configureChip_cb hidl_status_cb) { + if (!is_valid_) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID)); return Void(); + } invalidateAndRemoveAllIfaces(); // TODO add implementation + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS)); return Void(); } -Return WifiChip::getMode() { - if (!is_valid_) - return 0; +Return WifiChip::getMode(getMode_cb hidl_status_cb) { + if (!is_valid_) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + UINT32_MAX); + return Void(); + } // TODO add implementation - return 0; -} - -Return WifiChip::requestChipDebugInfo() { - if (!is_valid_) - return Void(); - - IWifiChipEventCallback::ChipDebugInfo result; - - std::pair ret = - legacy_hal_.lock()->getDriverVersion(); - if (ret.first != WIFI_SUCCESS) { - LOG(ERROR) << "Failed to get driver version: " - << LegacyErrorToString(ret.first); - FailureReason reason = CreateFailureReasonLegacyError( - ret.first, " failed to get driver version"); - for (const auto& callback : callbacks_) { - callback->onChipDebugInfoFailure(reason); - } - return Void(); - } - result.driverDescription = ret.second.c_str(); - - ret = legacy_hal_.lock()->getFirmwareVersion(); - if (ret.first != WIFI_SUCCESS) { - LOG(ERROR) << "Failed to get firmware version: " - << LegacyErrorToString(ret.first); - FailureReason reason = CreateFailureReasonLegacyError( - ret.first, " failed to get firmware version"); - for (const auto& callback : callbacks_) { - callback->onChipDebugInfoFailure(reason); - } - return Void(); - } - result.firmwareDescription = ret.second.c_str(); - - for (const auto& callback : callbacks_) { - callback->onChipDebugInfoAvailable(result); - } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), 0); return Void(); } -Return WifiChip::requestDriverDebugDump() { - if (!is_valid_) - return Void(); - - std::pair> ret = - legacy_hal_.lock()->requestDriverMemoryDump(); - if (ret.first != WIFI_SUCCESS) { - LOG(ERROR) << "Failed to get driver debug dump: " - << LegacyErrorToString(ret.first); - FailureReason reason = CreateFailureReasonLegacyError(ret.first, ""); - for (const auto& callback : callbacks_) { - callback->onDriverDebugDumpFailure(reason); - } +Return WifiChip::requestChipDebugInfo( + requestChipDebugInfo_cb hidl_status_cb) { + if (!is_valid_) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + IWifiChip::ChipDebugInfo()); + return Void(); + } + + IWifiChip::ChipDebugInfo result; + + wifi_error legacy_status; + std::string driver_desc; + std::tie(legacy_status, driver_desc) = legacy_hal_.lock()->getDriverVersion(); + if (legacy_status != WIFI_SUCCESS) { + LOG(ERROR) << "Failed to get driver version: " + << legacyErrorToString(legacy_status); + WifiStatus status = createWifiStatusFromLegacyError( + legacy_status, "failed to get driver version"); + hidl_status_cb(status, result); + return Void(); + } + result.driverDescription = driver_desc.c_str(); + + std::string firmware_desc; + std::tie(legacy_status, firmware_desc) = + legacy_hal_.lock()->getFirmwareVersion(); + if (legacy_status != WIFI_SUCCESS) { + LOG(ERROR) << "Failed to get firmware version: " + << legacyErrorToString(legacy_status); + WifiStatus status = createWifiStatusFromLegacyError( + legacy_status, "failed to get firmware version"); + hidl_status_cb(status, result); + return Void(); + } + result.firmwareDescription = firmware_desc.c_str(); + + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), result); + return Void(); +} + +Return WifiChip::requestDriverDebugDump( + requestDriverDebugDump_cb hidl_status_cb) { + if (!is_valid_) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + hidl_vec()); + return Void(); + } + + wifi_error legacy_status; + std::vector driver_dump; + std::tie(legacy_status, driver_dump) = + legacy_hal_.lock()->requestDriverMemoryDump(); + if (legacy_status != WIFI_SUCCESS) { + LOG(ERROR) << "Failed to get driver debug dump: " + << legacyErrorToString(legacy_status); + hidl_status_cb(createWifiStatusFromLegacyError(legacy_status), + hidl_vec()); return Void(); } - auto& driver_dump = ret.second; hidl_vec hidl_data; hidl_data.setToExternal(reinterpret_cast(driver_dump.data()), driver_dump.size()); - for (const auto& callback : callbacks_) { - callback->onDriverDebugDumpAvailable(hidl_data); - } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), hidl_data); return Void(); } -Return WifiChip::requestFirmwareDebugDump() { - if (!is_valid_) - return Void(); - - std::pair> ret = - legacy_hal_.lock()->requestFirmwareMemoryDump(); - if (ret.first != WIFI_SUCCESS) { - LOG(ERROR) << "Failed to get firmware debug dump: " - << LegacyErrorToString(ret.first); - FailureReason reason = CreateFailureReasonLegacyError(ret.first, ""); - for (const auto& callback : callbacks_) { - callback->onFirmwareDebugDumpFailure(reason); - } +Return WifiChip::requestFirmwareDebugDump( + requestFirmwareDebugDump_cb hidl_status_cb) { + if (!is_valid_) { + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + hidl_vec()); + return Void(); + } + + wifi_error legacy_status; + std::vector firmware_dump; + std::tie(legacy_status, firmware_dump) = + legacy_hal_.lock()->requestFirmwareMemoryDump(); + if (legacy_status != WIFI_SUCCESS) { + LOG(ERROR) << "Failed to get firmware debug dump: " + << legacyErrorToString(legacy_status); + hidl_status_cb(createWifiStatusFromLegacyError(legacy_status), + hidl_vec()); return Void(); } - auto& firmware_dump = ret.second; hidl_vec hidl_data; hidl_data.setToExternal(reinterpret_cast(firmware_dump.data()), firmware_dump.size()); - for (const auto& callback : callbacks_) { - callback->onFirmwareDebugDumpAvailable(hidl_data); - } + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), hidl_data); return Void(); } -Return WifiChip::createApIface(createApIface_cb cb) { +Return WifiChip::createApIface(createApIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } // TODO(b/31997422): Disallow this based on the chip combination. std::string ifname = legacy_hal_.lock()->getApIfaceName(); ap_iface_ = new WifiApIface(ifname, legacy_hal_); - cb(ap_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), ap_iface_); return Void(); } -Return WifiChip::getApIfaceNames(getApIfaceNames_cb cb) { +Return WifiChip::getApIfaceNames(getApIfaceNames_cb hidl_status_cb) { if (!is_valid_) { - cb(hidl_vec()); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + hidl_vec()); return Void(); } @@ -216,41 +237,47 @@ Return WifiChip::getApIfaceNames(getApIfaceNames_cb cb) { if (ap_iface_.get()) { ifname = legacy_hal_.lock()->getApIfaceName().c_str(); } - cb(createHidlVecOfIfaceNames(ifname)); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), + createHidlVecOfIfaceNames(ifname)); return Void(); } -Return WifiChip::getApIface(const hidl_string& ifname, getApIface_cb cb) { +Return WifiChip::getApIface(const hidl_string& ifname, + getApIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } if (ap_iface_.get() && (ifname.c_str() == legacy_hal_.lock()->getApIfaceName())) { - cb(ap_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), ap_iface_); } else { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS), + nullptr); } return Void(); } -Return WifiChip::createNanIface(createNanIface_cb cb) { +Return WifiChip::createNanIface(createNanIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } // TODO(b/31997422): Disallow this based on the chip combination. std::string ifname = legacy_hal_.lock()->getNanIfaceName(); nan_iface_ = new WifiNanIface(ifname, legacy_hal_); - cb(nan_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), nan_iface_); return Void(); } -Return WifiChip::getNanIfaceNames(getNanIfaceNames_cb cb) { +Return WifiChip::getNanIfaceNames(getNanIfaceNames_cb hidl_status_cb) { if (!is_valid_) { - cb(hidl_vec()); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + hidl_vec()); return Void(); } @@ -258,42 +285,47 @@ Return WifiChip::getNanIfaceNames(getNanIfaceNames_cb cb) { if (nan_iface_.get()) { ifname = legacy_hal_.lock()->getNanIfaceName().c_str(); } - cb(createHidlVecOfIfaceNames(ifname)); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), + createHidlVecOfIfaceNames(ifname)); return Void(); } Return WifiChip::getNanIface(const hidl_string& ifname, - getNanIface_cb cb) { + getNanIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } if (nan_iface_.get() && (ifname.c_str() == legacy_hal_.lock()->getNanIfaceName())) { - cb(nan_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), nan_iface_); } else { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS), + nullptr); } return Void(); } -Return WifiChip::createP2pIface(createP2pIface_cb cb) { +Return WifiChip::createP2pIface(createP2pIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } // TODO(b/31997422): Disallow this based on the chip combination. std::string ifname = legacy_hal_.lock()->getP2pIfaceName(); p2p_iface_ = new WifiP2pIface(ifname, legacy_hal_); - cb(p2p_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), p2p_iface_); return Void(); } -Return WifiChip::getP2pIfaceNames(getP2pIfaceNames_cb cb) { +Return WifiChip::getP2pIfaceNames(getP2pIfaceNames_cb hidl_status_cb) { if (!is_valid_) { - cb(hidl_vec()); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + hidl_vec()); return Void(); } @@ -301,42 +333,47 @@ Return WifiChip::getP2pIfaceNames(getP2pIfaceNames_cb cb) { if (p2p_iface_.get()) { ifname = legacy_hal_.lock()->getP2pIfaceName().c_str(); } - cb(createHidlVecOfIfaceNames(ifname)); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), + createHidlVecOfIfaceNames(ifname)); return Void(); } Return WifiChip::getP2pIface(const hidl_string& ifname, - getP2pIface_cb cb) { + getP2pIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } if (p2p_iface_.get() && (ifname.c_str() == legacy_hal_.lock()->getP2pIfaceName())) { - cb(p2p_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), p2p_iface_); } else { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS), + nullptr); } return Void(); } -Return WifiChip::createStaIface(createStaIface_cb cb) { +Return WifiChip::createStaIface(createStaIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } // TODO(b/31997422): Disallow this based on the chip combination. std::string ifname = legacy_hal_.lock()->getStaIfaceName(); sta_iface_ = new WifiStaIface(ifname, legacy_hal_); - cb(sta_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), sta_iface_); return Void(); } -Return WifiChip::getStaIfaceNames(getStaIfaceNames_cb cb) { +Return WifiChip::getStaIfaceNames(getStaIfaceNames_cb hidl_status_cb) { if (!is_valid_) { - cb(hidl_vec()); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + hidl_vec()); return Void(); } @@ -344,36 +381,40 @@ Return WifiChip::getStaIfaceNames(getStaIfaceNames_cb cb) { if (sta_iface_.get()) { ifname = legacy_hal_.lock()->getStaIfaceName().c_str(); } - cb(createHidlVecOfIfaceNames(ifname)); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), + createHidlVecOfIfaceNames(ifname)); return Void(); } Return WifiChip::getStaIface(const hidl_string& ifname, - getStaIface_cb cb) { + getStaIface_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } if (sta_iface_.get() && (ifname.c_str() == legacy_hal_.lock()->getStaIfaceName())) { - cb(sta_iface_); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), sta_iface_); } else { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_INVALID_ARGS), + nullptr); } return Void(); } -Return WifiChip::createRttController(const sp& bound_iface, - createRttController_cb cb) { +Return WifiChip::createRttController( + const sp& bound_iface, createRttController_cb hidl_status_cb) { if (!is_valid_) { - cb(nullptr); + hidl_status_cb(createWifiStatus(WifiStatusCode::ERROR_WIFI_CHIP_INVALID), + nullptr); return Void(); } sp rtt = new WifiRttController(bound_iface, legacy_hal_); rtt_controllers_.emplace_back(rtt); - cb(rtt); + hidl_status_cb(createWifiStatus(WifiStatusCode::SUCCESS), rtt); return Void(); } diff --git a/wifi/1.0/default/wifi_chip.h b/wifi/1.0/default/wifi_chip.h index 94ffa63ec2..dd9117e63e 100644 --- a/wifi/1.0/default/wifi_chip.h +++ b/wifi/1.0/default/wifi_chip.h @@ -59,39 +59,46 @@ class WifiChip : public IWifiChip { void invalidate(); // HIDL methods exposed. - Return getId() override; + Return getId(getId_cb hidl_status_cb) override; Return registerEventCallback( - const sp& callback) override; - Return getAvailableModes(getAvailableModes_cb cb) override; - Return configureChip(uint32_t mode_id) override; - Return getMode() override; - Return requestChipDebugInfo() override; - Return requestDriverDebugDump() override; - Return requestFirmwareDebugDump() override; - Return createApIface(createApIface_cb cb) override; - Return getApIfaceNames(getApIfaceNames_cb cb) override; - Return getApIface(const hidl_string& ifname, getApIface_cb cb) override; - Return createNanIface(createNanIface_cb cb) override; - Return getNanIfaceNames(getNanIfaceNames_cb cb) override; + const sp& event_callback, + registerEventCallback_cb hidl_status_cb) override; + Return getAvailableModes(getAvailableModes_cb hidl_status_cb) override; + Return configureChip(uint32_t mode_id, + configureChip_cb hidl_status_cb) override; + Return getMode(getMode_cb hidl_status_cb) override; + Return requestChipDebugInfo( + requestChipDebugInfo_cb hidl_status_cb) override; + Return requestDriverDebugDump( + requestDriverDebugDump_cb hidl_status_cb) override; + Return requestFirmwareDebugDump( + requestFirmwareDebugDump_cb hidl_status_cb) override; + Return createApIface(createApIface_cb hidl_status_cb) override; + Return getApIfaceNames(getApIfaceNames_cb hidl_status_cb) override; + Return getApIface(const hidl_string& ifname, + getApIface_cb hidl_status_cb) override; + Return createNanIface(createNanIface_cb hidl_status_cb) override; + Return getNanIfaceNames(getNanIfaceNames_cb hidl_status_cb) override; Return getNanIface(const hidl_string& ifname, - getNanIface_cb cb) override; - Return createP2pIface(createP2pIface_cb cb) override; - Return getP2pIfaceNames(getP2pIfaceNames_cb cb) override; + getNanIface_cb hidl_status_cb) override; + Return createP2pIface(createP2pIface_cb hidl_status_cb) override; + Return getP2pIfaceNames(getP2pIfaceNames_cb hidl_status_cb) override; Return getP2pIface(const hidl_string& ifname, - getP2pIface_cb cb) override; - Return createStaIface(createStaIface_cb cb) override; - Return getStaIfaceNames(getStaIfaceNames_cb cb) override; + getP2pIface_cb hidl_status_cb) override; + Return createStaIface(createStaIface_cb hidl_status_cb) override; + Return getStaIfaceNames(getStaIfaceNames_cb hidl_status_cb) override; Return getStaIface(const hidl_string& ifname, - getStaIface_cb cb) override; - Return createRttController(const sp& bound_iface, - createRttController_cb cb) override; + getStaIface_cb hidl_status_cb) override; + Return createRttController( + const sp& bound_iface, + createRttController_cb hidl_status_cb) override; private: void invalidateAndRemoveAllIfaces(); ChipId chip_id_; std::weak_ptr legacy_hal_; - std::vector> callbacks_; + std::vector> event_callbacks_; sp ap_iface_; sp nan_iface_; sp p2p_iface_;