Offload HAL HIDL: Add return values to synchronous calls

Enhance the HIDL interface to return values indicating the status of the
operation requested by the API.

Bug: 32842314
Test: VTS
Change-Id: I9a75e2524c0256d5da405d5b1b5919d5342deebf
diff --git a/wifi/offload/1.0/IOffload.hal b/wifi/offload/1.0/IOffload.hal
index 7ad902c..4819519 100644
--- a/wifi/offload/1.0/IOffload.hal
+++ b/wifi/offload/1.0/IOffload.hal
@@ -25,21 +25,28 @@
      *
      * @param ScanParam paramters for scanning
      * @param ScanFilter settings to filter scan result
-     * @return boolean status indicating success (true) when configuration
-     *            is applied or failure (false) for invalid configuration
+     * @return OffloadStatus indicating status of operation provided by this API
+     * If OffloadStatusCode::OK is returned, the operation was successful
+     * If OffloadStatusCode::NO_CONNECTION is returned, connection to the hardware is lost
+     * If OffloadStatusCode::ERROR is returned, requested operation could not be completed
      */
     @entry
     @callflow(next={"setEventCallback", "subscribeScanResults"})
-    configureScans(ScanParam param, ScanFilter filter);
+    configureScans(ScanParam param, ScanFilter filter) generates (OffloadStatus status);
 
     /**
      * Get scan statistics
      *
+     * @return OffloadStatus indicating status of operation provided by this API
      * @return ScanStats statistics of scans performed
+     * If OffloadStatusCode::OK is returned, the operation was successful
+     * If OffloadStatusCode::NO_CONNECTION is returned, connection to the hardware is lost
+     * If OffloadStatusCode::ERROR is returned, requested operation could not be completed
+     * If OffloadStatusCode::TIMEOUT is returned, time out waiting for the requested data
      */
     @exit
     @callflow(next={"subscribeScanResults", "unsubscribeScanResults", "getScanStats"})
-    getScanStats() generates (ScanStats scanStats);
+    getScanStats() generates (OffloadStatus status, ScanStats scanStats);
 
     /**
      * Subscribe to asynchronous scan events sent by offload module. This enables
@@ -50,9 +57,13 @@
      *
      * @param delayMs an integer expressing the minimum delay in mS after
      *        subscribing when scan results must be delivered to the client
+     * @return OffloadStatus indicating status of operation provided by this API
+     * If OffloadStatusCode::OK is returned, the operation was successful
+     * If OffloadStatusCode::NO_CONNECTION is returned, connection to the hardware is lost
+     * If OffloadStatusCode::ERROR is returned, requested operation could not be completed
      */
     @callflow(next={"unsubscribeScanResults", "getScanStats"})
-    subscribeScanResults(uint32_t delayMs);
+    subscribeScanResults(uint32_t delayMs) generates (OffloadStatus status);
 
     /**
      * Unsubscribe to scan events sent by the offload module, hence disabling scans.
diff --git a/wifi/offload/1.0/types.hal b/wifi/offload/1.0/types.hal
index 38d5eda..234f3fc 100644
--- a/wifi/offload/1.0/types.hal
+++ b/wifi/offload/1.0/types.hal
@@ -202,18 +202,25 @@
 /**
  * Defines a list of return codes to indicate status of Offload HAL
  */
-enum OffloadStatus : uint32_t {
+enum OffloadStatusCode : uint32_t {
     /* No error */
-    OFFLOAD_STATUS_OK,
+    OK,
     /* No Connection to underlying implementation */
-    OFFLOAD_STATUS_NO_CONNECTION,
+    NO_CONNECTION,
     /* Operation timeout */
-    OFFLOAD_STATUS_TIMEOUT,
+    TIMEOUT,
     /* Other errors */
-    OFFLOAD_STATUS_ERROR
+    ERROR
 };
 
-
+/**
+ * Generic structures to return the status of an operation
+ */
+struct OffloadStatus {
+  OffloadStatusCode code;
+  /* Error message */
+  string description;
+};
 
 
 
diff --git a/wifi/offload/1.0/vts/functional/VtsHalWifiOffloadV1_0TargetTest.cpp b/wifi/offload/1.0/vts/functional/VtsHalWifiOffloadV1_0TargetTest.cpp
index 3020542..55f5a87 100644
--- a/wifi/offload/1.0/vts/functional/VtsHalWifiOffloadV1_0TargetTest.cpp
+++ b/wifi/offload/1.0/vts/functional/VtsHalWifiOffloadV1_0TargetTest.cpp
@@ -26,6 +26,8 @@
 
 #include <vector>
 
+#include "hidl_call_util.h"
+
 using ::android::hardware::wifi::offload::V1_0::IOffload;
 using ::android::hardware::wifi::offload::V1_0::IOffloadCallback;
 using ::android::hardware::wifi::offload::V1_0::ScanResult;
@@ -33,6 +35,7 @@
 using ::android::hardware::wifi::offload::V1_0::ScanFilter;
 using ::android::hardware::wifi::offload::V1_0::ScanStats;
 using ::android::hardware::wifi::offload::V1_0::OffloadStatus;
+using ::android::hardware::wifi::offload::V1_0::OffloadStatusCode;
 using ::android::hardware::Return;
 using ::android::hardware::Void;
 using ::android::hardware::hidl_vec;
@@ -89,7 +92,7 @@
             return Void();
         };
 
