Merge "Fix result check of IBinder::pingBinder on vibrator HAl wrapper" (fbbd1a5e) · Commits · e / os / android_frameworks_native-old

services/vibratorservice/VibratorHalController.cpp

+1 −1

Original line number	Diff line number	Diff line
		@@ -91,7 +91,7 @@ std::shared_ptr<HalWrapper> HalConnector::connect(std::shared_ptr<CallbackSchedu
		template <typename T>
		HalResult<T> HalController::processHalResult(HalResult<T> result, const char* functionName) {
		if (result.isFailed()) {
		ALOGE("%s failed: Vibrator HAL not available", functionName);
		ALOGE("%s failed: %s", functionName, result.errorMessage());
		std::lock_guard<std::mutex> lock(mConnectedHalMutex);
		mConnectedHal->tryReconnect();
		}

services/vibratorservice/VibratorHalWrapper.cpp

+23 −12

Original line number	Diff line number	Diff line
		@@ -67,6 +67,10 @@ 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(binder::Status status, T data) {
		if (status.exceptionCode() == binder::Status::EX_UNSUPPORTED_OPERATION) {
		@@ -75,7 +79,7 @@ HalResult<T> HalResult<T>::fromStatus(binder::Status status, T data) {
		if (status.isOk()) {
		return HalResult<T>::ok(data);
		}
		return HalResult<T>::failed();
		return HalResult<T>::failed(std::string(status.toString8().c_str()));
		}

		template <typename T>
		@@ -86,24 +90,32 @@ HalResult<T> HalResult<T>::fromStatus(V1_0::Status status, T data) {
		case V1_0::Status::UNSUPPORTED_OPERATION:
		return HalResult<T>::unsupported();
		default:
		return HalResult<T>::failed();
		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();
		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();
		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) {
		return HalResult<void>::unsupported();
		@@ -111,7 +123,7 @@ HalResult<void> HalResult<void>::fromStatus(binder::Status status) {
		if (status.isOk()) {
		return HalResult<void>::ok();
		}
		return HalResult<void>::failed();
		return HalResult<void>::failed(std::string(status.toString8().c_str()));
		}

		HalResult<void> HalResult<void>::fromStatus(V1_0::Status status) {
		@@ -121,13 +133,13 @@ HalResult<void> HalResult<void>::fromStatus(V1_0::Status status) {
		case V1_0::Status::UNSUPPORTED_OPERATION:
		return HalResult<void>::unsupported();
		default:
		return HalResult<void>::failed();
		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();
		return ret.isOk() ? HalResult<void>::ok() : HalResult<void>::failed(ret.description());
		}

		// -------------------------------------------------------------------------------------------------
		@@ -149,8 +161,7 @@ private:
		// -------------------------------------------------------------------------------------------------

		HalResult<void> AidlHalWrapper::ping() {
		// TODO(b/153415251): Investigate why IBinder::pingBinder() is returning false even on success.
		return getCapabilitiesInternal().isFailed() ? HalResult<void>::failed() : HalResult<void>::ok();
		return HalResult<void>::fromStatus(IInterface::asBinder(getHal())->pingBinder());
		}

		void AidlHalWrapper::tryReconnect() {
		@@ -456,15 +467,15 @@ HalResult<milliseconds> HidlHalWrapperV1_3::performEffect(
		}

		HalResult<Capabilities> HidlHalWrapperV1_3::getCapabilitiesInternal() {
		Capabilities capabilities = Capabilities::NONE;

		sp<V1_3::IVibrator> hal = getHal();
		auto amplitudeResult = hal->supportsAmplitudeControl();
		if (!amplitudeResult.isOk()) {
		return HalResult<Capabilities>::failed();
		return HalResult<Capabilities>::fromReturn(amplitudeResult, capabilities);
		}

		auto externalControlResult = hal->supportsExternalControl();
		Capabilities capabilities = Capabilities::NONE;

		if (amplitudeResult.withDefault(false)) {
		capabilities \|= Capabilities::AMPLITUDE_CONTROL;
		}

services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h

+19 −10

Original line number	Diff line number	Diff line
		@@ -35,8 +35,10 @@ template <typename T>
		class HalResult {
		public:
		static HalResult<T> ok(T value) { return HalResult(value); }
		static HalResult<T> failed() { return HalResult(/* unsupported= */ false); }
		static HalResult<T> unsupported() { return HalResult(/* unsupported= */ true); }
		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);
		static HalResult<T> fromStatus(hardware::vibrator::V1_0::Status status, T data);
		@@ -53,13 +55,17 @@ public:
		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(); }

		private:
		std::optional<T> mValue;
		std::string mErrorMessage;
		bool mUnsupported;

		explicit HalResult(T value) : mValue(std::make_optional(value)), mUnsupported(false) {}
		explicit HalResult(bool unsupported) : mValue(), mUnsupported(unsupported) {}
		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) {}
		};

		// Empty result of a call to the Vibrator HAL wrapper.
		@@ -67,11 +73,10 @@ template <>
		class HalResult<void> {
		public:
		static HalResult<void> ok() { return HalResult(); }
		static HalResult<void> failed() { return HalResult(/* failed= */ true); }
		static HalResult<void> unsupported() {
		return HalResult(/* failed= / false, / unsupported= */ true);
		}
		static HalResult<void> failed(std::string msg) { return HalResult(std::move(msg)); }
		static HalResult<void> unsupported() { return HalResult(/* unsupported= */ 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);

		@@ -81,13 +86,17 @@ public:
		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(); }

		private:
		std::string mErrorMessage;
		bool mFailed;
		bool mUnsupported;

		explicit HalResult(bool failed = false, bool unsupported = false)
		: mFailed(failed), mUnsupported(unsupported) {}
		explicit HalResult(bool unsupported = false)
		: mErrorMessage(), mFailed(false), mUnsupported(unsupported) {}
		explicit HalResult(std::string errorMessage)
		: mErrorMessage(std::move(errorMessage)), mFailed(true), mUnsupported(false) {}
		};

		// -------------------------------------------------------------------------------------------------

services/vibratorservice/test/VibratorHalControllerTest.cpp

+7 −7

Original line number	Diff line number	Diff line
		@@ -257,10 +257,10 @@ TEST_F(VibratorHalControllerTest, TestUnsupportedApiResultDoNotResetHalConnectio

		TEST_F(VibratorHalControllerTest, TestFailedApiResultResetsHalConnection) {
		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());
		vibrator::HalResult<void>::failed("message"),
		vibrator::HalResult<vibrator::Capabilities>::failed("message"),
		vibrator::HalResult<std::vector<Effect>>::failed("message"),
		vibrator::HalResult<milliseconds>::failed("message"));

		ASSERT_EQ(0, mConnectCounter);

		@@ -286,7 +286,7 @@ TEST_F(VibratorHalControllerTest, TestFailedApiResultReturnsSuccessAfterRetries)
		InSequence seq;
		EXPECT_CALL(*mMockHal.get(), ping())
		.Times(Exactly(1))
		.WillRepeatedly(Return(vibrator::HalResult<void>::failed()));
		.WillRepeatedly(Return(vibrator::HalResult<void>::failed("message")));
		EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1));
		EXPECT_CALL(*mMockHal.get(), ping())
		.Times(Exactly(1))
		@@ -351,11 +351,11 @@ TEST_F(VibratorHalControllerTest, TestScheduledCallbackSurvivesReconnection) {
		});
		EXPECT_CALL(*mMockHal.get(), ping())
		.Times(Exactly(1))
		.WillRepeatedly(Return(vibrator::HalResult<void>::failed()));
		.WillRepeatedly(Return(vibrator::HalResult<void>::failed("message")));
		EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1));
		EXPECT_CALL(*mMockHal.get(), ping())
		.Times(Exactly(1))
		.WillRepeatedly(Return(vibrator::HalResult<void>::failed()));
		.WillRepeatedly(Return(vibrator::HalResult<void>::failed("message")));
		EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1));
		}

