Cleanup NN callback error handling
This CL introduces a new templated class CallbackValue to handle HIDL
"return value" callbacks in a terser and more readable way.
This CL also introduces a new macro HANDLE_HAL_STATUS to return from the
current function when an error is present with the ability to append a
more descriptive error message.
Finally, this CL changes the behavior of synchronous executions. Prior
to this CL, IPreparedModel fell back to an asynchronous execution if the
synchronous execution was allowed and failed. This change instead
returns a failure if synchronous execution is allowed and fails.
Bug: 173084343
Test: mma
Change-Id: I62714a932e71dfc77401bbcb9eaaaf3d94fb9707
Merged-In: I62714a932e71dfc77401bbcb9eaaaf3d94fb9707
(cherry picked from commit 98ed9baf5de85599847b2b2f53585243c3b7b776)
diff --git a/neuralnetworks/1.0/utils/include/nnapi/hal/1.0/Callbacks.h b/neuralnetworks/1.0/utils/include/nnapi/hal/1.0/Callbacks.h
index 2e00fce..3b32e1d 100644
--- a/neuralnetworks/1.0/utils/include/nnapi/hal/1.0/Callbacks.h
+++ b/neuralnetworks/1.0/utils/include/nnapi/hal/1.0/Callbacks.h
@@ -32,6 +32,26 @@
namespace android::hardware::neuralnetworks::V1_0::utils {
+// Converts the results of IDevice::getSupportedOperations* to the NN canonical format. On success,
+// this function returns with the supported operations as indicated by a driver. On failure, this
+// function returns with the appropriate nn::GeneralError.
+nn::GeneralResult<std::vector<bool>> supportedOperationsCallback(
+ ErrorStatus status, const hidl_vec<bool>& supportedOperations);
+
+// Converts the results of IDevice::prepareModel* to the NN canonical format. On success, this
+// function returns with a non-null nn::SharedPreparedModel with a feature level of
+// nn::Version::ANDROID_OC_MR1. On failure, this function returns with the appropriate
+// nn::GeneralError.
+nn::GeneralResult<nn::SharedPreparedModel> prepareModelCallback(
+ ErrorStatus status, const sp<IPreparedModel>& preparedModel);
+
+// Converts the results of IDevice::execute* to the NN canonical format. On success, this function
+// returns with an empty output shape vector and no timing information. On failure, this function
+// returns with the appropriate nn::ExecutionError.
+nn::ExecutionResult<std::pair<std::vector<nn::OutputShape>, nn::Timing>> executionCallback(
+ ErrorStatus status);
+
+// A HIDL callback class to receive the results of IDevice::prepareModel asynchronously.
class PreparedModelCallback final : public IPreparedModelCallback,
public hal::utils::IProtectedCallback {
public:
@@ -44,11 +64,10 @@
Data get();
private:
- void notifyInternal(Data result);
-
hal::utils::TransferValue<Data> mData;
};
+// A HIDL callback class to receive the results of IDevice::execute asynchronously.
class ExecutionCallback final : public IExecutionCallback, public hal::utils::IProtectedCallback {
public:
using Data = nn::ExecutionResult<std::pair<std::vector<nn::OutputShape>, nn::Timing>>;
@@ -60,8 +79,6 @@
Data get();
private:
- void notifyInternal(Data result);
-
hal::utils::TransferValue<Data> mData;
};
diff --git a/neuralnetworks/1.0/utils/src/Callbacks.cpp b/neuralnetworks/1.0/utils/src/Callbacks.cpp
index a0bdb3c..ea3ea56 100644
--- a/neuralnetworks/1.0/utils/src/Callbacks.cpp
+++ b/neuralnetworks/1.0/utils/src/Callbacks.cpp
@@ -27,6 +27,7 @@
#include <nnapi/Result.h>
#include <nnapi/Types.h>
#include <nnapi/hal/CommonUtils.h>
+#include <nnapi/hal/HandleError.h>
#include <nnapi/hal/ProtectCallback.h>
#include <nnapi/hal/TransferValue.h>
@@ -36,63 +37,52 @@
// lifetimes across processes and for protecting asynchronous calls across HIDL.
namespace android::hardware::neuralnetworks::V1_0::utils {
-namespace {
-nn::GeneralResult<nn::SharedPreparedModel> convertPreparedModel(
- const sp<IPreparedModel>& preparedModel) {
- return NN_TRY(utils::PreparedModel::create(preparedModel));
+nn::GeneralResult<std::vector<bool>> supportedOperationsCallback(
+ ErrorStatus status, const hidl_vec<bool>& supportedOperations) {
+ HANDLE_HAL_STATUS(status) << "get supported operations failed with " << toString(status);
+ return supportedOperations;
}
-} // namespace
+nn::GeneralResult<nn::SharedPreparedModel> prepareModelCallback(
+ ErrorStatus status, const sp<IPreparedModel>& preparedModel) {
+ HANDLE_HAL_STATUS(status) << "model preparation failed with " << toString(status);
+ return NN_TRY(PreparedModel::create(preparedModel));
+}
+
+nn::ExecutionResult<std::pair<std::vector<nn::OutputShape>, nn::Timing>> executionCallback(
+ ErrorStatus status) {
+ HANDLE_HAL_STATUS(status) << "execution failed with " << toString(status);
+ return {};
+}
Return<void> PreparedModelCallback::notify(ErrorStatus status,
const sp<IPreparedModel>& preparedModel) {
- if (status != ErrorStatus::NONE) {
- const auto canonical = nn::convert(status).value_or(nn::ErrorStatus::GENERAL_FAILURE);
- notifyInternal(NN_ERROR(canonical) << "preparedModel failed with " << toString(status));
- } else if (preparedModel == nullptr) {
- notifyInternal(NN_ERROR(nn::ErrorStatus::GENERAL_FAILURE)
- << "Returned preparedModel is nullptr");
- } else {
- notifyInternal(convertPreparedModel(preparedModel));
- }
+ mData.put(prepareModelCallback(status, preparedModel));
return Void();
}
void PreparedModelCallback::notifyAsDeadObject() {
- notifyInternal(NN_ERROR(nn::ErrorStatus::DEAD_OBJECT) << "Dead object");
+ mData.put(NN_ERROR(nn::ErrorStatus::DEAD_OBJECT) << "Dead object");
}
PreparedModelCallback::Data PreparedModelCallback::get() {
return mData.take();
}
-void PreparedModelCallback::notifyInternal(PreparedModelCallback::Data result) {
- mData.put(std::move(result));
-}
-
// ExecutionCallback methods begin here
Return<void> ExecutionCallback::notify(ErrorStatus status) {
- if (status != ErrorStatus::NONE) {
- const auto canonical = nn::convert(status).value_or(nn::ErrorStatus::GENERAL_FAILURE);
- notifyInternal(NN_ERROR(canonical) << "execute failed with " << toString(status));
- } else {
- notifyInternal({});
- }
+ mData.put(executionCallback(status));
return Void();
}
void ExecutionCallback::notifyAsDeadObject() {
- notifyInternal(NN_ERROR(nn::ErrorStatus::DEAD_OBJECT) << "Dead object");
+ mData.put(NN_ERROR(nn::ErrorStatus::DEAD_OBJECT) << "Dead object");
}
ExecutionCallback::Data ExecutionCallback::get() {
return mData.take();
}
-void ExecutionCallback::notifyInternal(ExecutionCallback::Data result) {
- mData.put(std::move(result));
-}
-
} // namespace android::hardware::neuralnetworks::V1_0::utils
diff --git a/neuralnetworks/1.0/utils/src/Device.cpp b/neuralnetworks/1.0/utils/src/Device.cpp
index 83e0015..93bd81a 100644
--- a/neuralnetworks/1.0/utils/src/Device.cpp
+++ b/neuralnetworks/1.0/utils/src/Device.cpp
@@ -31,6 +31,7 @@
#include <nnapi/hal/CommonUtils.h>
#include <nnapi/hal/HandleError.h>
#include <nnapi/hal/ProtectCallback.h>
+#include <nnapi/hal/TransferValue.h>
#include <functional>
#include <memory>
@@ -44,24 +45,21 @@
namespace android::hardware::neuralnetworks::V1_0::utils {
namespace {
-nn::GeneralResult<nn::Capabilities> initCapabilities(V1_0::IDevice* device) {
+nn::GeneralResult<nn::Capabilities> capabilitiesCallback(ErrorStatus status,
+ const Capabilities& capabilities) {
+ HANDLE_HAL_STATUS(status) << "getting capabilities failed with " << toString(status);
+ return nn::convert(capabilities);
+}
+
+nn::GeneralResult<nn::Capabilities> getCapabilitiesFrom(V1_0::IDevice* device) {
CHECK(device != nullptr);
- nn::GeneralResult<nn::Capabilities> result = NN_ERROR(nn::ErrorStatus::GENERAL_FAILURE)
- << "uninitialized";
- const auto cb = [&result](ErrorStatus status, const Capabilities& capabilities) {
- if (status != ErrorStatus::NONE) {
- const auto canonical = nn::convert(status).value_or(nn::ErrorStatus::GENERAL_FAILURE);
- result = NN_ERROR(canonical) << "getCapabilities failed with " << toString(status);
- } else {
- result = nn::convert(capabilities);
- }
- };
+ auto cb = hal::utils::CallbackValue(capabilitiesCallback);
const auto ret = device->getCapabilities(cb);
HANDLE_TRANSPORT_FAILURE(ret);
- return result;
+ return cb.take();
}
} // namespace
@@ -77,7 +75,7 @@
<< "V1_0::utils::Device::create must have non-null device";
}
- auto capabilities = NN_TRY(initCapabilities(device.get()));
+ auto capabilities = NN_TRY(getCapabilitiesFrom(device.get()));
auto deathHandler = NN_TRY(hal::utils::DeathHandler::create(device));
return std::make_shared<const Device>(PrivateConstructorTag{}, std::move(name),
@@ -134,27 +132,12 @@
const auto hidlModel = NN_TRY(convert(modelInShared));
- nn::GeneralResult<std::vector<bool>> result = NN_ERROR(nn::ErrorStatus::GENERAL_FAILURE)
- << "uninitialized";
- auto cb = [&result, &model](ErrorStatus status, const hidl_vec<bool>& supportedOperations) {
- if (status != ErrorStatus::NONE) {
- const auto canonical = nn::convert(status).value_or(nn::ErrorStatus::GENERAL_FAILURE);
- result = NN_ERROR(canonical)
- << "getSupportedOperations failed with " << toString(status);
- } else if (supportedOperations.size() != model.main.operations.size()) {
- result = NN_ERROR(nn::ErrorStatus::GENERAL_FAILURE)
- << "getSupportedOperations returned vector of size "
- << supportedOperations.size() << " but expected "
- << model.main.operations.size();
- } else {
- result = supportedOperations;
- }
- };
+ auto cb = hal::utils::CallbackValue(supportedOperationsCallback);
const auto ret = kDevice->getSupportedOperations(hidlModel, cb);
HANDLE_TRANSPORT_FAILURE(ret);
- return result;
+ return cb.take();
}
nn::GeneralResult<nn::SharedPreparedModel> Device::prepareModel(
@@ -173,10 +156,7 @@
const auto ret = kDevice->prepareModel(hidlModel, cb);
const auto status = HANDLE_TRANSPORT_FAILURE(ret);
- if (status != ErrorStatus::NONE) {
- const auto canonical = nn::convert(status).value_or(nn::ErrorStatus::GENERAL_FAILURE);
- return NN_ERROR(canonical) << "prepareModel failed with " << toString(status);
- }
+ HANDLE_HAL_STATUS(status) << "model preparation failed with " << toString(status);
return cb->get();
}
diff --git a/neuralnetworks/1.0/utils/src/PreparedModel.cpp b/neuralnetworks/1.0/utils/src/PreparedModel.cpp
index c1dd1d9..c0c22fb 100644
--- a/neuralnetworks/1.0/utils/src/PreparedModel.cpp
+++ b/neuralnetworks/1.0/utils/src/PreparedModel.cpp
@@ -42,8 +42,7 @@
nn::GeneralResult<std::shared_ptr<const PreparedModel>> PreparedModel::create(
sp<V1_0::IPreparedModel> preparedModel) {
if (preparedModel == nullptr) {
- return NN_ERROR(nn::ErrorStatus::INVALID_ARGUMENT)
- << "V1_0::utils::PreparedModel::create must have non-null preparedModel";
+ return NN_ERROR() << "V1_0::utils::PreparedModel::create must have non-null preparedModel";
}
auto deathHandler = NN_TRY(hal::utils::DeathHandler::create(preparedModel));
@@ -71,10 +70,7 @@
const auto ret = kPreparedModel->execute(hidlRequest, cb);
const auto status = HANDLE_TRANSPORT_FAILURE(ret);
- if (status != ErrorStatus::NONE) {
- const auto canonical = nn::convert(status).value_or(nn::ErrorStatus::GENERAL_FAILURE);
- return NN_ERROR(canonical) << "execute failed with " << toString(status);
- }
+ HANDLE_HAL_STATUS(status) << "execution failed with " << toString(status);
auto result = NN_TRY(cb->get());
NN_TRY(hal::utils::makeExecutionFailure(