-        Return<void> onError(OffloadStatus status) {
+        Return<void> onError(const OffloadStatus& status) override {
             OffloadCallbackArgs args;
             args.error_code_ = status;
             NotifyFromCallback(kOffloadCallbackSendError, args);
@@ -106,15 +109,15 @@
  */
 TEST_F(WifiOffloadHidlTest, setEventCallback) {
     auto returnObject = wifi_offload_->setEventCallback(wifi_offload_cb_);
-    ASSERT_EQ(returnObject.isOk(), true);
+    ASSERT_EQ(true, returnObject.isOk());
 }
 
 /*
  * Verify that subscribeScanResults method returns without errors
  */
 TEST_F(WifiOffloadHidlTest, subscribeScanResults) {
-    auto returnObject = wifi_offload_->subscribeScanResults(0);
-    ASSERT_EQ(returnObject.isOk(), true);
+    const auto& result = HIDL_INVOKE(wifi_offload_, subscribeScanResults, 0);
+    ASSERT_EQ(OffloadStatusCode::OK, result.code);
 }
 
 /*
@@ -122,7 +125,7 @@
  */
 TEST_F(WifiOffloadHidlTest, unsubscribeScanResults) {
     auto returnObject = wifi_offload_->unsubscribeScanResults();
-    ASSERT_EQ(returnObject.isOk(), true);
+    ASSERT_EQ(true, returnObject.isOk());
 }
 
 /*
@@ -131,21 +134,18 @@
 TEST_F(WifiOffloadHidlTest, configureScans) {
     ScanParam* pScanParam = new ScanParam();
     ScanFilter* pScanFilter = new ScanFilter();
-    auto returnObject =
-        wifi_offload_->configureScans(*pScanParam, *pScanFilter);
-    ASSERT_EQ(returnObject.isOk(), true);
+    const auto& result =
+        HIDL_INVOKE(wifi_offload_, configureScans, *pScanParam, *pScanFilter);
+    ASSERT_EQ(OffloadStatusCode::OK, result.code);
 }
 
 /*
  * Verify that getScanStats returns without any errors
  */
 TEST_F(WifiOffloadHidlTest, getScanStats) {
-    ScanStats* pScanStats = new ScanStats();
-    const auto& returnObject =
-        wifi_offload_->getScanStats([pScanStats](ScanStats scanStats) -> void {
-            *pScanStats = std::move(scanStats);
-        });
-    ASSERT_EQ(returnObject.isOk(), true);
+    const auto& result = HIDL_INVOKE(wifi_offload_, getScanStats);
+    OffloadStatus status = result.first;
+    ASSERT_EQ(OffloadStatusCode::OK, status.code);
 }
 
 /*
@@ -167,7 +167,7 @@
     wifi_offload_cb_->onScanResult(scan_results);
     auto res =
         wifi_offload_cb_->WaitForCallback(kOffloadCallbackSendScanResult);
-    ASSERT_EQ(res.no_timeout, true);
+    ASSERT_EQ(true, res.no_timeout);
 }
 
 /*
@@ -175,9 +175,10 @@
  */
 TEST_F(WifiOffloadHidlTest, getError) {
     wifi_offload_->setEventCallback(wifi_offload_cb_);
-    wifi_offload_cb_->onError(OffloadStatus::OFFLOAD_STATUS_ERROR);
+    OffloadStatus status = {OffloadStatusCode::ERROR, ""};
+    wifi_offload_cb_->onError(status);
     auto res = wifi_offload_cb_->WaitForCallback(kOffloadCallbackSendError);
-    ASSERT_EQ(res.no_timeout, true);
+    ASSERT_EQ(true, res.no_timeout);
 }
 
 // A class for test environment setup
diff --git a/wifi/offload/1.0/vts/functional/hidl_call_util.h b/wifi/offload/1.0/vts/functional/hidl_call_util.h
new file mode 100644
index 0000000..f3ca517
--- /dev/null
+++ b/wifi/offload/1.0/vts/functional/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 <VtsHalHidlTargetTestBase.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__))