services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp

+8 −11

Original line number	Diff line number	Diff line
		@@ -116,19 +116,16 @@ ACTION(TriggerCallbackInArg2) {
		}

		TEST_F(VibratorHalWrapperAidlTest, TestPing) {
		{
		InSequence seq;
		EXPECT_CALL(*mMockHal.get(), getCapabilities(_))
		.Times(Exactly(3))
		.WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY)))
		.WillOnce(Return(
		Status::fromExceptionCode(Status::Exception::EX_UNSUPPORTED_OPERATION)))
		.WillRepeatedly(Return(Status()));
		}
		EXPECT_CALL(*mMockHal.get(), onAsBinder())
		.Times(Exactly(2))
		.WillRepeatedly(Return(mMockBinder.get()));
		EXPECT_CALL(*mMockBinder.get(), pingBinder())
		.Times(Exactly(2))
		.WillOnce(Return(android::OK))
		.WillRepeatedly(Return(android::DEAD_OBJECT));

		ASSERT_TRUE(mWrapper->ping().isFailed());
		ASSERT_TRUE(mWrapper->ping().isOk());
		ASSERT_TRUE(mWrapper->ping().isOk());
		ASSERT_TRUE(mWrapper->ping().isFailed());
		}

		TEST_F(VibratorHalWrapperAidlTest, TestOnWithCallbackSupport) {