wifi(vts): simplify HIDL calls
Presently, the core of the Wifi test logic is
obscured by the boilerplate required to create
a callback.
This CL provides some utilities to simplify
the creation of a HIDL result callback, and
migrates existing Wifi code to use the new
utilities.
Along the way: add a .clang-format file, so
that I don't misformat code with 2-space
indents (the Google default).
Bug: 34817351
Test: vts-tradefed run commandAndExit vts --module=HalWifiHidlTargetTest
Change-Id: Id2c728f96c3369c74adc8dfce7228b0a15a0781e
diff --git a/wifi/1.0/vts/functional/Android.bp b/wifi/1.0/vts/functional/Android.bp
index e2cd7b1..8a5d7e0 100644
--- a/wifi/1.0/vts/functional/Android.bp
+++ b/wifi/1.0/vts/functional/Android.bp
@@ -21,6 +21,7 @@
"main.cpp",
"wifi_ap_iface_hidl_test.cpp",
"wifi_chip_hidl_test.cpp",
+ "wifi_hidl_call_util_selftest.cpp",
"wifi_hidl_test.cpp",
"wifi_hidl_test_utils.cpp",
"wifi_nan_iface_hidl_test.cpp",
diff --git a/wifi/1.0/vts/functional/wifi_hidl_call_util.h b/wifi/1.0/vts/functional/wifi_hidl_call_util.h
new file mode 100644
index 0000000..03200a0
--- /dev/null
+++ b/wifi/1.0/vts/functional/wifi_hidl_call_util.h
@@ -0,0 +1,123 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <functional>
+#include <tuple>
+#include <type_traits>
+#include <utility>
+
+#include <gtest/gtest.h>
+
+namespace {
+namespace detail {
+template <typename>
+struct functionArgSaver;
+
+// Provides a std::function that takes one argument, and a buffer
+// wherein the function will store its argument. The buffer has
+// the same type as the argument, but with const and reference
+// modifiers removed.
+template <typename ArgT>
+struct functionArgSaver<std::function<void(ArgT)>> final {
+ using StorageT = typename std::remove_const<
+ typename std::remove_reference<ArgT>::type>::type;
+
+ std::function<void(ArgT)> saveArgs = [this](ArgT arg) {
+ this->saved_values = arg;
+ };
+
+ StorageT saved_values;
+};
+
+// Provides a std::function that takes two arguments, and a buffer
+// wherein the function will store its arguments. The buffer is a
+// std::pair, whose elements have the same types as the arguments
+// (but with const and reference modifiers removed).
+template <typename Arg1T, typename Arg2T>
+struct functionArgSaver<std::function<void(Arg1T, Arg2T)>> final {
+ using StorageT =
+ std::pair<typename std::remove_const<
+ typename std::remove_reference<Arg1T>::type>::type,
+ typename std::remove_const<
+ typename std::remove_reference<Arg2T>::type>::type>;
+
+ std::function<void(Arg1T, Arg2T)> saveArgs = [this](Arg1T arg1,
+ Arg2T arg2) {
+ this->saved_values = {arg1, arg2};
+ };
+
+ StorageT saved_values;
+};
+
+// Provides a std::function that takes three or more arguments, and a
+// buffer wherein the function will store its arguments. The buffer is a
+// std::tuple whose elements have the same types as the arguments (but
+// with const and reference modifiers removed).
+template <typename... ArgT>
+struct functionArgSaver<std::function<void(ArgT...)>> final {
+ using StorageT = std::tuple<typename std::remove_const<
+ typename std::remove_reference<ArgT>::type>::type...>;
+
+ std::function<void(ArgT...)> saveArgs = [this](ArgT... arg) {
+ this->saved_values = {arg...};
+ };
+
+ StorageT saved_values;
+};
+
+// Invokes |method| on |object|, providing |method| a CallbackT as the
+// final argument. Returns a copy of the parameters that |method| provided
+// to CallbackT. (The parameters are returned by value.)
+template <typename CallbackT, typename MethodT, typename ObjectT,
+ typename... ArgT>
+typename functionArgSaver<CallbackT>::StorageT invokeMethod(
+ MethodT method, ObjectT object, ArgT&&... methodArg) {
+ functionArgSaver<CallbackT> result_buffer;
+ const auto& res = ((*object).*method)(std::forward<ArgT>(methodArg)...,
+ result_buffer.saveArgs);
+ EXPECT_TRUE(res.isOk());
+ return result_buffer.saved_values;
+}
+} // namespace detail
+} // namespace
+
+// Invokes |method| on |strong_pointer|, passing provided arguments through to
+// |method|.
+//
+// Returns either:
+// - A copy of the result callback parameter (for callbacks with a single
+// parameter), OR
+// - A pair containing a copy of the result callback parameters (for callbacks
+// with two parameters), OR
+// - A tuple containing a copy of the result callback paramters (for callbacks
+// with three or more parameters).
+//
+// Example usage:
+// EXPECT_EQ(WifiStatusCode::SUCCESS,
+// HIDL_INVOKE(strong_pointer, methodReturningWifiStatus).code);
+// EXPECT_EQ(WifiStatusCode::SUCCESS,
+// HIDL_INVOKE(strong_pointer, methodReturningWifiStatusAndOneMore)
+// .first.code);
+// EXPECT_EQ(WifiStatusCode::SUCCESS, std::get<0>(
+// HIDL_INVOKE(strong_pointer, methodReturningWifiStatusAndTwoMore))
+// .code);
+#define HIDL_INVOKE(strong_pointer, method, ...) \
+ (detail::invokeMethod< \
+ std::remove_reference<decltype(*strong_pointer)>::type::method##_cb>( \
+ &std::remove_reference<decltype(*strong_pointer)>::type::method, \
+ strong_pointer, ##__VA_ARGS__))
diff --git a/wifi/1.0/vts/functional/wifi_hidl_call_util_selftest.cpp b/wifi/1.0/vts/functional/wifi_hidl_call_util_selftest.cpp
new file mode 100644
index 0000000..129bdb2
--- /dev/null
+++ b/wifi/1.0/vts/functional/wifi_hidl_call_util_selftest.cpp
@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <functional>
+#include <type_traits>
+
+#include <hidl/Status.h>
+#include <utils/RefBase.h>
+#include <utils/StrongPointer.h>
+
+#include "wifi_hidl_call_util.h"
+
+namespace {
+/*
+ * Example of a user-defined data-type.
+ *
+ * Used to verify that, within the internals of HIDL_INVOKE,
+ * reference parameters are stored by copy.
+ */
+class Dummy {};
+
+/*
+ * Example of what a HIDL-generated proxy might look like.
+ */
+class IExample : public ::android::RefBase {
+ public:
+ // The callback type, for a method called startWithCallbackCopy, which
+ // has a callback that takes an |int|. Both the name, and the value,
+ // must match what would appear in HIDL-generated code.
+ using startWithCallbackCopy_cb = std::function<void(int)>;
+
+ // The callback type, for a method called startWithCallbackReference, which
+ // has a callback that takes an |int|. Both the name, and the value,
+ // must match what would appear in HIDL-generated code.
+ using startWithCallbackReference_cb = std::function<void(int)>;
+
+ // Constants which allow tests to verify that the proxy methods can
+ // correctly return a value. We use different values for by-copy and
+ // by-reference, to double-check that a call was dispatched properly.
+ static constexpr int kByCopyResult = 42;
+ static constexpr int kByReferenceResult = 420;
+
+ // Example of what a no-arg method would look like, if the callback
+ // is passed by-value.
+ ::android::hardware::Return<void> startWithCallbackCopy(
+ startWithCallbackCopy_cb _hidl_cb) {
+ _hidl_cb(kByCopyResult);
+ return ::android::hardware::Void();
+ }
+ // Example of what a no-arg method would look like, if the callback
+ // is passed by const-reference.
+ ::android::hardware::Return<void> startWithCallbackReference(
+ const startWithCallbackReference_cb& _hidl_cb) {
+ _hidl_cb(kByReferenceResult);
+ return ::android::hardware::Void();
+ }
+};
+
+constexpr int IExample::kByCopyResult;
+constexpr int IExample::kByReferenceResult;
+} // namespace
+
+static_assert(std::is_same<int, detail::functionArgSaver<
+ std::function<void(int)>>::StorageT>::value,
+ "Single-arg result should be stored directly.");
+
+static_assert(
+ std::is_same<std::pair<int, long>, detail::functionArgSaver<std::function<
+ void(int, long)>>::StorageT>::value,
+ "Two-arg result should be stored as a pair.");
+
+static_assert(
+ std::is_same<std::tuple<char, int, long>,
+ detail::functionArgSaver<
+ std::function<void(char, int, long)>>::StorageT>::value,
+ "Three-arg result should be stored as a tuple.");
+
+static_assert(std::is_same<Dummy, detail::functionArgSaver<std::function<
+ void(const Dummy&)>>::StorageT>::value,
+ "Reference should be stored by copy.");
+
+/*
+ * Verifies that HIDL_INVOKE can be used with methods that take the result
+ * callback as a by-value parameter. (This reflects the current implementation
+ * of HIDL-generated code.)
+ */
+TEST(HidlInvokeTest, WorksWithMethodThatTakesResultCallbackByValue) {
+ ::android::sp<IExample> sp = new IExample();
+ EXPECT_EQ(IExample::kByCopyResult, HIDL_INVOKE(sp, startWithCallbackCopy));
+}
+
+/*
+ * Verifies that HIDL_INVOKE can be used with methods that take the result
+ * callback as a const-reference parameter. (This ensures that HIDL_INVOKE will
+ * continue to work, if the HIDL-generated code switches to const-ref.)
+ */
+TEST(HidlInvokeTest, WorksWithMethodThatTakesResultCallbackByConstReference) {
+ ::android::sp<IExample> sp = new IExample();
+ EXPECT_EQ(IExample::kByReferenceResult,
+ HIDL_INVOKE(sp, startWithCallbackReference));
+}
diff --git a/wifi/1.0/vts/functional/wifi_hidl_test_utils.cpp b/wifi/1.0/vts/functional/wifi_hidl_test_utils.cpp
index 050bba3..8f34a88 100644
--- a/wifi/1.0/vts/functional/wifi_hidl_test_utils.cpp
+++ b/wifi/1.0/vts/functional/wifi_hidl_test_utils.cpp
@@ -16,6 +16,7 @@
#include <gtest/gtest.h>
+#include "wifi_hidl_call_util.h"
#include "wifi_hidl_test_utils.h"
using ::android::hardware::wifi::V1_0::IWifi;
@@ -52,41 +53,23 @@
return nullptr;
}
- bool operation_failed = false;
- wifi->start([&](WifiStatus status) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- });
- if (operation_failed) {
+ if (HIDL_INVOKE(wifi, start).code != WifiStatusCode::SUCCESS) {
return nullptr;
}
- std::vector<ChipId> wifi_chip_ids;
- wifi->getChipIds(
- [&](const WifiStatus& status, const hidl_vec<ChipId>& chip_ids) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- wifi_chip_ids = chip_ids;
- });
- // We don't expect more than 1 chip currently.
- if (operation_failed || wifi_chip_ids.size() != 1) {
+ const auto& status_and_chip_ids = HIDL_INVOKE(wifi, getChipIds);
+ const auto& chip_ids = status_and_chip_ids.second;
+ if (status_and_chip_ids.first.code != WifiStatusCode::SUCCESS ||
+ chip_ids.size() != 1) {
return nullptr;
}
- sp<IWifiChip> wifi_chip;
- wifi->getChip(wifi_chip_ids[0],
- [&](const WifiStatus& status, const sp<IWifiChip>& chip) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- wifi_chip = chip;
- });
- if (operation_failed) {
+ const auto& status_and_chip = HIDL_INVOKE(wifi, getChip, chip_ids[0]);
+ if (status_and_chip.first.code != WifiStatusCode::SUCCESS) {
return nullptr;
}
- return wifi_chip;
+
+ return status_and_chip.second;
}
// Since we currently only support one iface of each type. Just iterate thru the
@@ -116,30 +99,18 @@
bool configureChipToSupportIfaceType(const sp<IWifiChip>& wifi_chip,
IfaceType type) {
- bool operation_failed = false;
- std::vector<IWifiChip::ChipMode> chip_modes;
- wifi_chip->getAvailableModes(
- [&](WifiStatus status, const hidl_vec<IWifiChip::ChipMode>& modes) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- chip_modes = modes;
- });
- if (operation_failed) {
+ const auto& status_and_modes = HIDL_INVOKE(wifi_chip, getAvailableModes);
+ if (status_and_modes.first.code != WifiStatusCode::SUCCESS) {
return false;
}
ChipModeId mode_id;
- if (!findModeToSupportIfaceType(type, chip_modes, &mode_id)) {
+ if (!findModeToSupportIfaceType(type, status_and_modes.second, &mode_id)) {
return false;
}
- wifi_chip->configureChip(mode_id, [&](WifiStatus status) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- });
- if (operation_failed) {
+ if (HIDL_INVOKE(wifi_chip, configureChip, mode_id).code !=
+ WifiStatusCode::SUCCESS) {
return false;
}
return true;
@@ -154,19 +125,11 @@
return nullptr;
}
- bool operation_failed = false;
- sp<IWifiApIface> wifi_ap_iface;
- wifi_chip->createApIface(
- [&](const WifiStatus& status, const sp<IWifiApIface>& iface) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- wifi_ap_iface = iface;
- });
- if (operation_failed) {
+ const auto& status_and_iface = HIDL_INVOKE(wifi_chip, createApIface);
+ if (status_and_iface.first.code != WifiStatusCode::SUCCESS) {
return nullptr;
}
- return wifi_ap_iface;
+ return status_and_iface.second;
}
sp<IWifiNanIface> getWifiNanIface() {
@@ -178,19 +141,11 @@
return nullptr;
}
- bool operation_failed = false;
- sp<IWifiNanIface> wifi_nan_iface;
- wifi_chip->createNanIface(
- [&](const WifiStatus& status, const sp<IWifiNanIface>& iface) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- wifi_nan_iface = iface;
- });
- if (operation_failed) {
+ const auto& status_and_iface = HIDL_INVOKE(wifi_chip, createNanIface);
+ if (status_and_iface.first.code != WifiStatusCode::SUCCESS) {
return nullptr;
}
- return wifi_nan_iface;
+ return status_and_iface.second;
}
sp<IWifiP2pIface> getWifiP2pIface() {
@@ -202,19 +157,11 @@
return nullptr;
}
- bool operation_failed = false;
- sp<IWifiP2pIface> wifi_p2p_iface;
- wifi_chip->createP2pIface(
- [&](const WifiStatus& status, const sp<IWifiP2pIface>& iface) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- wifi_p2p_iface = iface;
- });
- if (operation_failed) {
+ const auto& status_and_iface = HIDL_INVOKE(wifi_chip, createP2pIface);
+ if (status_and_iface.first.code != WifiStatusCode::SUCCESS) {
return nullptr;
}
- return wifi_p2p_iface;
+ return status_and_iface.second;
}
sp<IWifiStaIface> getWifiStaIface() {
@@ -226,19 +173,11 @@
return nullptr;
}
- bool operation_failed = false;
- sp<IWifiStaIface> wifi_sta_iface;
- wifi_chip->createStaIface(
- [&](const WifiStatus& status, const sp<IWifiStaIface>& iface) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- wifi_sta_iface = iface;
- });
- if (operation_failed) {
+ const auto& status_and_iface = HIDL_INVOKE(wifi_chip, createStaIface);
+ if (status_and_iface.first.code != WifiStatusCode::SUCCESS) {
return nullptr;
}
- return wifi_sta_iface;
+ return status_and_iface.second;
}
sp<IWifiRttController> getWifiRttController() {
@@ -251,26 +190,16 @@
return nullptr;
}
- bool operation_failed = false;
- sp<IWifiRttController> wifi_rtt_controller;
- wifi_chip->createRttController(
- wifi_sta_iface, [&](const WifiStatus& status,
- const sp<IWifiRttController>& controller) {
- if (status.code != WifiStatusCode::SUCCESS) {
- operation_failed = true;
- }
- wifi_rtt_controller = controller;
- });
- if (operation_failed) {
+ const auto& status_and_controller =
+ HIDL_INVOKE(wifi_chip, createRttController, wifi_sta_iface);
+ if (status_and_controller.first.code != WifiStatusCode::SUCCESS) {
return nullptr;
}
- return wifi_rtt_controller;
+ return status_and_controller.second;
}
void stopWifi() {
sp<IWifi> wifi = getWifi();
ASSERT_NE(wifi, nullptr);
- wifi->stop([](const WifiStatus& status) {
- ASSERT_EQ(status.code, WifiStatusCode::SUCCESS);
- });
+ ASSERT_EQ(HIDL_INVOKE(wifi, stop).code, WifiStatusCode::SUCCESS);
}