Add retries to vibrator HAL controller
Moving retry behavior from VibratorService.cpp to new vibrator HAL
controller class, to prevent failures on api calls when the service
connection can be recovered right away.
Bug: 153418251
Test: atest libvibratorservice_test
Change-Id: I322c4e9f15dfeefdc5dca9c37a3f1110b26dd2f6
diff --git a/services/vibratorservice/VibratorHalController.cpp b/services/vibratorservice/VibratorHalController.cpp
index ef1d061..896c162 100644
--- a/services/vibratorservice/VibratorHalController.cpp
+++ b/services/vibratorservice/VibratorHalController.cpp
@@ -45,6 +45,8 @@
// -------------------------------------------------------------------------------------------------
+static constexpr int MAX_RETRIES = 1;
+
template <typename T>
using hal_connect_fn = std::function<sp<T>()>;
@@ -111,6 +113,14 @@
// -------------------------------------------------------------------------------------------------
+std::shared_ptr<HalWrapper> HalController::initHal() {
+ std::lock_guard<std::mutex> lock(mConnectedHalMutex);
+ if (mConnectedHal == nullptr) {
+ mConnectedHal = mHalConnector->connect(mCallbackScheduler);
+ }
+ return mConnectedHal;
+}
+
template <typename T>
HalResult<T> HalController::processHalResult(HalResult<T> result, const char* functionName) {
if (result.isFailed()) {
@@ -124,20 +134,21 @@
template <typename T>
HalResult<T> HalController::apply(HalController::hal_fn<T>& halFn, const char* functionName) {
- std::shared_ptr<HalWrapper> hal = nullptr;
- {
- std::lock_guard<std::mutex> lock(mConnectedHalMutex);
- if (mConnectedHal == nullptr) {
- mConnectedHal = mHalConnector->connect(mCallbackScheduler);
- }
- hal = mConnectedHal;
- }
- if (hal) {
- return processHalResult(halFn(hal), functionName);
+ std::shared_ptr<HalWrapper> hal = initHal();
+ if (hal == nullptr) {
+ ALOGV("Skipped %s because Vibrator HAL is not available", functionName);
+ return HalResult<T>::unsupported();
}
- ALOGV("Skipped %s because Vibrator HAL is not available", functionName);
- return HalResult<T>::unsupported();
+ HalResult<T> ret = processHalResult(halFn(hal), functionName);
+ for (int i = 0; i < MAX_RETRIES && ret.isFailed(); i++) {
+ hal = initHal();
+ if (hal) {
+ ret = processHalResult(halFn(hal), functionName);
+ }
+ }
+
+ return ret;
}
// -------------------------------------------------------------------------------------------------
diff --git a/services/vibratorservice/include/vibratorservice/VibratorHalController.h b/services/vibratorservice/include/vibratorservice/VibratorHalController.h
index e254969..1fb7d05 100644
--- a/services/vibratorservice/include/vibratorservice/VibratorHalController.h
+++ b/services/vibratorservice/include/vibratorservice/VibratorHalController.h
@@ -81,6 +81,8 @@
// Shared pointer to allow local copies to be used by different threads.
std::shared_ptr<HalWrapper> mConnectedHal GUARDED_BY(mConnectedHalMutex);
+ std::shared_ptr<HalWrapper> initHal();
+
template <typename T>
HalResult<T> processHalResult(HalResult<T> result, const char* functionName);
diff --git a/services/vibratorservice/test/VibratorHalControllerTest.cpp b/services/vibratorservice/test/VibratorHalControllerTest.cpp
index 3d8b069..24e6a1e 100644
--- a/services/vibratorservice/test/VibratorHalControllerTest.cpp
+++ b/services/vibratorservice/test/VibratorHalControllerTest.cpp
@@ -42,6 +42,8 @@
using namespace std::chrono_literals;
using namespace testing;
+static constexpr int MAX_ATTEMPTS = 2;
+
// -------------------------------------------------------------------------------------------------
class MockHalWrapper : public vibrator::HalWrapper {
@@ -125,41 +127,45 @@
std::shared_ptr<MockHalWrapper> mMockHal;
std::unique_ptr<vibrator::HalController> mController;
- void setHalExpectations(std::vector<CompositeEffect> compositeEffects,
+ void setHalExpectations(int32_t cardinality, std::vector<CompositeEffect> compositeEffects,
vibrator::HalResult<void> voidResult,
vibrator::HalResult<vibrator::Capabilities> capabilitiesResult,
vibrator::HalResult<std::vector<Effect>> effectsResult,
vibrator::HalResult<milliseconds> durationResult) {
InSequence seq;
- EXPECT_CALL(*mMockHal.get(), ping()).Times(Exactly(1)).WillRepeatedly(Return(voidResult));
- EXPECT_CALL(*mMockHal.get(), on(Eq(10ms), _))
- .Times(Exactly(1))
+ EXPECT_CALL(*mMockHal.get(), ping())
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(voidResult));
- EXPECT_CALL(*mMockHal.get(), off()).Times(Exactly(1)).WillRepeatedly(Return(voidResult));
+ EXPECT_CALL(*mMockHal.get(), on(Eq(10ms), _))
+ .Times(Exactly(cardinality))
+ .WillRepeatedly(Return(voidResult));
+ EXPECT_CALL(*mMockHal.get(), off())
+ .Times(Exactly(cardinality))
+ .WillRepeatedly(Return(voidResult));
EXPECT_CALL(*mMockHal.get(), setAmplitude(Eq(255)))
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(voidResult));
EXPECT_CALL(*mMockHal.get(), setExternalControl(Eq(true)))
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(voidResult));
EXPECT_CALL(*mMockHal.get(),
alwaysOnEnable(Eq(1), Eq(Effect::CLICK), Eq(EffectStrength::LIGHT)))
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(voidResult));
EXPECT_CALL(*mMockHal.get(), alwaysOnDisable(Eq(1)))
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(voidResult));
EXPECT_CALL(*mMockHal.get(), getCapabilities())
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(capabilitiesResult));
EXPECT_CALL(*mMockHal.get(), getSupportedEffects())
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(effectsResult));
EXPECT_CALL(*mMockHal.get(), performEffect(Eq(Effect::CLICK), Eq(EffectStrength::LIGHT), _))
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(durationResult));
EXPECT_CALL(*mMockHal.get(), performComposedEffect(Eq(compositeEffects), _))
- .Times(Exactly(1))
+ .Times(Exactly(cardinality))
.WillRepeatedly(Return(voidResult));
}
};
@@ -176,7 +182,7 @@
compositeEffects.push_back(
vibrator::TestFactory::createCompositeEffect(CompositePrimitive::THUD, 1000ms, 1.0f));
- setHalExpectations(compositeEffects, vibrator::HalResult<void>::ok(),
+ setHalExpectations(/* cardinality= */ 1, compositeEffects, vibrator::HalResult<void>::ok(),
vibrator::HalResult<vibrator::Capabilities>::ok(
vibrator::Capabilities::ON_CALLBACK),
vibrator::HalResult<std::vector<Effect>>::ok(supportedEffects),
@@ -210,7 +216,8 @@
}
TEST_F(VibratorHalControllerTest, TestUnsupportedApiResultDoNotResetHalConnection) {
- setHalExpectations(std::vector<CompositeEffect>(), vibrator::HalResult<void>::unsupported(),
+ setHalExpectations(/* cardinality= */ 1, std::vector<CompositeEffect>(),
+ vibrator::HalResult<void>::unsupported(),
vibrator::HalResult<vibrator::Capabilities>::unsupported(),
vibrator::HalResult<std::vector<Effect>>::unsupported(),
vibrator::HalResult<milliseconds>::unsupported());
@@ -237,7 +244,8 @@
}
TEST_F(VibratorHalControllerTest, TestFailedApiResultResetsHalConnection) {
- setHalExpectations(std::vector<CompositeEffect>(), vibrator::HalResult<void>::failed(),
+ setHalExpectations(MAX_ATTEMPTS, std::vector<CompositeEffect>(),
+ vibrator::HalResult<void>::failed(),
vibrator::HalResult<vibrator::Capabilities>::failed(),
vibrator::HalResult<std::vector<Effect>>::failed(),
vibrator::HalResult<milliseconds>::failed());
@@ -258,8 +266,23 @@
ASSERT_TRUE(
mController->performComposedEffect(std::vector<CompositeEffect>(), []() {}).isFailed());
- // One reconnection attempt per api call.
- ASSERT_EQ(11, mConnectCounter);
+ // One reconnection attempt + retry attempts per api call.
+ ASSERT_EQ(11 * MAX_ATTEMPTS, mConnectCounter);
+}
+
+TEST_F(VibratorHalControllerTest, TestFailedApiResultReturnsSuccessAfterRetries) {
+ {
+ InSequence seq;
+ EXPECT_CALL(*mMockHal.get(), ping())
+ .Times(Exactly(2))
+ .WillOnce(Return(vibrator::HalResult<void>::failed()))
+ .WillRepeatedly(Return(vibrator::HalResult<void>::ok()));
+ }
+
+ ASSERT_EQ(0, mConnectCounter);
+ ASSERT_TRUE(mController->ping().isOk());
+ // One connect + one retry.
+ ASSERT_EQ(2, mConnectCounter);
}
TEST_F(VibratorHalControllerTest, TestMultiThreadConnectsOnlyOnce) {
@@ -279,7 +302,7 @@
ASSERT_EQ(1, mConnectCounter);
}
-TEST_F(VibratorHalControllerTest, TestNoHalReturnsUnsupportedAndAttemptsToReconnect) {
+TEST_F(VibratorHalControllerTest, TestNoVibratorReturnsUnsupportedAndAttemptsToReconnect) {
auto failingHalConnector = std::make_unique<FailingHalConnector>(&mConnectCounter);
mController =
std::make_unique<vibrator::HalController>(std::move(failingHalConnector), nullptr);
@@ -300,7 +323,7 @@
ASSERT_TRUE(mController->performComposedEffect(std::vector<CompositeEffect>(), []() {})
.isUnsupported());
- // One reconnection attempt per api call.
+ // One reconnection attempt per api call, no retry.
ASSERT_EQ(11, mConnectCounter);
}
@@ -314,7 +337,7 @@
return vibrator::HalResult<void>::ok();
});
EXPECT_CALL(*mMockHal.get(), ping())
- .Times(Exactly(1))
+ .Times(Exactly(MAX_ATTEMPTS))
.WillRepeatedly(Return(vibrator::HalResult<void>::failed()));
}