Merge "Use IBinder to identify callbacks in VHAL" into oc-dev (2e25e562) · Commits · e / os / android_hardware_interfaces

automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h

+8 −4

Original line number	Original line	Diff line number	Diff line
	@@ -78,6 +78,8 @@ struct HalClientValues {
	std::list<VehiclePropValue *> values;		std::list<VehiclePropValue *> values;
	};		};

			using ClientId = uint64_t;

	class SubscriptionManager {		class SubscriptionManager {
	public:		public:
	using OnPropertyUnsubscribed = std::function<void(int32_t)>;		using OnPropertyUnsubscribed = std::function<void(int32_t)>;
	@@ -100,7 +102,8 @@ public:
	* Updates subscription. Returns the vector of properties subscription that		* Updates subscription. Returns the vector of properties subscription that
	* needs to be updated in VehicleHAL.		* needs to be updated in VehicleHAL.
	*/		*/
	StatusCode addOrUpdateSubscription(const sp<IVehicleCallback>& callback,		StatusCode addOrUpdateSubscription(ClientId clientId,
			const sp<IVehicleCallback>& callback,
	const hidl_vec<SubscribeOptions>& optionList,		const hidl_vec<SubscribeOptions>& optionList,
	std::list<SubscribeOptions>* outUpdatedOptions);		std::list<SubscribeOptions>* outUpdatedOptions);

	@@ -119,7 +122,7 @@ public:
	* If there are no clients subscribed to given properties than callback function provided		* If there are no clients subscribed to given properties than callback function provided
	* in the constructor will be called.		* in the constructor will be called.
	*/		*/
	void unsubscribe(const sp<IVehicleCallback>& callback, int32_t propId);		void unsubscribe(ClientId clientId, int32_t propId);
	private:		private:
	std::list<sp<HalClient>> getSubscribedClientsLocked(int32_t propId,		std::list<sp<HalClient>> getSubscribedClientsLocked(int32_t propId,
	int32_t area,		int32_t area,
	@@ -131,7 +134,8 @@ private:

	sp<HalClientVector> getClientsForPropertyLocked(int32_t propId) const;		sp<HalClientVector> getClientsForPropertyLocked(int32_t propId) const;

	sp<HalClient> getOrCreateHalClientLocked(const sp<IVehicleCallback> &callback);		sp<HalClient> getOrCreateHalClientLocked(ClientId callingPid,
			const sp<IVehicleCallback>& callback);

	void onCallbackDead(uint64_t cookie);		void onCallbackDead(uint64_t cookie);

	@@ -160,7 +164,7 @@ private:

	mutable std::mutex mLock;		mutable std::mutex mLock;

	std::map<sp<IVehicleCallback>, sp<HalClient>> mClients;		std::map<ClientId, sp<HalClient>> mClients;
	std::map<int32_t, sp<HalClientVector>> mPropToClients;		std::map<int32_t, sp<HalClientVector>> mPropToClients;
	std::map<int32_t, SubscribeOptions> mHalEventSubscribeOptions;		std::map<int32_t, SubscribeOptions> mHalEventSubscribeOptions;

automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h

+1 −0

Original line number	Original line	Diff line number	Diff line
	@@ -101,6 +101,7 @@ private:
	static bool isSampleRateFixed(VehiclePropertyChangeMode mode);		static bool isSampleRateFixed(VehiclePropertyChangeMode mode);
	static float checkSampleRate(const VehiclePropConfig& config,		static float checkSampleRate(const VehiclePropConfig& config,
	float sampleRate);		float sampleRate);
			static ClientId getClientId(const sp<IVehicleCallback>& callback);
	private:		private:
	VehicleHal* mHal;		VehicleHal* mHal;
	std::unique_ptr<VehiclePropConfigIndex> mConfigIndex;		std::unique_ptr<VehiclePropConfigIndex> mConfigIndex;

automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp

+12 −10

Original line number	Original line	Diff line number	Diff line
	@@ -97,6 +97,7 @@ std::vector<int32_t> HalClient::getSubscribedProperties() const {
	}		}

	StatusCode SubscriptionManager::addOrUpdateSubscription(		StatusCode SubscriptionManager::addOrUpdateSubscription(
			ClientId clientId,
	const sp<IVehicleCallback> &callback,		const sp<IVehicleCallback> &callback,
	const hidl_vec<SubscribeOptions> &optionList,		const hidl_vec<SubscribeOptions> &optionList,
	std::list<SubscribeOptions>* outUpdatedSubscriptions) {		std::list<SubscribeOptions>* outUpdatedSubscriptions) {
	@@ -106,7 +107,7 @@ StatusCode SubscriptionManager::addOrUpdateSubscription(

	ALOGI("SubscriptionManager::addOrUpdateSubscription, callback: %p", callback.get());		ALOGI("SubscriptionManager::addOrUpdateSubscription, callback: %p", callback.get());

	const sp<HalClient>& client = getOrCreateHalClientLocked(callback);		const sp<HalClient>& client = getOrCreateHalClientLocked(clientId, callback);
	if (client.get() == nullptr) {		if (client.get() == nullptr) {
	return StatusCode::INTERNAL_ERROR;		return StatusCode::INTERNAL_ERROR;
	}		}
	@@ -221,10 +222,11 @@ sp<HalClientVector> SubscriptionManager::getClientsForPropertyLocked(
	}		}

	sp<HalClient> SubscriptionManager::getOrCreateHalClientLocked(		sp<HalClient> SubscriptionManager::getOrCreateHalClientLocked(
	const sp<IVehicleCallback>& callback) {		ClientId clientId, const sp<IVehicleCallback>& callback) {
	auto it = mClients.find(callback);		auto it = mClients.find(clientId);

	if (it == mClients.end()) {		if (it == mClients.end()) {
	uint64_t cookie = reinterpret_cast<uint64_t>(callback.get());		uint64_t cookie = reinterpret_cast<uint64_t>(clientId);
	ALOGI("Creating new client and linking to death recipient, cookie: 0x%" PRIx64, cookie);		ALOGI("Creating new client and linking to death recipient, cookie: 0x%" PRIx64, cookie);
	auto res = callback->linkToDeath(mCallbackDeathRecipient, cookie);		auto res = callback->linkToDeath(mCallbackDeathRecipient, cookie);
	if (!res.isOk()) { // Client is already dead?		if (!res.isOk()) { // Client is already dead?
	@@ -234,18 +236,18 @@ sp<HalClient> SubscriptionManager::getOrCreateHalClientLocked(
	}		}

	sp<HalClient> client = new HalClient(callback);		sp<HalClient> client = new HalClient(callback);
	mClients.emplace(callback, client);		mClients.insert({clientId, client});
	return client;		return client;
	} else {		} else {
	return it->second;		return it->second;
	}		}
	}		}

	void SubscriptionManager::unsubscribe(const sp<IVehicleCallback>& callback,		void SubscriptionManager::unsubscribe(ClientId clientId,
	int32_t propId) {		int32_t propId) {
	MuxGuard g(mLock);		MuxGuard g(mLock);
	auto propertyClients = getClientsForPropertyLocked(propId);		auto propertyClients = getClientsForPropertyLocked(propId);
	auto clientIter = mClients.find(callback);		auto clientIter = mClients.find(clientId);
	if (clientIter == mClients.end()) {		if (clientIter == mClients.end()) {
	ALOGW("Unable to unsubscribe: no callback found, propId: 0x%x", propId);		ALOGW("Unable to unsubscribe: no callback found, propId: 0x%x", propId);
	} else {		} else {
	@@ -285,12 +287,12 @@ void SubscriptionManager::unsubscribe(const sp<IVehicleCallback>& callback,

	void SubscriptionManager::onCallbackDead(uint64_t cookie) {		void SubscriptionManager::onCallbackDead(uint64_t cookie) {
	ALOGI("%s, cookie: 0x%" PRIx64, __func__, cookie);		ALOGI("%s, cookie: 0x%" PRIx64, __func__, cookie);
	IVehicleCallback* callback = reinterpret_cast<IVehicleCallback*>(cookie);		ClientId clientId = cookie;

	std::vector<int32_t> props;		std::vector<int32_t> props;
	{		{
	MuxGuard g(mLock);		MuxGuard g(mLock);
	const auto& it = mClients.find(callback);		const auto& it = mClients.find(clientId);
	if (it == mClients.end()) {		if (it == mClients.end()) {
	return; // Nothing to do here, client wasn't subscribed to any properties.		return; // Nothing to do here, client wasn't subscribed to any properties.
	}		}
	@@ -299,7 +301,7 @@ void SubscriptionManager::onCallbackDead(uint64_t cookie) {
	}		}

	for (int32_t propId : props) {		for (int32_t propId : props) {
	unsubscribe(callback, propId);		unsubscribe(clientId, propId);
	}		}
	}		}

automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp

+16 −2

Original line number	Original line	Diff line number	Diff line
	@@ -22,6 +22,7 @@
	#include <fstream>		#include <fstream>

	#include <android/log.h>		#include <android/log.h>
			#include <android/hardware/automotive/vehicle/2.0/BpHwVehicleCallback.h>

	#include "VehicleUtils.h"		#include "VehicleUtils.h"

	@@ -154,7 +155,8 @@ Return<StatusCode> VehicleHalManager::subscribe(const sp<IVehicleCallback> &call
	}		}

	std::list<SubscribeOptions> updatedOptions;		std::list<SubscribeOptions> updatedOptions;
	auto res = mSubscriptionManager.addOrUpdateSubscription(callback, verifiedOptions,		auto res = mSubscriptionManager.addOrUpdateSubscription(getClientId(callback),
			callback, verifiedOptions,
	&updatedOptions);		&updatedOptions);
	if (StatusCode::OK != res) {		if (StatusCode::OK != res) {
	ALOGW("%s failed to subscribe, error code: %d", __func__, res);		ALOGW("%s failed to subscribe, error code: %d", __func__, res);
	@@ -170,7 +172,7 @@ Return<StatusCode> VehicleHalManager::subscribe(const sp<IVehicleCallback> &call

	Return<StatusCode> VehicleHalManager::unsubscribe(const sp<IVehicleCallback>& callback,		Return<StatusCode> VehicleHalManager::unsubscribe(const sp<IVehicleCallback>& callback,
	int32_t propId) {		int32_t propId) {
	mSubscriptionManager.unsubscribe(callback, propId);		mSubscriptionManager.unsubscribe(getClientId(callback), propId);
	return StatusCode::OK;		return StatusCode::OK;
	}		}

	@@ -341,6 +343,18 @@ void VehicleHalManager::onAllClientsUnsubscribed(int32_t propertyId) {
	mHal->unsubscribe(propertyId);		mHal->unsubscribe(propertyId);
	}		}

			ClientId VehicleHalManager::getClientId(const sp<IVehicleCallback>& callback) {
			//TODO(b/32172906): rework this to get some kind of unique id for callback interface when this
			// feature is ready in HIDL.

			if (callback->isRemote()) {
			BpHwVehicleCallback* hwCallback = static_cast<BpHwVehicleCallback*>(callback.get());
			return static_cast<ClientId>(reinterpret_cast<intptr_t>(hwCallback->onAsBinder()));
			} else {
			return static_cast<ClientId>(reinterpret_cast<intptr_t>(callback.get()));
			}
			}

	} // namespace V2_0		} // namespace V2_0
	} // namespace vehicle		} // namespace vehicle
	} // namespace automotive		} // namespace automotive

automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp

+22 −16

Original line number	Original line	Diff line number	Diff line
	@@ -119,8 +119,10 @@ private:

	TEST_F(SubscriptionManagerTest, multipleClients) {		TEST_F(SubscriptionManagerTest, multipleClients) {
	std::list<SubscribeOptions> updatedOptions;		std::list<SubscribeOptions> updatedOptions;
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));		ASSERT_EQ(StatusCode::OK,
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb2, subscrToProp1, &updatedOptions));		manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
			ASSERT_EQ(StatusCode::OK,
			manager.addOrUpdateSubscription(2, cb2, subscrToProp1, &updatedOptions));

	auto clients = manager.getSubscribedClients(		auto clients = manager.getSubscribedClients(
	PROP1,		PROP1,
	@@ -132,7 +134,8 @@ TEST_F(SubscriptionManagerTest, multipleClients) {

	TEST_F(SubscriptionManagerTest, negativeCases) {		TEST_F(SubscriptionManagerTest, negativeCases) {
	std::list<SubscribeOptions> updatedOptions;		std::list<SubscribeOptions> updatedOptions;
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));		ASSERT_EQ(StatusCode::OK,
			manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));

	// Wrong zone		// Wrong zone
	auto clients = manager.getSubscribedClients(		auto clients = manager.getSubscribedClients(
	@@ -158,7 +161,8 @@ TEST_F(SubscriptionManagerTest, negativeCases) {

	TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {		TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {
	std::list<SubscribeOptions> updatedOptions;		std::list<SubscribeOptions> updatedOptions;
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));		ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(1, cb1, subscrToProp1,
			&updatedOptions));

	auto clients = manager.getSubscribedClients(		auto clients = manager.getSubscribedClients(
	PROP1,		PROP1,
	@@ -169,7 +173,7 @@ TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {

	// Same property, but different zone, to make sure we didn't unsubscribe		// Same property, but different zone, to make sure we didn't unsubscribe
	// from previous zone.		// from previous zone.
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, {		ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(1, cb1, {
	SubscribeOptions {		SubscribeOptions {
	.propId = PROP1,		.propId = PROP1,
	.vehicleAreas = toInt(VehicleAreaZone::ROW_2),		.vehicleAreas = toInt(VehicleAreaZone::ROW_2),
	@@ -190,15 +194,17 @@ TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {

	TEST_F(SubscriptionManagerTest, unsubscribe) {		TEST_F(SubscriptionManagerTest, unsubscribe) {
	std::list<SubscribeOptions> updatedOptions;		std::list<SubscribeOptions> updatedOptions;
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));		ASSERT_EQ(StatusCode::OK,
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb2, subscrToProp2, &updatedOptions));		manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
	ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb3, subscrToProp1and2,		ASSERT_EQ(StatusCode::OK,
	&updatedOptions));		manager.addOrUpdateSubscription(2, cb2, subscrToProp2, &updatedOptions));
			ASSERT_EQ(StatusCode::OK,
			manager.addOrUpdateSubscription(3, cb3, subscrToProp1and2, &updatedOptions));

	ASSERT_ALL_EXISTS({ cb1, cb3 }, extractCallbacks(clientsToProp1()));		ASSERT_ALL_EXISTS({ cb1, cb3 }, extractCallbacks(clientsToProp1()));
	ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));		ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));

	manager.unsubscribe(cb1, PROP1);		manager.unsubscribe(1, PROP1);
	assertOnPropertyUnsubscribedNotCalled();		assertOnPropertyUnsubscribedNotCalled();
	ASSERT_ALL_EXISTS({cb3}, extractCallbacks(clientsToProp1()));		ASSERT_ALL_EXISTS({cb3}, extractCallbacks(clientsToProp1()));

	@@ -206,20 +212,20 @@ TEST_F(SubscriptionManagerTest, unsubscribe) {
	ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));		ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));

	// No one subscribed to PROP1, subscription for PROP2 is not affected.		// No one subscribed to PROP1, subscription for PROP2 is not affected.
	manager.unsubscribe(cb3, PROP1);		manager.unsubscribe(3, PROP1);
	assertLastUnsubscribedProperty(PROP1);		assertLastUnsubscribedProperty(PROP1);
	ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));		ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));

	manager.unsubscribe(cb3, PROP2);		manager.unsubscribe(3, PROP2);
	assertOnPropertyUnsubscribedNotCalled();		assertOnPropertyUnsubscribedNotCalled();
	ASSERT_ALL_EXISTS({cb2}, extractCallbacks(clientsToProp2()));		ASSERT_ALL_EXISTS({cb2}, extractCallbacks(clientsToProp2()));

	// The last client unsubscribed from this property.		// The last client unsubscribed from this property.
	manager.unsubscribe(cb2, PROP2);		manager.unsubscribe(2, PROP2);
	assertLastUnsubscribedProperty(PROP2);		assertLastUnsubscribedProperty(PROP2);

	// No one subscribed anymore		// No one subscribed anymore
	manager.unsubscribe(cb1, PROP1);		manager.unsubscribe(1, PROP1);
	assertLastUnsubscribedProperty(PROP1);		assertLastUnsubscribedProperty(PROP1);
	}		}