wifi: Fixing Nits
Changes in the CL to fix bugs found while integrating
with framework:
1. Added IWifiApIface.setCountryCode()
Bug: 34682168
2. Fixed documentation nits in .hal files.
Bug: 34468218
3. Changed roaming state enum values.
Bug: 34715231
4. Added log for EBUSY in legacyErrorToString()
Bug: 34715469
5. Remove "-Wno-unused parameter" from Android.mk
Bug: 34611289
6. Remove the erroneous |onFailure| callback invocation in wifi.cpp.
7. Add null terminations to strings passed to legacy HAL API's
8. Remove unused |IWifiStaIface.stopDebugPacketFateMonitoring|.
Test: Compiles
Change-Id: If3a3e2f360a378c59dc56b8bbe3a0c6d400b3bd8
diff --git a/wifi/1.0/Android.mk b/wifi/1.0/Android.mk
index 1058cc7..708c14c 100644
--- a/wifi/1.0/Android.mk
+++ b/wifi/1.0/Android.mk
@@ -1589,6 +1589,8 @@
$(GEN): PRIVATE_DEPS := $(LOCAL_PATH)/IWifiApIface.hal
$(GEN): PRIVATE_DEPS += $(LOCAL_PATH)/IWifiIface.hal
$(GEN): $(LOCAL_PATH)/IWifiIface.hal
+$(GEN): PRIVATE_DEPS += $(LOCAL_PATH)/types.hal
+$(GEN): $(LOCAL_PATH)/types.hal
$(GEN): PRIVATE_OUTPUT_DIR := $(intermediates)
$(GEN): PRIVATE_CUSTOM_TOOL = \
$(PRIVATE_HIDL) -o $(PRIVATE_OUTPUT_DIR) \
@@ -3447,6 +3449,8 @@
$(GEN): PRIVATE_DEPS := $(LOCAL_PATH)/IWifiApIface.hal
$(GEN): PRIVATE_DEPS += $(LOCAL_PATH)/IWifiIface.hal
$(GEN): $(LOCAL_PATH)/IWifiIface.hal
+$(GEN): PRIVATE_DEPS += $(LOCAL_PATH)/types.hal
+$(GEN): $(LOCAL_PATH)/types.hal
$(GEN): PRIVATE_OUTPUT_DIR := $(intermediates)
$(GEN): PRIVATE_CUSTOM_TOOL = \
$(PRIVATE_HIDL) -o $(PRIVATE_OUTPUT_DIR) \
diff --git a/wifi/1.0/IWifiApIface.hal b/wifi/1.0/IWifiApIface.hal
index 6bc3580..aeca2cd 100644
--- a/wifi/1.0/IWifiApIface.hal
+++ b/wifi/1.0/IWifiApIface.hal
@@ -22,5 +22,15 @@
* Interface used to represent a single AP iface.
*/
interface IWifiApIface extends IWifiIface {
- /** TODO(rpius): Add methods to the interface. */
+ /**
+ * Set country code for this iface.
+ *
+ * @param code 2 byte country code (as defined in ISO 3166) to set.
+ * @return status Status of the operation.
+ * Possible status codes:
+ * |WifiStatusCode.SUCCESS|,
+ * |WifiStatusCode.FAILURE_UNKNOWN|,
+ * |WifiStatusCode.FAILURE_IFACE_INVALID|
+ */
+ setCountryCode(int8_t[2] code) generates (WifiStatus status);
};
diff --git a/wifi/1.0/IWifiStaIface.hal b/wifi/1.0/IWifiStaIface.hal
index 6a738a9..2c72ead 100644
--- a/wifi/1.0/IWifiStaIface.hal
+++ b/wifi/1.0/IWifiStaIface.hal
@@ -439,7 +439,8 @@
/**
* API to start packet fate monitoring.
- * - Once stared, monitoring must remain active until HAL is unloaded.
+ * - Once started, monitoring must remain active until HAL is stopped or the
+ * chip is reconfigured.
* - When HAL is unloaded, all packet fate buffers must be cleared.
* - The packet fates are used to monitor the state of packets transmitted/
* received during association.
@@ -455,20 +456,6 @@
startDebugPacketFateMonitoring() generates (WifiStatus status);
/**
- * API to stop packet fate monitoring.
- *
- * @return status WifiStatus of the operation.
- * Possible status codes:
- * |WifiStatusCode.SUCCESS|,
- * |WifiStatusCode.ERROR_WIFI_IFACE_INVALID|,
- * |WifiStatusCode.ERROR_NOT_SUPPORTED|,
- * |WifiStatusCode.ERROR_NOT_STARTED|,
- * |WifiStatusCode.ERROR_NOT_AVAILABLE|,
- * |WifiStatusCode.ERROR_UNKNOWN|
- */
- stopDebugPacketFateMonitoring() generates (WifiStatus status);
-
- /**
* API to retrieve fates of outbound packets.
* - HAL implementation must return the fates of
* all the frames transmitted for the most recent association.
diff --git a/wifi/1.0/default/Android.mk b/wifi/1.0/default/Android.mk
index f0c78ea..144c067 100644
--- a/wifi/1.0/default/Android.mk
+++ b/wifi/1.0/default/Android.mk
@@ -16,7 +16,7 @@
include $(CLEAR_VARS)
LOCAL_MODULE := android.hardware.wifi@1.0-service
LOCAL_MODULE_RELATIVE_PATH := hw
-LOCAL_CPPFLAGS := -Wall -Wno-unused-parameter -Werror -Wextra
+LOCAL_CPPFLAGS := -Wall -Werror -Wextra
LOCAL_SRC_FILES := \
hidl_struct_util.cpp \
service.cpp \
diff --git a/wifi/1.0/default/wifi.cpp b/wifi/1.0/default/wifi.cpp
index c06abe8..8feb836 100644
--- a/wifi/1.0/default/wifi.cpp
+++ b/wifi/1.0/default/wifi.cpp
@@ -107,11 +107,6 @@
LOG(ERROR) << "Failed to invoke onStart callback";
};
}
- for (const auto& callback : event_callbacks_) {
- if (!callback->onFailure(wifi_status).isOk()) {
- LOG(ERROR) << "Failed to invoke onFailure callback";
- }
- }
} else {
for (const auto& callback : event_callbacks_) {
if (!callback->onFailure(wifi_status).isOk()) {
diff --git a/wifi/1.0/default/wifi_ap_iface.cpp b/wifi/1.0/default/wifi_ap_iface.cpp
index b8b7a3a..1a8b31d 100644
--- a/wifi/1.0/default/wifi_ap_iface.cpp
+++ b/wifi/1.0/default/wifi_ap_iface.cpp
@@ -55,6 +55,15 @@
hidl_status_cb);
}
+Return<void> WifiApIface::setCountryCode(const hidl_array<int8_t, 2>& code,
+ setCountryCode_cb hidl_status_cb) {
+ return validateAndCall(this,
+ WifiStatusCode::ERROR_WIFI_IFACE_INVALID,
+ &WifiApIface::setCountryCodeInternal,
+ hidl_status_cb,
+ code);
+}
+
std::pair<WifiStatus, std::string> WifiApIface::getNameInternal() {
return {createWifiStatus(WifiStatusCode::SUCCESS), ifname_};
}
@@ -63,6 +72,13 @@
return {createWifiStatus(WifiStatusCode::SUCCESS), IfaceType::AP};
}
+WifiStatus WifiApIface::setCountryCodeInternal(
+ const std::array<int8_t, 2>& code) {
+ legacy_hal::wifi_error legacy_status =
+ legacy_hal_.lock()->setCountryCode(code);
+ return createWifiStatusFromLegacyError(legacy_status);
+}
+
} // namespace implementation
} // namespace V1_0
} // namespace wifi
diff --git a/wifi/1.0/default/wifi_ap_iface.h b/wifi/1.0/default/wifi_ap_iface.h
index ee5dc56..23d6435 100644
--- a/wifi/1.0/default/wifi_ap_iface.h
+++ b/wifi/1.0/default/wifi_ap_iface.h
@@ -42,11 +42,14 @@
// HIDL methods exposed.
Return<void> getName(getName_cb hidl_status_cb) override;
Return<void> getType(getType_cb hidl_status_cb) override;
+ Return<void> setCountryCode(const hidl_array<int8_t, 2>& code,
+ setCountryCode_cb hidl_status_cb) override;
private:
// Corresponding worker functions for the HIDL methods.
std::pair<WifiStatus, std::string> getNameInternal();
std::pair<WifiStatus, IfaceType> getTypeInternal();
+ WifiStatus setCountryCodeInternal(const std::array<int8_t, 2>& code);
std::string ifname_;
std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal_;
diff --git a/wifi/1.0/default/wifi_legacy_hal.cpp b/wifi/1.0/default/wifi_legacy_hal.cpp
index 3bfd2bb..e4eddcd 100644
--- a/wifi/1.0/default/wifi_legacy_hal.cpp
+++ b/wifi/1.0/default/wifi_legacy_hal.cpp
@@ -22,12 +22,7 @@
#include "wifi_legacy_hal.h"
#include "wifi_legacy_hal_stubs.h"
-namespace android {
-namespace hardware {
-namespace wifi {
-namespace V1_0 {
-namespace implementation {
-namespace legacy_hal {
+namespace {
// Constants ported over from the legacy HAL calling code
// (com_android_server_wifi_WifiNative.cpp). This will all be thrown
// away when this shim layer is replaced by the real vendor
@@ -39,6 +34,21 @@
static constexpr uint32_t kMaxWakeReasonStatsArraySize = 32;
static constexpr uint32_t kMaxRingBuffers = 10;
+// Helper function to create a non-const char* for legacy Hal API's.
+std::vector<char> makeCharVec(const std::string& str) {
+ std::vector<char> vec(str.size() + 1);
+ vec.assign(str.begin(), str.end());
+ vec.push_back('\0');
+ return vec;
+}
+} // namespace
+
+namespace android {
+namespace hardware {
+namespace wifi {
+namespace V1_0 {
+namespace implementation {
+namespace legacy_hal {
// Legacy HAL functions accept "C" style function pointers, so use global
// functions to pass to the legacy HAL function and store the corresponding
// std::function methods to be invoked.
@@ -804,19 +814,17 @@
uint32_t verbose_level,
uint32_t max_interval_sec,
uint32_t min_data_size) {
- std::vector<char> ring_name_internal(ring_name.begin(), ring_name.end());
return global_func_table_.wifi_start_logging(wlan_interface_handle_,
verbose_level,
0,
max_interval_sec,
min_data_size,
- ring_name_internal.data());
+ makeCharVec(ring_name).data());
}
wifi_error WifiLegacyHal::getRingBufferData(const std::string& ring_name) {
- std::vector<char> ring_name_internal(ring_name.begin(), ring_name.end());
return global_func_table_.wifi_get_ring_data(wlan_interface_handle_,
- ring_name_internal.data());
+ makeCharVec(ring_name).data());
}
wifi_error WifiLegacyHal::registerErrorAlertCallbackHandler(
@@ -1091,16 +1099,14 @@
wifi_error WifiLegacyHal::nanDataInterfaceCreate(
transaction_id id, const std::string& iface_name) {
- std::vector<char> iface_name_internal(iface_name.begin(), iface_name.end());
return global_func_table_.wifi_nan_data_interface_create(
- id, wlan_interface_handle_, iface_name_internal.data());
+ id, wlan_interface_handle_, makeCharVec(iface_name).data());
}
wifi_error WifiLegacyHal::nanDataInterfaceDelete(
transaction_id id, const std::string& iface_name) {
- std::vector<char> iface_name_internal(iface_name.begin(), iface_name.end());
return global_func_table_.wifi_nan_data_interface_delete(
- id, wlan_interface_handle_, iface_name_internal.data());
+ id, wlan_interface_handle_, makeCharVec(iface_name).data());
}
wifi_error WifiLegacyHal::nanDataRequestInitiator(
@@ -1124,6 +1130,12 @@
id, wlan_interface_handle_, &msg_internal);
}
+wifi_error WifiLegacyHal::setCountryCode(std::array<int8_t, 2> code) {
+ std::string code_str(code.data(), code.data() + code.size());
+ return global_func_table_.wifi_set_country_code(wlan_interface_handle_,
+ code_str.c_str());
+}
+
wifi_error WifiLegacyHal::retrieveWlanInterfaceHandle() {
const std::string& ifname_to_find = getStaIfaceName();
wifi_interface_handle* iface_handles = nullptr;
diff --git a/wifi/1.0/default/wifi_legacy_hal.h b/wifi/1.0/default/wifi_legacy_hal.h
index a3ac075..1ab74b7 100644
--- a/wifi/1.0/default/wifi_legacy_hal.h
+++ b/wifi/1.0/default/wifi_legacy_hal.h
@@ -256,6 +256,8 @@
wifi_error nanDataIndicationResponse(
transaction_id id, const NanDataPathIndicationResponse& msg);
wifi_error nanDataEnd(transaction_id id, const NanDataPathEndRequest& msg);
+ // AP functions.
+ wifi_error setCountryCode(std::array<int8_t, 2> code);
private:
// Retrieve the interface handle to be used for the "wlan" interface.
diff --git a/wifi/1.0/default/wifi_sta_iface.cpp b/wifi/1.0/default/wifi_sta_iface.cpp
index a00c5bc..be2fe37 100644
--- a/wifi/1.0/default/wifi_sta_iface.cpp
+++ b/wifi/1.0/default/wifi_sta_iface.cpp
@@ -258,14 +258,6 @@
hidl_status_cb);
}
-Return<void> WifiStaIface::stopDebugPacketFateMonitoring(
- stopDebugPacketFateMonitoring_cb hidl_status_cb) {
- return validateAndCall(this,
- WifiStatusCode::ERROR_WIFI_IFACE_INVALID,
- &WifiStaIface::stopDebugPacketFateMonitoringInternal,
- hidl_status_cb);
-}
-
Return<void> WifiStaIface::getDebugTxPacketFates(
getDebugTxPacketFates_cb hidl_status_cb) {
return validateAndCall(this,
@@ -567,11 +559,6 @@
return createWifiStatusFromLegacyError(legacy_status);
}
-WifiStatus WifiStaIface::stopDebugPacketFateMonitoringInternal() {
- // There is no stop in legacy HAL implementation.
- return createWifiStatus(WifiStatusCode::ERROR_NOT_SUPPORTED);
-}
-
std::pair<WifiStatus, std::vector<WifiDebugTxPacketFateReport>>
WifiStaIface::getDebugTxPacketFatesInternal() {
legacy_hal::wifi_error legacy_status;
diff --git a/wifi/1.0/default/wifi_sta_iface.h b/wifi/1.0/default/wifi_sta_iface.h
index 311c991..ca79c5b 100644
--- a/wifi/1.0/default/wifi_sta_iface.h
+++ b/wifi/1.0/default/wifi_sta_iface.h
@@ -97,8 +97,6 @@
uint32_t cmd_id, stopSendingKeepAlivePackets_cb hidl_status_cb) override;
Return<void> startDebugPacketFateMonitoring(
startDebugPacketFateMonitoring_cb hidl_status_cb) override;
- Return<void> stopDebugPacketFateMonitoring(
- stopDebugPacketFateMonitoring_cb hidl_status_cb) override;
Return<void> getDebugTxPacketFates(
getDebugTxPacketFates_cb hidl_status_cb) override;
Return<void> getDebugRxPacketFates(
@@ -143,7 +141,6 @@
uint32_t period_in_ms);
WifiStatus stopSendingKeepAlivePacketsInternal(uint32_t cmd_id);
WifiStatus startDebugPacketFateMonitoringInternal();
- WifiStatus stopDebugPacketFateMonitoringInternal();
std::pair<WifiStatus, std::vector<WifiDebugTxPacketFateReport>>
getDebugTxPacketFatesInternal();
std::pair<WifiStatus, std::vector<WifiDebugRxPacketFateReport>>
diff --git a/wifi/1.0/default/wifi_status_util.cpp b/wifi/1.0/default/wifi_status_util.cpp
index 518032f..c2d0758 100644
--- a/wifi/1.0/default/wifi_status_util.cpp
+++ b/wifi/1.0/default/wifi_status_util.cpp
@@ -42,8 +42,9 @@
return "TOO_MANY_REQUESTS";
case legacy_hal::WIFI_ERROR_OUT_OF_MEMORY:
return "OUT_OF_MEMORY";
+ case legacy_hal::WIFI_ERROR_BUSY:
+ return "BUSY";
case legacy_hal::WIFI_ERROR_UNKNOWN:
- default:
return "UNKNOWN";
}
}
@@ -90,7 +91,6 @@
return createWifiStatus(WifiStatusCode::SUCCESS, desc);
case legacy_hal::WIFI_ERROR_UNKNOWN:
- default:
return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN, "unknown");
}
}
diff --git a/wifi/1.0/types.hal b/wifi/1.0/types.hal
index edf306d..bb00114 100644
--- a/wifi/1.0/types.hal
+++ b/wifi/1.0/types.hal
@@ -549,14 +549,14 @@
*/
enum StaRoamingState : uint8_t {
/**
+ * Driver/Firmware must not perform any roaming.
+ */
+ DISABLED = 0,
+ /**
* Driver/Firmware is allowed to perform roaming respecting
* the |StaRoamingConfig| parameters set using |configureRoaming|.
*/
- ENABLED = 0,
- /**
- * Driver/Firmware must not perform any roaming.
- */
- DISABLED = 1
+ ENABLED = 1
};
/**
@@ -2142,7 +2142,7 @@
};
/**
- * Struct describing packet fate report for each Rx frame.
+ * Struct describing packet fate report for each Tx frame.
*/
struct WifiDebugTxPacketFateReport {
WifiDebugTxPacketFate fate;