diff options
author | 2024-02-16 17:49:36 +0000 | |
---|---|---|
committer | 2024-02-16 18:26:13 +0000 | |
commit | 641248efeb103c47a1fd170fbf8bba7d35673151 (patch) | |
tree | 036eeba49446ecafaf8b688b3a538d613d2b2c7e /services/vibratorservice | |
parent | f8680e6d01b64ea8745b8c0aa335e6a4b54d8958 (diff) |
Fix retry logic for vibrator HAL requests
Update VibratorHalController to only reconnect with the IVibrator HAL
service and retry the operation if a transaction error or dead object
code is received.
This will avoid retrying vibrator calls with bad values when the error
code is illegal argument, for example, or when the vibrator is
returning illegal state.
Bug: 325204954
Test: atest libvibratorservice_test
Change-Id: I6764c736a64745ada5233329372d5c22f4e8aa74
Diffstat (limited to 'services/vibratorservice')
8 files changed, 345 insertions, 261 deletions
diff --git a/services/vibratorservice/VibratorHalWrapper.cpp b/services/vibratorservice/VibratorHalWrapper.cpp index 63ecaec06f..f10ba44d74 100644 --- a/services/vibratorservice/VibratorHalWrapper.cpp +++ b/services/vibratorservice/VibratorHalWrapper.cpp @@ -56,75 +56,6 @@ bool isStaticCastValid(Effect effect) { // ------------------------------------------------------------------------------------------------- -const constexpr char* STATUS_T_ERROR_MESSAGE_PREFIX = "status_t = "; -const constexpr char* STATUS_V_1_0_ERROR_MESSAGE_PREFIX = - "android::hardware::vibrator::V1_0::Status = "; - -template <typename T> -HalResult<T> HalResult<T>::fromStatus(V1_0::Status status, T data) { - switch (status) { - case V1_0::Status::OK: - return HalResult<T>::ok(data); - case V1_0::Status::UNSUPPORTED_OPERATION: - return HalResult<T>::unsupported(); - default: - return HalResult<T>::failed(STATUS_V_1_0_ERROR_MESSAGE_PREFIX + toString(status)); - } -} - -template <typename T> -template <typename R> -HalResult<T> HalResult<T>::fromReturn(hardware::Return<R>& ret, T data) { - return ret.isOk() ? HalResult<T>::ok(data) : HalResult<T>::failed(ret.description()); -} - -template <typename T> -template <typename R> -HalResult<T> HalResult<T>::fromReturn(hardware::Return<R>& ret, V1_0::Status status, T data) { - return ret.isOk() ? HalResult<T>::fromStatus(status, data) - : HalResult<T>::failed(ret.description()); -} - -// ------------------------------------------------------------------------------------------------- - -HalResult<void> HalResult<void>::fromStatus(status_t status) { - if (status == android::OK) { - return HalResult<void>::ok(); - } - return HalResult<void>::failed(STATUS_T_ERROR_MESSAGE_PREFIX + statusToString(status)); -} - -HalResult<void> HalResult<void>::fromStatus(binder::Status status) { - if (status.exceptionCode() == binder::Status::EX_UNSUPPORTED_OPERATION || - status.transactionError() == android::UNKNOWN_TRANSACTION) { - // UNKNOWN_TRANSACTION means the HAL implementation is an older version, so this is - // the same as the operation being unsupported by this HAL. Should not retry. - return HalResult<void>::unsupported(); - } - if (status.isOk()) { - return HalResult<void>::ok(); - } - return HalResult<void>::failed(std::string(status.toString8().c_str())); -} - -HalResult<void> HalResult<void>::fromStatus(V1_0::Status status) { - switch (status) { - case V1_0::Status::OK: - return HalResult<void>::ok(); - case V1_0::Status::UNSUPPORTED_OPERATION: - return HalResult<void>::unsupported(); - default: - return HalResult<void>::failed(STATUS_V_1_0_ERROR_MESSAGE_PREFIX + toString(status)); - } -} - -template <typename R> -HalResult<void> HalResult<void>::fromReturn(hardware::Return<R>& ret) { - return ret.isOk() ? HalResult<void>::ok() : HalResult<void>::failed(ret.description()); -} - -// ------------------------------------------------------------------------------------------------- - Info HalWrapper::getInfo() { getCapabilities(); getPrimitiveDurations(); @@ -269,7 +200,7 @@ HalResult<std::vector<float>> HalWrapper::getMaxAmplitudesInternal() { // ------------------------------------------------------------------------------------------------- HalResult<void> AidlHalWrapper::ping() { - return HalResult<void>::fromStatus(IInterface::asBinder(getHal())->pingBinder()); + return HalResultFactory::fromStatus(IInterface::asBinder(getHal())->pingBinder()); } void AidlHalWrapper::tryReconnect() { @@ -291,7 +222,7 @@ HalResult<void> AidlHalWrapper::on(milliseconds timeout, static_cast<int32_t>(capabilities.value() & Capabilities::ON_CALLBACK); auto cb = supportsCallback ? new HalCallbackWrapper(completionCallback) : nullptr; - auto ret = HalResult<void>::fromStatus(getHal()->on(timeout.count(), cb)); + auto ret = HalResultFactory::fromStatus(getHal()->on(timeout.count(), cb)); if (!supportsCallback && ret.isOk()) { mCallbackScheduler->schedule(completionCallback, timeout); } @@ -300,23 +231,23 @@ HalResult<void> AidlHalWrapper::on(milliseconds timeout, } HalResult<void> AidlHalWrapper::off() { - return HalResult<void>::fromStatus(getHal()->off()); + return HalResultFactory::fromStatus(getHal()->off()); } HalResult<void> AidlHalWrapper::setAmplitude(float amplitude) { - return HalResult<void>::fromStatus(getHal()->setAmplitude(amplitude)); + return HalResultFactory::fromStatus(getHal()->setAmplitude(amplitude)); } HalResult<void> AidlHalWrapper::setExternalControl(bool enabled) { - return HalResult<void>::fromStatus(getHal()->setExternalControl(enabled)); + return HalResultFactory::fromStatus(getHal()->setExternalControl(enabled)); } HalResult<void> AidlHalWrapper::alwaysOnEnable(int32_t id, Effect effect, EffectStrength strength) { - return HalResult<void>::fromStatus(getHal()->alwaysOnEnable(id, effect, strength)); + return HalResultFactory::fromStatus(getHal()->alwaysOnEnable(id, effect, strength)); } HalResult<void> AidlHalWrapper::alwaysOnDisable(int32_t id) { - return HalResult<void>::fromStatus(getHal()->alwaysOnDisable(id)); + return HalResultFactory::fromStatus(getHal()->alwaysOnDisable(id)); } HalResult<milliseconds> AidlHalWrapper::performEffect( @@ -330,7 +261,7 @@ HalResult<milliseconds> AidlHalWrapper::performEffect( auto result = getHal()->perform(effect, strength, cb, &lengthMs); milliseconds length = milliseconds(lengthMs); - auto ret = HalResult<milliseconds>::fromStatus(result, length); + auto ret = HalResultFactory::fromStatus<milliseconds>(result, length); if (!supportsCallback && ret.isOk()) { mCallbackScheduler->schedule(completionCallback, length); } @@ -357,38 +288,40 @@ HalResult<milliseconds> AidlHalWrapper::performComposedEffect( duration += milliseconds(effect.delayMs); } - return HalResult<milliseconds>::fromStatus(getHal()->compose(primitives, cb), duration); + return HalResultFactory::fromStatus<milliseconds>(getHal()->compose(primitives, cb), duration); } HalResult<void> AidlHalWrapper::performPwleEffect(const std::vector<PrimitivePwle>& primitives, const std::function<void()>& completionCallback) { // This method should always support callbacks, so no need to double check. auto cb = new HalCallbackWrapper(completionCallback); - return HalResult<void>::fromStatus(getHal()->composePwle(primitives, cb)); + return HalResultFactory::fromStatus(getHal()->composePwle(primitives, cb)); } HalResult<Capabilities> AidlHalWrapper::getCapabilitiesInternal() { int32_t capabilities = 0; auto result = getHal()->getCapabilities(&capabilities); - return HalResult<Capabilities>::fromStatus(result, static_cast<Capabilities>(capabilities)); + return HalResultFactory::fromStatus<Capabilities>(result, + static_cast<Capabilities>(capabilities)); } HalResult<std::vector<Effect>> AidlHalWrapper::getSupportedEffectsInternal() { std::vector<Effect> supportedEffects; auto result = getHal()->getSupportedEffects(&supportedEffects); - return HalResult<std::vector<Effect>>::fromStatus(result, supportedEffects); + return HalResultFactory::fromStatus<std::vector<Effect>>(result, supportedEffects); } HalResult<std::vector<Braking>> AidlHalWrapper::getSupportedBrakingInternal() { std::vector<Braking> supportedBraking; auto result = getHal()->getSupportedBraking(&supportedBraking); - return HalResult<std::vector<Braking>>::fromStatus(result, supportedBraking); + return HalResultFactory::fromStatus<std::vector<Braking>>(result, supportedBraking); } HalResult<std::vector<CompositePrimitive>> AidlHalWrapper::getSupportedPrimitivesInternal() { std::vector<CompositePrimitive> supportedPrimitives; auto result = getHal()->getSupportedPrimitives(&supportedPrimitives); - return HalResult<std::vector<CompositePrimitive>>::fromStatus(result, supportedPrimitives); + return HalResultFactory::fromStatus<std::vector<CompositePrimitive>>(result, + supportedPrimitives); } HalResult<std::vector<milliseconds>> AidlHalWrapper::getPrimitiveDurationsInternal( @@ -408,7 +341,7 @@ HalResult<std::vector<milliseconds>> AidlHalWrapper::getPrimitiveDurationsIntern } int32_t duration = 0; auto result = getHal()->getPrimitiveDuration(primitive, &duration); - auto halResult = HalResult<int32_t>::fromStatus(result, duration); + auto halResult = HalResultFactory::fromStatus<int32_t>(result, duration); if (halResult.isUnsupported()) { // Should not happen, supported primitives should always support requesting duration. ALOGE("Supported primitive %zu returned unsupported for getPrimitiveDuration", @@ -427,55 +360,55 @@ HalResult<std::vector<milliseconds>> AidlHalWrapper::getPrimitiveDurationsIntern HalResult<milliseconds> AidlHalWrapper::getPrimitiveDelayMaxInternal() { int32_t delay = 0; auto result = getHal()->getCompositionDelayMax(&delay); - return HalResult<milliseconds>::fromStatus(result, milliseconds(delay)); + return HalResultFactory::fromStatus<milliseconds>(result, milliseconds(delay)); } HalResult<milliseconds> AidlHalWrapper::getPrimitiveDurationMaxInternal() { int32_t delay = 0; auto result = getHal()->getPwlePrimitiveDurationMax(&delay); - return HalResult<milliseconds>::fromStatus(result, milliseconds(delay)); + return HalResultFactory::fromStatus<milliseconds>(result, milliseconds(delay)); } HalResult<int32_t> AidlHalWrapper::getCompositionSizeMaxInternal() { int32_t size = 0; auto result = getHal()->getCompositionSizeMax(&size); - return HalResult<int32_t>::fromStatus(result, size); + return HalResultFactory::fromStatus<int32_t>(result, size); } HalResult<int32_t> AidlHalWrapper::getPwleSizeMaxInternal() { int32_t size = 0; auto result = getHal()->getPwleCompositionSizeMax(&size); - return HalResult<int32_t>::fromStatus(result, size); + return HalResultFactory::fromStatus<int32_t>(result, size); } HalResult<float> AidlHalWrapper::getMinFrequencyInternal() { float minFrequency = 0; auto result = getHal()->getFrequencyMinimum(&minFrequency); - return HalResult<float>::fromStatus(result, minFrequency); + return HalResultFactory::fromStatus<float>(result, minFrequency); } HalResult<float> AidlHalWrapper::getResonantFrequencyInternal() { float f0 = 0; auto result = getHal()->getResonantFrequency(&f0); - return HalResult<float>::fromStatus(result, f0); + return HalResultFactory::fromStatus<float>(result, f0); } HalResult<float> AidlHalWrapper::getFrequencyResolutionInternal() { float frequencyResolution = 0; auto result = getHal()->getFrequencyResolution(&frequencyResolution); - return HalResult<float>::fromStatus(result, frequencyResolution); + return HalResultFactory::fromStatus<float>(result, frequencyResolution); } HalResult<float> AidlHalWrapper::getQFactorInternal() { float qFactor = 0; auto result = getHal()->getQFactor(&qFactor); - return HalResult<float>::fromStatus(result, qFactor); + return HalResultFactory::fromStatus<float>(result, qFactor); } HalResult<std::vector<float>> AidlHalWrapper::getMaxAmplitudesInternal() { std::vector<float> amplitudes; auto result = getHal()->getBandwidthAmplitudeMap(&litudes); - return HalResult<std::vector<float>>::fromStatus(result, amplitudes); + return HalResultFactory::fromStatus<std::vector<float>>(result, amplitudes); } sp<Aidl::IVibrator> AidlHalWrapper::getHal() { @@ -488,7 +421,7 @@ sp<Aidl::IVibrator> AidlHalWrapper::getHal() { template <typename I> HalResult<void> HidlHalWrapper<I>::ping() { auto result = getHal()->ping(); - return HalResult<void>::fromReturn(result); + return HalResultFactory::fromReturn(result); } template <typename I> @@ -504,7 +437,7 @@ template <typename I> HalResult<void> HidlHalWrapper<I>::on(milliseconds timeout, const std::function<void()>& completionCallback) { auto result = getHal()->on(timeout.count()); - auto ret = HalResult<void>::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); + auto ret = HalResultFactory::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); if (ret.isOk()) { mCallbackScheduler->schedule(completionCallback, timeout); } @@ -514,14 +447,14 @@ HalResult<void> HidlHalWrapper<I>::on(milliseconds timeout, template <typename I> HalResult<void> HidlHalWrapper<I>::off() { auto result = getHal()->off(); - return HalResult<void>::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); + return HalResultFactory::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); } template <typename I> HalResult<void> HidlHalWrapper<I>::setAmplitude(float amplitude) { uint8_t amp = static_cast<uint8_t>(amplitude * std::numeric_limits<uint8_t>::max()); auto result = getHal()->setAmplitude(amp); - return HalResult<void>::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); + return HalResultFactory::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); } template <typename I> @@ -547,7 +480,7 @@ HalResult<Capabilities> HidlHalWrapper<I>::getCapabilitiesInternal() { hardware::Return<bool> result = getHal()->supportsAmplitudeControl(); Capabilities capabilities = result.withDefault(false) ? Capabilities::AMPLITUDE_CONTROL : Capabilities::NONE; - return HalResult<Capabilities>::fromReturn(result, capabilities); + return HalResultFactory::fromReturn<Capabilities>(result, capabilities); } template <typename I> @@ -566,7 +499,7 @@ HalResult<milliseconds> HidlHalWrapper<I>::performInternal( auto result = std::invoke(performFn, handle, effect, effectStrength, effectCallback); milliseconds length = milliseconds(lengthMs); - auto ret = HalResult<milliseconds>::fromReturn(result, status, length); + auto ret = HalResultFactory::fromReturn<milliseconds>(result, status, length); if (ret.isOk()) { mCallbackScheduler->schedule(completionCallback, length); } @@ -638,7 +571,7 @@ HalResult<milliseconds> HidlHalWrapperV1_2::performEffect( HalResult<void> HidlHalWrapperV1_3::setExternalControl(bool enabled) { auto result = getHal()->setExternalControl(static_cast<uint32_t>(enabled)); - return HalResult<void>::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); + return HalResultFactory::fromStatus(result.withDefault(V1_0::Status::UNKNOWN_ERROR)); } HalResult<milliseconds> HidlHalWrapperV1_3::performEffect( @@ -671,7 +604,7 @@ HalResult<Capabilities> HidlHalWrapperV1_3::getCapabilitiesInternal() { sp<V1_3::IVibrator> hal = getHal(); auto amplitudeResult = hal->supportsAmplitudeControl(); if (!amplitudeResult.isOk()) { - return HalResult<Capabilities>::fromReturn(amplitudeResult, capabilities); + return HalResultFactory::fromReturn<Capabilities>(amplitudeResult, capabilities); } auto externalControlResult = hal->supportsExternalControl(); @@ -686,7 +619,7 @@ HalResult<Capabilities> HidlHalWrapperV1_3::getCapabilitiesInternal() { } } - return HalResult<Capabilities>::fromReturn(externalControlResult, capabilities); + return HalResultFactory::fromReturn<Capabilities>(externalControlResult, capabilities); } // ------------------------------------------------------------------------------------------------- diff --git a/services/vibratorservice/VibratorManagerHalController.cpp b/services/vibratorservice/VibratorManagerHalController.cpp index 0df0bfa19a..aa5b7fc86f 100644 --- a/services/vibratorservice/VibratorManagerHalController.cpp +++ b/services/vibratorservice/VibratorManagerHalController.cpp @@ -46,8 +46,6 @@ template <typename T> HalResult<T> ManagerHalController::processHalResult(HalResult<T> result, const char* functionName) { if (result.isFailed()) { ALOGE("VibratorManager HAL %s failed: %s", functionName, result.errorMessage()); - std::lock_guard<std::mutex> lock(mConnectedHalMutex); - mConnectedHal->tryReconnect(); } return result; } @@ -70,12 +68,16 @@ HalResult<T> ManagerHalController::apply(ManagerHalController::hal_fn<T>& halFn, hal = mConnectedHal; } - HalResult<T> ret = processHalResult(halFn(hal), functionName); - for (int i = 0; i < MAX_RETRIES && ret.isFailed(); i++) { - ret = processHalResult(halFn(hal), functionName); + HalResult<T> result = processHalResult(halFn(hal), functionName); + for (int i = 0; i < MAX_RETRIES && result.shouldRetry(); i++) { + { + std::lock_guard<std::mutex> lock(mConnectedHalMutex); + mConnectedHal->tryReconnect(); + } + result = processHalResult(halFn(hal), functionName); } - return ret; + return result; } // ------------------------------------------------------------------------------------------------- diff --git a/services/vibratorservice/VibratorManagerHalWrapper.cpp b/services/vibratorservice/VibratorManagerHalWrapper.cpp index 6e660e7e41..13412667e0 100644 --- a/services/vibratorservice/VibratorManagerHalWrapper.cpp +++ b/services/vibratorservice/VibratorManagerHalWrapper.cpp @@ -55,8 +55,8 @@ HalResult<std::shared_ptr<HalController>> LegacyManagerHalWrapper::getVibrator(i return HalResult<std::shared_ptr<HalController>>::ok(mController); } // Controller.init did not connect to any vibrator HAL service, so the device has no vibrator. - return HalResult<std::shared_ptr<HalController>>::failed(MISSING_VIBRATOR_MESSAGE_PREFIX + - std::to_string(id)); + return HalResult<std::shared_ptr<HalController>>::failed( + (MISSING_VIBRATOR_MESSAGE_PREFIX + std::to_string(id)).c_str()); } HalResult<void> LegacyManagerHalWrapper::prepareSynced(const std::vector<int32_t>&) { @@ -75,10 +75,10 @@ HalResult<void> LegacyManagerHalWrapper::cancelSynced() { std::shared_ptr<HalWrapper> AidlManagerHalWrapper::connectToVibrator( int32_t vibratorId, std::shared_ptr<CallbackScheduler> callbackScheduler) { - std::function<HalResult<sp<Aidl::IVibrator>>()> reconnectFn = [=]() { + std::function<HalResult<sp<Aidl::IVibrator>>()> reconnectFn = [=, this]() { sp<Aidl::IVibrator> vibrator; auto result = this->getHal()->getVibrator(vibratorId, &vibrator); - return HalResult<sp<Aidl::IVibrator>>::fromStatus(result, vibrator); + return HalResultFactory::fromStatus<sp<Aidl::IVibrator>>(result, vibrator); }; auto result = reconnectFn(); if (!result.isOk()) { @@ -88,12 +88,12 @@ std::shared_ptr<HalWrapper> AidlManagerHalWrapper::connectToVibrator( if (!vibrator) { return nullptr; } - return std::move(std::make_unique<AidlHalWrapper>(std::move(callbackScheduler), - std::move(vibrator), reconnectFn)); + return std::make_unique<AidlHalWrapper>(std::move(callbackScheduler), std::move(vibrator), + reconnectFn); } HalResult<void> AidlManagerHalWrapper::ping() { - return HalResult<void>::fromStatus(IInterface::asBinder(getHal())->pingBinder()); + return HalResultFactory::fromStatus(IInterface::asBinder(getHal())->pingBinder()); } void AidlManagerHalWrapper::tryReconnect() { @@ -112,8 +112,8 @@ HalResult<ManagerCapabilities> AidlManagerHalWrapper::getCapabilities() { } int32_t cap = 0; auto result = getHal()->getCapabilities(&cap); - auto ret = HalResult<ManagerCapabilities>::fromStatus(result, - static_cast<ManagerCapabilities>(cap)); + auto capabilities = static_cast<ManagerCapabilities>(cap); + auto ret = HalResultFactory::fromStatus<ManagerCapabilities>(result, capabilities); if (ret.isOk()) { // Cache copy of returned value. mCapabilities.emplace(ret.value()); @@ -129,7 +129,7 @@ HalResult<std::vector<int32_t>> AidlManagerHalWrapper::getVibratorIds() { } std::vector<int32_t> ids; auto result = getHal()->getVibratorIds(&ids); - auto ret = HalResult<std::vector<int32_t>>::fromStatus(result, ids); + auto ret = HalResultFactory::fromStatus<std::vector<int32_t>>(result, ids); if (ret.isOk()) { // Cache copy of returned value and the individual controllers. mVibratorIds.emplace(ret.value()); @@ -152,12 +152,12 @@ HalResult<std::shared_ptr<HalController>> AidlManagerHalWrapper::getVibrator(int if (it != mVibrators.end()) { return HalResult<std::shared_ptr<HalController>>::ok(it->second); } - return HalResult<std::shared_ptr<HalController>>::failed(MISSING_VIBRATOR_MESSAGE_PREFIX + - std::to_string(id)); + return HalResult<std::shared_ptr<HalController>>::failed( + (MISSING_VIBRATOR_MESSAGE_PREFIX + std::to_string(id)).c_str()); } HalResult<void> AidlManagerHalWrapper::prepareSynced(const std::vector<int32_t>& ids) { - auto ret = HalResult<void>::fromStatus(getHal()->prepareSynced(ids)); + auto ret = HalResultFactory::fromStatus(getHal()->prepareSynced(ids)); if (ret.isOk()) { // Force reload of all vibrator controllers that were prepared for a sync operation here. // This will trigger calls to getVibrator(id) on each controller, so they can use the @@ -179,11 +179,11 @@ HalResult<void> AidlManagerHalWrapper::triggerSynced( bool supportsCallback = capabilities.isOk() && static_cast<int32_t>(capabilities.value() & ManagerCapabilities::TRIGGER_CALLBACK); auto cb = supportsCallback ? new HalCallbackWrapper(completionCallback) : nullptr; - return HalResult<void>::fromStatus(getHal()->triggerSynced(cb)); + return HalResultFactory::fromStatus(getHal()->triggerSynced(cb)); } HalResult<void> AidlManagerHalWrapper::cancelSynced() { - auto ret = HalResult<void>::fromStatus(getHal()->cancelSynced()); + auto ret = HalResultFactory::fromStatus(getHal()->cancelSynced()); if (ret.isOk()) { // Force reload of all vibrator controllers that were prepared for a sync operation before. // This will trigger calls to getVibrator(id) on each controller, so they can use the diff --git a/services/vibratorservice/include/vibratorservice/VibratorHalController.h b/services/vibratorservice/include/vibratorservice/VibratorHalController.h index 6b73d17b0a..f97442ddee 100644 --- a/services/vibratorservice/include/vibratorservice/VibratorHalController.h +++ b/services/vibratorservice/include/vibratorservice/VibratorHalController.h @@ -64,7 +64,29 @@ public: */ Info getInfo() { static Info sDefaultInfo = InfoCache().get(); - return apply<Info>([](HalWrapper* hal) { return hal->getInfo(); }, sDefaultInfo, "getInfo"); + if (!init()) { + ALOGV("Skipped getInfo because Vibrator HAL is not available"); + return sDefaultInfo; + } + std::shared_ptr<HalWrapper> hal; + { + std::lock_guard<std::mutex> lock(mConnectedHalMutex); + hal = mConnectedHal; + } + + for (int i = 0; i < MAX_RETRIES; i++) { + Info result = hal.get()->getInfo(); + result.logFailures(); + if (result.shouldRetry()) { + tryReconnect(); + } else { + return result; + } + } + + Info result = hal.get()->getInfo(); + result.logFailures(); + return result; } /* Calls given HAL function, applying automatic retries to reconnect with the HAL when the @@ -72,7 +94,7 @@ public: */ template <typename T> HalResult<T> doWithRetry(const HalFunction<HalResult<T>>& halFn, const char* functionName) { - return apply(halFn, HalResult<T>::unsupported(), functionName); + return doWithRetry<T>(halFn, HalResult<T>::unsupported(), functionName); } private: @@ -90,7 +112,8 @@ private: * function name is for logging purposes. */ template <typename T> - T apply(const HalFunction<T>& halFn, T defaultValue, const char* functionName) { + HalResult<T> doWithRetry(const HalFunction<HalResult<T>>& halFn, HalResult<T> defaultValue, + const char* functionName) { if (!init()) { ALOGV("Skipped %s because Vibrator HAL is not available", functionName); return defaultValue; @@ -101,16 +124,22 @@ private: hal = mConnectedHal; } - for (int i = 0; i < MAX_RETRIES; i++) { - T result = halFn(hal.get()); - if (result.isFailedLogged(functionName)) { - tryReconnect(); - } else { - return result; - } + HalResult<T> result = doOnce(hal.get(), halFn, functionName); + for (int i = 0; i < MAX_RETRIES && result.shouldRetry(); i++) { + tryReconnect(); + result = doOnce(hal.get(), halFn, functionName); } + return result; + } - return halFn(hal.get()); + template <typename T> + HalResult<T> doOnce(HalWrapper* hal, const HalFunction<HalResult<T>>& halFn, + const char* functionName) { + HalResult<T> result = halFn(hal); + if (result.isFailed()) { + ALOGE("Vibrator HAL %s failed: %s", functionName, result.errorMessage()); + } + return result; } }; diff --git a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h index d2cc9ad7df..39c4eb441e 100644 --- a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h +++ b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h @@ -31,101 +31,154 @@ namespace vibrator { // ------------------------------------------------------------------------------------------------- +// Base class to represent a generic result of a call to the Vibrator HAL wrapper. +class BaseHalResult { +public: + bool isOk() const { return mStatus == SUCCESS; } + bool isFailed() const { return mStatus == FAILED; } + bool isUnsupported() const { return mStatus == UNSUPPORTED; } + bool shouldRetry() const { return isFailed() && mDeadObject; } + const char* errorMessage() const { return mErrorMessage.c_str(); } + +protected: + enum Status { SUCCESS, UNSUPPORTED, FAILED }; + Status mStatus; + std::string mErrorMessage; + bool mDeadObject; + + explicit BaseHalResult(Status status, const char* errorMessage = "", bool deadObject = false) + : mStatus(status), mErrorMessage(errorMessage), mDeadObject(deadObject) {} + virtual ~BaseHalResult() = default; +}; + // Result of a call to the Vibrator HAL wrapper, holding data if successful. template <typename T> -class HalResult { +class HalResult : public BaseHalResult { public: static HalResult<T> ok(T value) { return HalResult(value); } - static HalResult<T> failed(std::string msg) { - return HalResult(std::move(msg), /* unsupported= */ false); - } - static HalResult<T> unsupported() { return HalResult("", /* unsupported= */ true); } - - static HalResult<T> fromStatus(binder::Status status, T data) { - if (status.exceptionCode() == binder::Status::EX_UNSUPPORTED_OPERATION || - status.transactionError() == android::UNKNOWN_TRANSACTION) { - // UNKNOWN_TRANSACTION means the HAL implementation is an older version, so this is - // the same as the operation being unsupported by this HAL. Should not retry. - return HalResult<T>::unsupported(); - } - if (status.isOk()) { - return HalResult<T>::ok(data); - } - return HalResult<T>::failed(status.toString8().c_str()); + static HalResult<T> unsupported() { return HalResult(Status::UNSUPPORTED); } + static HalResult<T> failed(const char* msg) { return HalResult(Status::FAILED, msg); } + static HalResult<T> transactionFailed(const char* msg) { + return HalResult(Status::FAILED, msg, /* deadObject= */ true); } - static HalResult<T> fromStatus(hardware::vibrator::V1_0::Status status, T data); - - template <typename R> - static HalResult<T> fromReturn(hardware::Return<R>& ret, T data); - - template <typename R> - static HalResult<T> fromReturn(hardware::Return<R>& ret, - hardware::vibrator::V1_0::Status status, T data); // This will throw std::bad_optional_access if this result is not ok. const T& value() const { return mValue.value(); } const T valueOr(T&& defaultValue) const { return mValue.value_or(defaultValue); } - bool isOk() const { return !mUnsupported && mValue.has_value(); } - bool isFailed() const { return !mUnsupported && !mValue.has_value(); } - bool isUnsupported() const { return mUnsupported; } - const char* errorMessage() const { return mErrorMessage.c_str(); } - bool isFailedLogged(const char* functionNameForLogging) const { - if (isFailed()) { - ALOGE("Vibrator HAL %s failed: %s", functionNameForLogging, errorMessage()); - return true; - } - return false; - } private: std::optional<T> mValue; - std::string mErrorMessage; - bool mUnsupported; explicit HalResult(T value) - : mValue(std::make_optional(value)), mErrorMessage(), mUnsupported(false) {} - explicit HalResult(std::string errorMessage, bool unsupported) - : mValue(), mErrorMessage(std::move(errorMessage)), mUnsupported(unsupported) {} + : BaseHalResult(Status::SUCCESS), mValue(std::make_optional(value)) {} + explicit HalResult(Status status, const char* errorMessage = "", bool deadObject = false) + : BaseHalResult(status, errorMessage, deadObject), mValue() {} }; // Empty result of a call to the Vibrator HAL wrapper. template <> -class HalResult<void> { +class HalResult<void> : public BaseHalResult { public: - static HalResult<void> ok() { return HalResult(); } - static HalResult<void> failed(std::string msg) { return HalResult(std::move(msg)); } - static HalResult<void> unsupported() { return HalResult(/* unsupported= */ true); } + static HalResult<void> ok() { return HalResult(Status::SUCCESS); } + static HalResult<void> unsupported() { return HalResult(Status::UNSUPPORTED); } + static HalResult<void> failed(const char* msg) { return HalResult(Status::FAILED, msg); } + static HalResult<void> transactionFailed(const char* msg) { + return HalResult(Status::FAILED, msg, /* deadObject= */ true); + } - static HalResult<void> fromStatus(status_t status); - static HalResult<void> fromStatus(binder::Status status); - static HalResult<void> fromStatus(hardware::vibrator::V1_0::Status status); +private: + explicit HalResult(Status status, const char* errorMessage = "", bool deadObject = false) + : BaseHalResult(status, errorMessage, deadObject) {} +}; + +// ------------------------------------------------------------------------------------------------- + +// Factory functions that convert failed HIDL/AIDL results into HalResult instances. +// Implementation of static template functions needs to be in this header file for the linker. +class HalResultFactory { +public: + template <typename T> + static HalResult<T> fromStatus(binder::Status status, T data) { + return status.isOk() ? HalResult<T>::ok(data) : fromFailedStatus<T>(status); + } + + template <typename T> + static HalResult<T> fromStatus(hardware::vibrator::V1_0::Status status, T data) { + return (status == hardware::vibrator::V1_0::Status::OK) ? HalResult<T>::ok(data) + : fromFailedStatus<T>(status); + } + + template <typename T, typename R> + static HalResult<T> fromReturn(hardware::Return<R>& ret, T data) { + return ret.isOk() ? HalResult<T>::ok(data) : fromFailedReturn<T, R>(ret); + } + + template <typename T, typename R> + static HalResult<T> fromReturn(hardware::Return<R>& ret, + hardware::vibrator::V1_0::Status status, T data) { + return ret.isOk() ? fromStatus<T>(status, data) : fromFailedReturn<T, R>(ret); + } + + static HalResult<void> fromStatus(status_t status) { + return (status == android::OK) ? HalResult<void>::ok() : fromFailedStatus<void>(status); + } + + static HalResult<void> fromStatus(binder::Status status) { + return status.isOk() ? HalResult<void>::ok() : fromFailedStatus<void>(status); + } + + static HalResult<void> fromStatus(hardware::vibrator::V1_0::Status status) { + return (status == hardware::vibrator::V1_0::Status::OK) ? HalResult<void>::ok() + : fromFailedStatus<void>(status); + } template <typename R> - static HalResult<void> fromReturn(hardware::Return<R>& ret); + static HalResult<void> fromReturn(hardware::Return<R>& ret) { + return ret.isOk() ? HalResult<void>::ok() : fromFailedReturn<void, R>(ret); + } - bool isOk() const { return !mUnsupported && !mFailed; } - bool isFailed() const { return !mUnsupported && mFailed; } - bool isUnsupported() const { return mUnsupported; } - const char* errorMessage() const { return mErrorMessage.c_str(); } - bool isFailedLogged(const char* functionNameForLogging) const { - if (isFailed()) { - ALOGE("Vibrator HAL %s failed: %s", functionNameForLogging, errorMessage()); - return true; +private: + template <typename T> + static HalResult<T> fromFailedStatus(status_t status) { + auto msg = "status_t = " + statusToString(status); + return (status == android::DEAD_OBJECT) ? HalResult<T>::transactionFailed(msg.c_str()) + : HalResult<T>::failed(msg.c_str()); + } + + template <typename T> + static HalResult<T> fromFailedStatus(binder::Status status) { + if (status.exceptionCode() == binder::Status::EX_UNSUPPORTED_OPERATION || + status.transactionError() == android::UNKNOWN_TRANSACTION) { + // UNKNOWN_TRANSACTION means the HAL implementation is an older version, so this is + // the same as the operation being unsupported by this HAL. Should not retry. + return HalResult<T>::unsupported(); } - return false; + if (status.exceptionCode() == binder::Status::EX_TRANSACTION_FAILED) { + return HalResult<T>::transactionFailed(status.toString8().c_str()); + } + return HalResult<T>::failed(status.toString8().c_str()); } -private: - std::string mErrorMessage; - bool mFailed; - bool mUnsupported; + template <typename T> + static HalResult<T> fromFailedStatus(hardware::vibrator::V1_0::Status status) { + switch (status) { + case hardware::vibrator::V1_0::Status::UNSUPPORTED_OPERATION: + return HalResult<T>::unsupported(); + default: + auto msg = "android::hardware::vibrator::V1_0::Status = " + toString(status); + return HalResult<T>::failed(msg.c_str()); + } + } - explicit HalResult(bool unsupported = false) - : mErrorMessage(), mFailed(false), mUnsupported(unsupported) {} - explicit HalResult(std::string errorMessage) - : mErrorMessage(std::move(errorMessage)), mFailed(true), mUnsupported(false) {} + template <typename T, typename R> + static HalResult<T> fromFailedReturn(hardware::Return<R>& ret) { + return ret.isDeadObject() ? HalResult<T>::transactionFailed(ret.description().c_str()) + : HalResult<T>::failed(ret.description().c_str()); + } }; +// ------------------------------------------------------------------------------------------------- + class HalCallbackWrapper : public hardware::vibrator::BnVibratorCallback { public: HalCallbackWrapper(std::function<void()> completionCallback) @@ -192,21 +245,44 @@ public: const HalResult<float> qFactor; const HalResult<std::vector<float>> maxAmplitudes; - bool isFailedLogged(const char*) const { - return capabilities.isFailedLogged("getCapabilities") || - supportedEffects.isFailedLogged("getSupportedEffects") || - supportedBraking.isFailedLogged("getSupportedBraking") || - supportedPrimitives.isFailedLogged("getSupportedPrimitives") || - primitiveDurations.isFailedLogged("getPrimitiveDuration") || - primitiveDelayMax.isFailedLogged("getPrimitiveDelayMax") || - pwlePrimitiveDurationMax.isFailedLogged("getPwlePrimitiveDurationMax") || - compositionSizeMax.isFailedLogged("getCompositionSizeMax") || - pwleSizeMax.isFailedLogged("getPwleSizeMax") || - minFrequency.isFailedLogged("getMinFrequency") || - resonantFrequency.isFailedLogged("getResonantFrequency") || - frequencyResolution.isFailedLogged("getFrequencyResolution") || - qFactor.isFailedLogged("getQFactor") || - maxAmplitudes.isFailedLogged("getMaxAmplitudes"); + void logFailures() const { + logFailure<Capabilities>(capabilities, "getCapabilities"); + logFailure<std::vector<hardware::vibrator::Effect>>(supportedEffects, + "getSupportedEffects"); + logFailure<std::vector<hardware::vibrator::Braking>>(supportedBraking, + "getSupportedBraking"); + logFailure<std::vector<hardware::vibrator::CompositePrimitive>>(supportedPrimitives, + "getSupportedPrimitives"); + logFailure<std::vector<std::chrono::milliseconds>>(primitiveDurations, + "getPrimitiveDuration"); + logFailure<std::chrono::milliseconds>(primitiveDelayMax, "getPrimitiveDelayMax"); + logFailure<std::chrono::milliseconds>(pwlePrimitiveDurationMax, + "getPwlePrimitiveDurationMax"); + logFailure<int32_t>(compositionSizeMax, "getCompositionSizeMax"); + logFailure<int32_t>(pwleSizeMax, "getPwleSizeMax"); + logFailure<float>(minFrequency, "getMinFrequency"); + logFailure<float>(resonantFrequency, "getResonantFrequency"); + logFailure<float>(frequencyResolution, "getFrequencyResolution"); + logFailure<float>(qFactor, "getQFactor"); + logFailure<std::vector<float>>(maxAmplitudes, "getMaxAmplitudes"); + } + + bool shouldRetry() const { + return capabilities.shouldRetry() || supportedEffects.shouldRetry() || + supportedBraking.shouldRetry() || supportedPrimitives.shouldRetry() || + primitiveDurations.shouldRetry() || primitiveDelayMax.shouldRetry() || + pwlePrimitiveDurationMax.shouldRetry() || compositionSizeMax.shouldRetry() || + pwleSizeMax.shouldRetry() || minFrequency.shouldRetry() || + resonantFrequency.shouldRetry() || frequencyResolution.shouldRetry() || + qFactor.shouldRetry() || maxAmplitudes.shouldRetry(); + } + +private: + template <typename T> + void logFailure(HalResult<T> result, const char* functionName) const { + if (result.isFailed()) { + ALOGE("Vibrator HAL %s failed: %s", functionName, result.errorMessage()); + } } }; @@ -230,27 +306,29 @@ public: } private: + // Create a transaction failed results as default so we can retry on the first time we get them. static const constexpr char* MSG = "never loaded"; - HalResult<Capabilities> mCapabilities = HalResult<Capabilities>::failed(MSG); + HalResult<Capabilities> mCapabilities = HalResult<Capabilities>::transactionFailed(MSG); HalResult<std::vector<hardware::vibrator::Effect>> mSupportedEffects = - HalResult<std::vector<hardware::vibrator::Effect>>::failed(MSG); + HalResult<std::vector<hardware::vibrator::Effect>>::transactionFailed(MSG); HalResult<std::vector<hardware::vibrator::Braking>> mSupportedBraking = - HalResult<std::vector<hardware::vibrator::Braking>>::failed(MSG); + HalResult<std::vector<hardware::vibrator::Braking>>::transactionFailed(MSG); HalResult<std::vector<hardware::vibrator::CompositePrimitive>> mSupportedPrimitives = - HalResult<std::vector<hardware::vibrator::CompositePrimitive>>::failed(MSG); + HalResult<std::vector<hardware::vibrator::CompositePrimitive>>::transactionFailed(MSG); HalResult<std::vector<std::chrono::milliseconds>> mPrimitiveDurations = - HalResult<std::vector<std::chrono::milliseconds>>::failed(MSG); + HalResult<std::vector<std::chrono::milliseconds>>::transactionFailed(MSG); HalResult<std::chrono::milliseconds> mPrimitiveDelayMax = - HalResult<std::chrono::milliseconds>::failed(MSG); + HalResult<std::chrono::milliseconds>::transactionFailed(MSG); HalResult<std::chrono::milliseconds> mPwlePrimitiveDurationMax = - HalResult<std::chrono::milliseconds>::failed(MSG); - HalResult<int32_t> mCompositionSizeMax = HalResult<int>::failed(MSG); - HalResult<int32_t> mPwleSizeMax = HalResult<int>::failed(MSG); - HalResult<float> mMinFrequency = HalResult<float>::failed(MSG); - HalResult<float> mResonantFrequency = HalResult<float>::failed(MSG); - HalResult<float> mFrequencyResolution = HalResult<float>::failed(MSG); - HalResult<float> mQFactor = HalResult<float>::failed(MSG); - HalResult<std::vector<float>> mMaxAmplitudes = HalResult<std::vector<float>>::failed(MSG); + HalResult<std::chrono::milliseconds>::transactionFailed(MSG); + HalResult<int32_t> mCompositionSizeMax = HalResult<int>::transactionFailed(MSG); + HalResult<int32_t> mPwleSizeMax = HalResult<int>::transactionFailed(MSG); + HalResult<float> mMinFrequency = HalResult<float>::transactionFailed(MSG); + HalResult<float> mResonantFrequency = HalResult<float>::transactionFailed(MSG); + HalResult<float> mFrequencyResolution = HalResult<float>::transactionFailed(MSG); + HalResult<float> mQFactor = HalResult<float>::transactionFailed(MSG); + HalResult<std::vector<float>> mMaxAmplitudes = + HalResult<std::vector<float>>::transactionFailed(MSG); friend class HalWrapper; }; diff --git a/services/vibratorservice/test/VibratorHalControllerTest.cpp b/services/vibratorservice/test/VibratorHalControllerTest.cpp index 8e77bc57f3..9b95d74899 100644 --- a/services/vibratorservice/test/VibratorHalControllerTest.cpp +++ b/services/vibratorservice/test/VibratorHalControllerTest.cpp @@ -107,17 +107,38 @@ TEST_F(VibratorHalControllerTest, TestInit) { ASSERT_EQ(1, mConnectCounter); } -TEST_F(VibratorHalControllerTest, TestGetInfoRetriesOnAnyFailure) { +TEST_F(VibratorHalControllerTest, TestGetInfoRetriesOnTransactionFailure) { EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1)); EXPECT_CALL(*mMockHal.get(), getCapabilitiesInternal()) .Times(Exactly(2)) - .WillOnce(Return(vibrator::HalResult<vibrator::Capabilities>::failed("message"))) + .WillOnce(Return(vibrator::HalResult<vibrator::Capabilities>::transactionFailed("msg"))) .WillRepeatedly(Return(vibrator::HalResult<vibrator::Capabilities>::ok( vibrator::Capabilities::ON_CALLBACK))); auto result = mController->getInfo(); - ASSERT_FALSE(result.capabilities.isFailed()); + ASSERT_TRUE(result.capabilities.isOk()); + ASSERT_EQ(1, mConnectCounter); +} + +TEST_F(VibratorHalControllerTest, TestGetInfoDoesNotRetryOnOperationFailure) { + EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(0)); + EXPECT_CALL(*mMockHal.get(), getCapabilitiesInternal()) + .Times(Exactly(1)) + .WillRepeatedly(Return(vibrator::HalResult<vibrator::Capabilities>::failed("msg"))); + auto result = mController->getInfo(); + ASSERT_TRUE(result.capabilities.isFailed()); + ASSERT_EQ(1, mConnectCounter); +} + +TEST_F(VibratorHalControllerTest, TestGetInfoDoesNotRetryOnUnsupported) { + EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(0)); + EXPECT_CALL(*mMockHal.get(), getCapabilitiesInternal()) + .Times(Exactly(1)) + .WillRepeatedly(Return(vibrator::HalResult<vibrator::Capabilities>::unsupported())); + + auto result = mController->getInfo(); + ASSERT_TRUE(result.capabilities.isUnsupported()); ASSERT_EQ(1, mConnectCounter); } @@ -128,48 +149,54 @@ TEST_F(VibratorHalControllerTest, TestApiCallsAreForwardedToHal) { auto result = mController->doWithRetry<void>(ON_FN, "on"); ASSERT_TRUE(result.isOk()); - ASSERT_EQ(1, mConnectCounter); } -TEST_F(VibratorHalControllerTest, TestUnsupportedApiResultDoNotResetHalConnection) { +TEST_F(VibratorHalControllerTest, TestUnsupportedApiResultDoesNotResetHalConnection) { + EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(0)); EXPECT_CALL(*mMockHal.get(), off()) .Times(Exactly(1)) .WillRepeatedly(Return(vibrator::HalResult<void>::unsupported())); - ASSERT_EQ(0, mConnectCounter); auto result = mController->doWithRetry<void>(OFF_FN, "off"); ASSERT_TRUE(result.isUnsupported()); ASSERT_EQ(1, mConnectCounter); } -TEST_F(VibratorHalControllerTest, TestFailedApiResultResetsHalConnection) { +TEST_F(VibratorHalControllerTest, TestOperationFailedApiResultDoesNotResetHalConnection) { + EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(0)); EXPECT_CALL(*mMockHal.get(), on(_, _)) - .Times(Exactly(2)) + .Times(Exactly(1)) .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message"))); - EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1)); - ASSERT_EQ(0, mConnectCounter); + auto result = mController->doWithRetry<void>(ON_FN, "on"); + ASSERT_TRUE(result.isFailed()); + ASSERT_EQ(1, mConnectCounter); +} + +TEST_F(VibratorHalControllerTest, TestTransactionFailedApiResultResetsHalConnection) { + EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1)); + EXPECT_CALL(*mMockHal.get(), on(_, _)) + .Times(Exactly(2)) + .WillRepeatedly(Return(vibrator::HalResult<void>::transactionFailed("message"))); auto result = mController->doWithRetry<void>(ON_FN, "on"); ASSERT_TRUE(result.isFailed()); ASSERT_EQ(1, mConnectCounter); } -TEST_F(VibratorHalControllerTest, TestFailedApiResultReturnsSuccessAfterRetries) { +TEST_F(VibratorHalControllerTest, TestTransactionFailedApiResultReturnsSuccessAfterRetries) { { InSequence seq; EXPECT_CALL(*mMockHal.get(), ping()) .Times(Exactly(1)) - .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message"))); + .WillRepeatedly(Return(vibrator::HalResult<void>::transactionFailed("message"))); EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1)); EXPECT_CALL(*mMockHal.get(), ping()) .Times(Exactly(1)) .WillRepeatedly(Return(vibrator::HalResult<void>::ok())); } - ASSERT_EQ(0, mConnectCounter); - auto result = mController->doWithRetry<void>(PING_FN, "ping"); ASSERT_TRUE(result.isOk()); ASSERT_EQ(1, mConnectCounter); @@ -221,11 +248,11 @@ TEST_F(VibratorHalControllerTest, TestScheduledCallbackSurvivesReconnection) { }); EXPECT_CALL(*mMockHal.get(), ping()) .Times(Exactly(1)) - .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message"))); + .WillRepeatedly(Return(vibrator::HalResult<void>::transactionFailed("message"))); EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1)); EXPECT_CALL(*mMockHal.get(), ping()) .Times(Exactly(1)) - .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message"))); + .WillRepeatedly(Return(vibrator::HalResult<void>::transactionFailed("message"))); } std::unique_ptr<int32_t> callbackCounter = std::make_unique<int32_t>(); diff --git a/services/vibratorservice/test/VibratorManagerHalControllerTest.cpp b/services/vibratorservice/test/VibratorManagerHalControllerTest.cpp index e5fbbaeac9..11a8b66968 100644 --- a/services/vibratorservice/test/VibratorManagerHalControllerTest.cpp +++ b/services/vibratorservice/test/VibratorManagerHalControllerTest.cpp @@ -98,8 +98,10 @@ protected: .WillRepeatedly(Return(voidResult)); if (cardinality > 1) { - // One reconnection call after each failure. - EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(7 * cardinality)); + // One reconnection for each retry. + EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(7 * (cardinality - 1))); + } else { + EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(0)); } } }; @@ -141,14 +143,12 @@ TEST_F(VibratorManagerHalControllerTest, TestApiCallsAreForwardedToHal) { ASSERT_EQ(1, mConnectCounter); } -TEST_F(VibratorManagerHalControllerTest, TestUnsupportedApiResultDoNotResetHalConnection) { +TEST_F(VibratorManagerHalControllerTest, TestUnsupportedApiResultDoesNotResetHalConnection) { setHalExpectations(/* cardinality= */ 1, vibrator::HalResult<void>::unsupported(), vibrator::HalResult<vibrator::ManagerCapabilities>::unsupported(), vibrator::HalResult<std::vector<int32_t>>::unsupported(), vibrator::HalResult<std::shared_ptr<HalController>>::unsupported()); - ASSERT_EQ(0, mConnectCounter); - ASSERT_TRUE(mController->ping().isUnsupported()); ASSERT_TRUE(mController->getCapabilities().isUnsupported()); ASSERT_TRUE(mController->getVibratorIds().isUnsupported()); @@ -160,13 +160,28 @@ TEST_F(VibratorManagerHalControllerTest, TestUnsupportedApiResultDoNotResetHalCo ASSERT_EQ(1, mConnectCounter); } -TEST_F(VibratorManagerHalControllerTest, TestFailedApiResultResetsHalConnection) { - setHalExpectations(MAX_ATTEMPTS, vibrator::HalResult<void>::failed("message"), - vibrator::HalResult<vibrator::ManagerCapabilities>::failed("message"), - vibrator::HalResult<std::vector<int32_t>>::failed("message"), - vibrator::HalResult<std::shared_ptr<HalController>>::failed("message")); +TEST_F(VibratorManagerHalControllerTest, TestOperationFailedApiResultDoesNotResetHalConnection) { + setHalExpectations(/* cardinality= */ 1, vibrator::HalResult<void>::failed("msg"), + vibrator::HalResult<vibrator::ManagerCapabilities>::failed("msg"), + vibrator::HalResult<std::vector<int32_t>>::failed("msg"), + vibrator::HalResult<std::shared_ptr<HalController>>::failed("msg")); - ASSERT_EQ(0, mConnectCounter); + ASSERT_TRUE(mController->ping().isFailed()); + ASSERT_TRUE(mController->getCapabilities().isFailed()); + ASSERT_TRUE(mController->getVibratorIds().isFailed()); + ASSERT_TRUE(mController->getVibrator(VIBRATOR_ID).isFailed()); + ASSERT_TRUE(mController->prepareSynced(VIBRATOR_IDS).isFailed()); + ASSERT_TRUE(mController->triggerSynced([]() {}).isFailed()); + ASSERT_TRUE(mController->cancelSynced().isFailed()); + + ASSERT_EQ(1, mConnectCounter); +} + +TEST_F(VibratorManagerHalControllerTest, TestTransactionFailedApiResultResetsHalConnection) { + setHalExpectations(MAX_ATTEMPTS, vibrator::HalResult<void>::transactionFailed("m"), + vibrator::HalResult<vibrator::ManagerCapabilities>::transactionFailed("m"), + vibrator::HalResult<std::vector<int32_t>>::transactionFailed("m"), + vibrator::HalResult<std::shared_ptr<HalController>>::transactionFailed("m")); ASSERT_TRUE(mController->ping().isFailed()); ASSERT_TRUE(mController->getCapabilities().isFailed()); @@ -184,14 +199,13 @@ TEST_F(VibratorManagerHalControllerTest, TestFailedApiResultReturnsSuccessAfterR InSequence seq; EXPECT_CALL(*mMockHal.get(), ping()) .Times(Exactly(1)) - .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message"))); + .WillRepeatedly(Return(vibrator::HalResult<void>::transactionFailed("message"))); EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1)); EXPECT_CALL(*mMockHal.get(), ping()) .Times(Exactly(1)) .WillRepeatedly(Return(vibrator::HalResult<void>::ok())); } - ASSERT_EQ(0, mConnectCounter); ASSERT_TRUE(mController->ping().isOk()); ASSERT_EQ(1, mConnectCounter); } diff --git a/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp b/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp index 1593cb18ec..dffc281fa1 100644 --- a/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp +++ b/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp @@ -254,13 +254,14 @@ TEST_F(VibratorManagerHalWrapperAidlTest, TestGetVibratorRecoversVibratorPointer EXPECT_CALL(*mMockHal.get(), getVibrator(Eq(kVibratorId), _)) .Times(Exactly(3)) .WillOnce(DoAll(SetArgPointee<1>(nullptr), - Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY)))) + Return(Status::fromExceptionCode( + Status::Exception::EX_TRANSACTION_FAILED)))) .WillRepeatedly(DoAll(SetArgPointee<1>(mMockVibrator), Return(Status()))); EXPECT_CALL(*mMockVibrator.get(), off()) .Times(Exactly(3)) - .WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY))) - .WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY))) + .WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_TRANSACTION_FAILED))) + .WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_TRANSACTION_FAILED))) .WillRepeatedly(Return(Status())); // Get vibrator controller is successful even if first getVibrator. |