diff options
author | Arthur Ishiguro <arthuri@google.com> | 2022-01-06 22:42:10 +0000 |
---|---|---|
committer | Arthur Ishiguro <arthuri@google.com> | 2022-01-07 16:58:59 +0000 |
commit | 070f47dd7fd64c0c979a85a65c23fe559f65a6ed (patch) | |
tree | fc67eb83a088356d969cd0921c10aaa2fa790ce8 | |
parent | 7ae4255438c5b187186416e2a5c84842eeed1192 (diff) |
Make return values of IContextHub.aidl more consistent
Follow the pattern of using well-defined AIDL error codes as
return values for methods.
Bug: 213474931
Test: Compile, run VTS
Change-Id: If04d989cf504161638ec47b2302e60cbf32db502
5 files changed, 104 insertions, 117 deletions
diff --git a/contexthub/aidl/aidl_api/android.hardware.contexthub/current/android/hardware/contexthub/IContextHub.aidl b/contexthub/aidl/aidl_api/android.hardware.contexthub/current/android/hardware/contexthub/IContextHub.aidl index facce4b65e..f0676bec74 100644 --- a/contexthub/aidl/aidl_api/android.hardware.contexthub/current/android/hardware/contexthub/IContextHub.aidl +++ b/contexthub/aidl/aidl_api/android.hardware.contexthub/current/android/hardware/contexthub/IContextHub.aidl @@ -35,14 +35,15 @@ package android.hardware.contexthub; @VintfStability interface IContextHub { List<android.hardware.contexthub.ContextHubInfo> getContextHubs(); - boolean loadNanoapp(in int contextHubId, in android.hardware.contexthub.NanoappBinary appBinary, in int transactionId); - boolean unloadNanoapp(in int contextHubId, in long appId, in int transactionId); - boolean disableNanoapp(in int contextHubId, in long appId, in int transactionId); - boolean enableNanoapp(in int contextHubId, in long appId, in int transactionId); + void loadNanoapp(in int contextHubId, in android.hardware.contexthub.NanoappBinary appBinary, in int transactionId); + void unloadNanoapp(in int contextHubId, in long appId, in int transactionId); + void disableNanoapp(in int contextHubId, in long appId, in int transactionId); + void enableNanoapp(in int contextHubId, in long appId, in int transactionId); void onSettingChanged(in android.hardware.contexthub.Setting setting, in boolean enabled); - boolean queryNanoapps(in int contextHubId); - boolean registerCallback(in int contextHubId, in android.hardware.contexthub.IContextHubCallback cb); - boolean sendMessageToHub(in int contextHubId, in android.hardware.contexthub.ContextHubMessage message); + void queryNanoapps(in int contextHubId); + void registerCallback(in int contextHubId, in android.hardware.contexthub.IContextHubCallback cb); + void sendMessageToHub(in int contextHubId, in android.hardware.contexthub.ContextHubMessage message); void onHostEndpointConnected(in android.hardware.contexthub.HostEndpointInfo hostEndpointInfo); void onHostEndpointDisconnected(char hostEndpointId); + const int EX_CONTEXT_HUB_UNSPECIFIED = -1; } diff --git a/contexthub/aidl/android/hardware/contexthub/IContextHub.aidl b/contexthub/aidl/android/hardware/contexthub/IContextHub.aidl index 33d241a3c3..2135041ed7 100644 --- a/contexthub/aidl/android/hardware/contexthub/IContextHub.aidl +++ b/contexthub/aidl/android/hardware/contexthub/IContextHub.aidl @@ -52,9 +52,12 @@ interface IContextHub { * @param appBinary The nanoapp binary with header * @param transactionId The transaction ID associated with this request * - * @return The return code + * @throws EX_ILLEGAL_ARGUMENT if any of the arguments are invalid. + * EX_UNSUPPORTED_OPERATION if this functionality is unsupported. + * EX_SERVICE_SPECIFIC on error + * - EX_CONTEXT_HUB_UNSPECIFIED if the request failed for other reasons. */ - boolean loadNanoapp(in int contextHubId, in NanoappBinary appBinary, in int transactionId); + void loadNanoapp(in int contextHubId, in NanoappBinary appBinary, in int transactionId); /** * Invokes the nanoapp's deinitialization "end()" entrypoint, and unloads the nanoapp. @@ -69,9 +72,12 @@ interface IContextHub { * @param appId The unique ID of the nanoapp * @param transactionId The transaction ID associated with this request * - * @return The return code + * @throws EX_ILLEGAL_ARGUMENT if any of the arguments are invalid. + * EX_UNSUPPORTED_OPERATION if this functionality is unsupported. + * EX_SERVICE_SPECIFIC on error + * - EX_CONTEXT_HUB_UNSPECIFIED if the request failed for other reasons. */ - boolean unloadNanoapp(in int contextHubId, in long appId, in int transactionId); + void unloadNanoapp(in int contextHubId, in long appId, in int transactionId); /** * Disables a nanoapp by invoking the nanoapp's "end()" entrypoint, but does not unload the @@ -87,9 +93,12 @@ interface IContextHub { * @param appId The unique ID of the nanoapp * @param transactionId The transaction ID associated with this request * - * @return The return code + * @throws EX_ILLEGAL_ARGUMENT if any of the arguments are invalid. + * EX_UNSUPPORTED_OPERATION if this functionality is unsupported. + * EX_SERVICE_SPECIFIC on error + * - EX_CONTEXT_HUB_UNSPECIFIED if the request failed for other reasons. */ - boolean disableNanoapp(in int contextHubId, in long appId, in int transactionId); + void disableNanoapp(in int contextHubId, in long appId, in int transactionId); /** * Enables a nanoapp by invoking the nanoapp's initialization "start()" entrypoint. @@ -104,9 +113,12 @@ interface IContextHub { * @param appId appIdentifier returned by the HAL * @param message message to be sent * - * @return true on success + * @throws EX_ILLEGAL_ARGUMENT if any of the arguments are invalid. + * EX_UNSUPPORTED_OPERATION if this functionality is unsupported. + * EX_SERVICE_SPECIFIC on error + * - EX_CONTEXT_HUB_UNSPECIFIED if the request failed for other reasons. */ - boolean enableNanoapp(in int contextHubId, in long appId, in int transactionId); + void enableNanoapp(in int contextHubId, in long appId, in int transactionId); /** * Notification sent by the framework to indicate that the user has changed a setting. @@ -124,9 +136,12 @@ interface IContextHub { * * @param contextHubId The identifier of the Context Hub * - * @return true on success + * @throws EX_ILLEGAL_ARGUMENT if any of the arguments are invalid. + * EX_UNSUPPORTED_OPERATION if this functionality is unsupported. + * EX_SERVICE_SPECIFIC on error + * - EX_CONTEXT_HUB_UNSPECIFIED if the request failed for other reasons. */ - boolean queryNanoapps(in int contextHubId); + void queryNanoapps(in int contextHubId); /** * Register a callback for the HAL implementation to send asynchronous messages to the service @@ -138,10 +153,11 @@ interface IContextHub { * @param contextHubId The identifier of the Context Hub * @param callback an implementation of the IContextHubCallbacks * - * @return true on success - * + * @throws EX_ILLEGAL_ARGUMENT if any of the arguments are invalid. + * EX_SERVICE_SPECIFIC on error + * - EX_CONTEXT_HUB_UNSPECIFIED if the request failed for other reasons. */ - boolean registerCallback(in int contextHubId, in IContextHubCallback cb); + void registerCallback(in int contextHubId, in IContextHubCallback cb); /** * Sends a message targeted to a nanoapp to the Context Hub. @@ -149,9 +165,11 @@ interface IContextHub { * @param contextHubId The identifier of the Context Hub * @param message The message to be sent * - * @return true on success + * @throws EX_ILLEGAL_ARGUMENT if any of the arguments are invalid. + * EX_SERVICE_SPECIFIC on error + * - EX_CONTEXT_HUB_UNSPECIFIED if the request failed for other reasons. */ - boolean sendMessageToHub(in int contextHubId, in ContextHubMessage message); + void sendMessageToHub(in int contextHubId, in ContextHubMessage message); /** * Invoked when a host endpoint has connected with the ContextHubService. @@ -173,8 +191,13 @@ interface IContextHub { * * @param hostEndPointId The ID of the host that has disconnected. * - * @return Status::ok on success - * EX_ILLEGAL_ARGUMENT if hostEndpointId is not associated with a connected host. + * @throws EX_ILLEGAL_ARGUMENT if hostEndpointId is not associated with a connected host. */ void onHostEndpointDisconnected(char hostEndpointId); + + /** + * Error codes that are used as service specific errors with the AIDL return + * value EX_SERVICE_SPECIFIC. + */ + const int EX_CONTEXT_HUB_UNSPECIFIED = -1; } diff --git a/contexthub/aidl/default/ContextHub.cpp b/contexthub/aidl/default/ContextHub.cpp index 6da690da4f..4c23cbc8bf 100644 --- a/contexthub/aidl/default/ContextHub.cpp +++ b/contexthub/aidl/default/ContextHub.cpp @@ -21,7 +21,9 @@ namespace android { namespace hardware { namespace contexthub { -::ndk::ScopedAStatus ContextHub::getContextHubs(std::vector<ContextHubInfo>* out_contextHubInfos) { +using ::ndk::ScopedAStatus; + +ScopedAStatus ContextHub::getContextHubs(std::vector<ContextHubInfo>* out_contextHubInfos) { ContextHubInfo hub = {}; hub.name = "Mock Context Hub"; hub.vendor = "AOSP"; @@ -39,85 +41,70 @@ namespace contexthub { } // We don't expose any nanoapps for the default impl, therefore all nanoapp-related APIs fail. -::ndk::ScopedAStatus ContextHub::loadNanoapp(int32_t /* in_contextHubId */, - const NanoappBinary& /* in_appBinary */, - int32_t /* in_transactionId */, bool* _aidl_return) { - *_aidl_return = false; - return ndk::ScopedAStatus::ok(); +ScopedAStatus ContextHub::loadNanoapp(int32_t /* in_contextHubId */, + const NanoappBinary& /* in_appBinary */, + int32_t /* in_transactionId */) { + return ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -::ndk::ScopedAStatus ContextHub::unloadNanoapp(int32_t /* in_contextHubId */, - int64_t /* in_appId */, - int32_t /* in_transactionId */, bool* _aidl_return) { - *_aidl_return = false; - return ndk::ScopedAStatus::ok(); +ScopedAStatus ContextHub::unloadNanoapp(int32_t /* in_contextHubId */, int64_t /* in_appId */, + int32_t /* in_transactionId */) { + return ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -::ndk::ScopedAStatus ContextHub::disableNanoapp(int32_t /* in_contextHubId */, - int64_t /* in_appId */, - int32_t /* in_transactionId */, - bool* _aidl_return) { - *_aidl_return = false; - return ndk::ScopedAStatus::ok(); +ScopedAStatus ContextHub::disableNanoapp(int32_t /* in_contextHubId */, int64_t /* in_appId */, + int32_t /* in_transactionId */) { + return ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -::ndk::ScopedAStatus ContextHub::enableNanoapp(int32_t /* in_contextHubId */, - int64_t /* in_appId */, - int32_t /* in_transactionId */, bool* _aidl_return) { - *_aidl_return = false; - return ndk::ScopedAStatus::ok(); +ScopedAStatus ContextHub::enableNanoapp(int32_t /* in_contextHubId */, int64_t /* in_appId */, + int32_t /* in_transactionId */) { + return ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION); } -::ndk::ScopedAStatus ContextHub::onSettingChanged(Setting /* in_setting */, bool /*in_enabled */) { +ScopedAStatus ContextHub::onSettingChanged(Setting /* in_setting */, bool /*in_enabled */) { return ndk::ScopedAStatus::ok(); } -::ndk::ScopedAStatus ContextHub::queryNanoapps(int32_t in_contextHubId, bool* _aidl_return) { +ScopedAStatus ContextHub::queryNanoapps(int32_t in_contextHubId) { if (in_contextHubId == kMockHubId && mCallback != nullptr) { std::vector<NanoappInfo> nanoapps; mCallback->handleNanoappInfo(nanoapps); - *_aidl_return = true; + return ndk::ScopedAStatus::ok(); } else { - *_aidl_return = false; + return ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } - - return ndk::ScopedAStatus::ok(); } -::ndk::ScopedAStatus ContextHub::registerCallback(int32_t in_contextHubId, - const std::shared_ptr<IContextHubCallback>& in_cb, - bool* _aidl_return) { +ScopedAStatus ContextHub::registerCallback(int32_t in_contextHubId, + const std::shared_ptr<IContextHubCallback>& in_cb) { if (in_contextHubId == kMockHubId) { mCallback = in_cb; - *_aidl_return = true; + return ndk::ScopedAStatus::ok(); } else { - *_aidl_return = false; + return ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } - return ndk::ScopedAStatus::ok(); } -::ndk::ScopedAStatus ContextHub::sendMessageToHub(int32_t in_contextHubId, - const ContextHubMessage& /* in_message */, - bool* _aidl_return) { +ScopedAStatus ContextHub::sendMessageToHub(int32_t in_contextHubId, + const ContextHubMessage& /* in_message */) { if (in_contextHubId == kMockHubId) { // Return true here to indicate that the HAL has accepted the message. // Successful delivery of the message to a nanoapp should be handled at // a higher level protocol. - *_aidl_return = true; + return ndk::ScopedAStatus::ok(); } else { - *_aidl_return = false; + return ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT); } - - return ndk::ScopedAStatus::ok(); } -::ndk::ScopedAStatus ContextHub::onHostEndpointConnected(const HostEndpointInfo& in_info) { +ScopedAStatus ContextHub::onHostEndpointConnected(const HostEndpointInfo& in_info) { mConnectedHostEndpoints.insert(in_info.hostEndpointId); return ndk::ScopedAStatus::ok(); } -::ndk::ScopedAStatus ContextHub::onHostEndpointDisconnected(char16_t in_hostEndpointId) { +ScopedAStatus ContextHub::onHostEndpointDisconnected(char16_t in_hostEndpointId) { if (mConnectedHostEndpoints.count(in_hostEndpointId) > 0) { mConnectedHostEndpoints.erase(in_hostEndpointId); return ndk::ScopedAStatus::ok(); diff --git a/contexthub/aidl/default/include/contexthub-impl/ContextHub.h b/contexthub/aidl/default/include/contexthub-impl/ContextHub.h index dd739e63f8..03d8432134 100644 --- a/contexthub/aidl/default/include/contexthub-impl/ContextHub.h +++ b/contexthub/aidl/default/include/contexthub-impl/ContextHub.h @@ -28,21 +28,19 @@ namespace contexthub { class ContextHub : public BnContextHub { ::ndk::ScopedAStatus getContextHubs(std::vector<ContextHubInfo>* out_contextHubInfos) override; ::ndk::ScopedAStatus loadNanoapp(int32_t in_contextHubId, const NanoappBinary& in_appBinary, - int32_t in_transactionId, bool* _aidl_return) override; + int32_t in_transactionId) override; ::ndk::ScopedAStatus unloadNanoapp(int32_t in_contextHubId, int64_t in_appId, - int32_t in_transactionId, bool* _aidl_return) override; + int32_t in_transactionId) override; ::ndk::ScopedAStatus disableNanoapp(int32_t in_contextHubId, int64_t in_appId, - int32_t in_transactionId, bool* _aidl_return) override; + int32_t in_transactionId) override; ::ndk::ScopedAStatus enableNanoapp(int32_t in_contextHubId, int64_t in_appId, - int32_t in_transactionId, bool* _aidl_return) override; + int32_t in_transactionId) override; ::ndk::ScopedAStatus onSettingChanged(Setting in_setting, bool in_enabled) override; - ::ndk::ScopedAStatus queryNanoapps(int32_t in_contextHubId, bool* _aidl_return) override; - ::ndk::ScopedAStatus registerCallback(int32_t in_contextHubId, - const std::shared_ptr<IContextHubCallback>& in_cb, - bool* _aidl_return) override; + ::ndk::ScopedAStatus queryNanoapps(int32_t in_contextHubId) override; + ::ndk::ScopedAStatus registerCallback( + int32_t in_contextHubId, const std::shared_ptr<IContextHubCallback>& in_cb) override; ::ndk::ScopedAStatus sendMessageToHub(int32_t in_contextHubId, - const ContextHubMessage& in_message, - bool* _aidl_return) override; + const ContextHubMessage& in_message) override; ::ndk::ScopedAStatus onHostEndpointConnected(const HostEndpointInfo& in_info) override; ::ndk::ScopedAStatus onHostEndpointDisconnected(char16_t in_hostEndpointId) override; diff --git a/contexthub/aidl/vts/VtsAidlHalContextHubTargetTest.cpp b/contexthub/aidl/vts/VtsAidlHalContextHubTargetTest.cpp index 392e23c9c1..a47f64e5be 100644 --- a/contexthub/aidl/vts/VtsAidlHalContextHubTargetTest.cpp +++ b/contexthub/aidl/vts/VtsAidlHalContextHubTargetTest.cpp @@ -103,15 +103,12 @@ class EmptyContextHubCallback : public android::hardware::contexthub::BnContextH }; TEST_P(ContextHubAidl, TestRegisterCallback) { - bool success; sp<EmptyContextHubCallback> cb = sp<EmptyContextHubCallback>::make(); - ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb, &success).isOk()); - ASSERT_TRUE(success); + ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb).isOk()); } TEST_P(ContextHubAidl, TestRegisterNullCallback) { - bool success; - ASSERT_TRUE(contextHub->registerCallback(getHubId(), nullptr, &success).isOk()); + ASSERT_TRUE(contextHub->registerCallback(getHubId(), nullptr).isOk()); } // Helper callback that puts the async appInfo callback data into a promise @@ -140,12 +137,8 @@ class QueryAppsCallback : public android::hardware::contexthub::BnContextHubCall // Calls queryApps() and checks the returned metadata TEST_P(ContextHubAidl, TestQueryApps) { sp<QueryAppsCallback> cb = sp<QueryAppsCallback>::make(); - bool success; - ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb, &success).isOk()); - ASSERT_TRUE(success); - - ASSERT_TRUE(contextHub->queryNanoapps(getHubId(), &success).isOk()); - ASSERT_TRUE(success); + ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb).isOk()); + ASSERT_TRUE(contextHub->queryNanoapps(getHubId()).isOk()); std::vector<NanoappInfo> appInfoList; ASSERT_TRUE(waitForCallback(cb->promise.get_future(), &appInfoList)); @@ -197,9 +190,7 @@ class ContextHubTransactionTest : public ContextHubAidl { public: virtual void SetUp() override { ContextHubAidl::SetUp(); - bool success; - ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb, &success).isOk()); - ASSERT_TRUE(success); + ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb).isOk()); } sp<TransactionResultCallback> cb = sp<TransactionResultCallback>::make(); @@ -213,9 +204,7 @@ TEST_P(ContextHubTransactionTest, TestSendMessageToNonExistentNanoapp) { std::fill(message.messageBody.begin(), message.messageBody.end(), 0); ALOGD("Sending message to non-existent nanoapp"); - bool success; - ASSERT_TRUE(contextHub->sendMessageToHub(getHubId(), message, &success).isOk()); - ASSERT_TRUE(success); + ASSERT_TRUE(contextHub->sendMessageToHub(getHubId(), message).isOk()); } TEST_P(ContextHubTransactionTest, TestLoadEmptyNanoapp) { @@ -229,9 +218,7 @@ TEST_P(ContextHubTransactionTest, TestLoadEmptyNanoapp) { emptyApp.targetChreApiMinorVersion = 0; ALOGD("Loading empty nanoapp"); - bool success; - ASSERT_TRUE(contextHub->loadNanoapp(getHubId(), emptyApp, cb->expectedTransactionId, &success) - .isOk()); + bool success = contextHub->loadNanoapp(getHubId(), emptyApp, cb->expectedTransactionId).isOk(); if (success) { bool transactionSuccess; ASSERT_TRUE(waitForCallback(cb->promise.get_future(), &transactionSuccess)); @@ -243,11 +230,9 @@ TEST_P(ContextHubTransactionTest, TestUnloadNonexistentNanoapp) { cb->expectedTransactionId = 1234; ALOGD("Unloading nonexistent nanoapp"); - bool success; - ASSERT_TRUE(contextHub - ->unloadNanoapp(getHubId(), kNonExistentAppId, cb->expectedTransactionId, - &success) - .isOk()); + bool success = + contextHub->unloadNanoapp(getHubId(), kNonExistentAppId, cb->expectedTransactionId) + .isOk(); if (success) { bool transactionSuccess; ASSERT_TRUE(waitForCallback(cb->promise.get_future(), &transactionSuccess)); @@ -259,11 +244,9 @@ TEST_P(ContextHubTransactionTest, TestEnableNonexistentNanoapp) { cb->expectedTransactionId = 2345; ALOGD("Enabling nonexistent nanoapp"); - bool success; - ASSERT_TRUE(contextHub - ->enableNanoapp(getHubId(), kNonExistentAppId, cb->expectedTransactionId, - &success) - .isOk()); + bool success = + contextHub->enableNanoapp(getHubId(), kNonExistentAppId, cb->expectedTransactionId) + .isOk(); if (success) { bool transactionSuccess; ASSERT_TRUE(waitForCallback(cb->promise.get_future(), &transactionSuccess)); @@ -275,11 +258,9 @@ TEST_P(ContextHubTransactionTest, TestDisableNonexistentNanoapp) { cb->expectedTransactionId = 3456; ALOGD("Disabling nonexistent nanoapp"); - bool success; - ASSERT_TRUE(contextHub - ->disableNanoapp(getHubId(), kNonExistentAppId, cb->expectedTransactionId, - &success) - .isOk()); + bool success = + contextHub->disableNanoapp(getHubId(), kNonExistentAppId, cb->expectedTransactionId) + .isOk(); if (success) { bool transactionSuccess; ASSERT_TRUE(waitForCallback(cb->promise.get_future(), &transactionSuccess)); @@ -290,16 +271,13 @@ TEST_P(ContextHubTransactionTest, TestDisableNonexistentNanoapp) { void ContextHubAidl::testSettingChanged(Setting setting) { // In VTS, we only test that sending the values doesn't cause things to blow up - GTS tests // verify the expected E2E behavior in CHRE - bool success; sp<EmptyContextHubCallback> cb = sp<EmptyContextHubCallback>::make(); - ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb, &success).isOk()); - ASSERT_TRUE(success); + ASSERT_TRUE(contextHub->registerCallback(getHubId(), cb).isOk()); ASSERT_TRUE(contextHub->onSettingChanged(setting, true /* enabled */).isOk()); ASSERT_TRUE(contextHub->onSettingChanged(setting, false /* enabled */).isOk()); - ASSERT_TRUE(contextHub->registerCallback(getHubId(), nullptr, &success).isOk()); - ASSERT_TRUE(success); + ASSERT_TRUE(contextHub->registerCallback(getHubId(), nullptr).isOk()); } TEST_P(ContextHubAidl, TestOnLocationSettingChanged) { |