Add subscribe/unsubscribe to IVehicleHardware.

These two functions replaces updateSampleRate. Previously
updateSampleRate will be called when a continuous property is
subscribed/unsubscribed. However, IVehicleHardware layer does not
know when an on-change property is subscribed/unsubscribed. This CL
introduces two new functions to notify IVehicleHardware when any
properties are subscribed/unsubscribed.

Test: atest DefaultVehicleHalTest
Bug: 306262618
Change-Id: I8d32d1eb919036015b5082f74e259bcd4b1bd29e
diff --git a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
index 3fecbd9..b813b11 100644
--- a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
@@ -82,35 +82,6 @@
             const std::vector<aidl::android::hardware::automotive::vehicle::GetValueRequest>&
                     requests) const = 0;
 
-    // Update the sampling rate for the specified property and the specified areaId (0 for global
-    // property) if server supports it. The property must be a continuous property.
-    // {@code sampleRate} means that for this specific property, the server must generate at least
-    // this many OnPropertyChange events per seconds.
-    // A sampleRate of 0 means the property is no longer subscribed and server does not need to
-    // generate any onPropertyEvent for this property.
-    // This would be called if sample rate is updated for a subscriber, a new subscriber is added
-    // or an existing subscriber is removed. For example:
-    // 1. We have no subscriber for speed.
-    // 2. A new subscriber is subscribing speed for 10 times/s, updsateSampleRate would be called
-    //    with sampleRate as 10. The impl is now polling vehicle speed from bus 10 times/s.
-    // 3. A new subscriber is subscribing speed for 5 times/s, because it is less than 10
-    //    times/sec, updateSampleRate would not be called.
-    // 4. The initial subscriber is removed, updateSampleRate would be called with sampleRate as
-    //    5, because now it only needs to report event 5times/sec. The impl can now poll vehicle
-    //    speed 5 times/s. If the impl is still polling at 10 times/s, that is okay as long as
-    //    the polling rate is larger than 5times/s. DefaultVehicleHal would ignore the additional
-    //    events.
-    // 5. The second subscriber is removed, updateSampleRate would be called with sampleRate as 0.
-    //    The impl can optionally disable the polling for vehicle speed.
-    //
-    // If the impl is always polling at {@code maxSampleRate} as specified in config, then this
-    // function can be a no-op.
-    virtual aidl::android::hardware::automotive::vehicle::StatusCode updateSampleRate(
-            [[maybe_unused]] int32_t propId, [[maybe_unused]] int32_t areaId,
-            [[maybe_unused]] float sampleRate) {
-        return aidl::android::hardware::automotive::vehicle::StatusCode::OK;
-    }
-
     // Dump debug information in the server.
     virtual DumpResult dump(const std::vector<std::string>& options) = 0;
 
@@ -145,6 +116,85 @@
         // By default batching is disabled.
         return std::chrono::nanoseconds(0);
     }
+
+    // A [propId, areaId] is newly subscribed or the update rate is changed.
+    //
+    // The 'options' contains the property ID, area ID and sample rate in Hz.
+    //
+    // For continuous property, the sample rate is never 0 and indicates the new sample rate (or
+    // the initial sample rate if this property was not subscribed before).
+    //
+    // For on-change property, the sample rate is always 0 and must be ignored.
+    //
+    // A subscription from VHAL client might not necessarily trigger this function.
+    // DefaultVehicleHal will aggregate all the subscriptions from all the clients and notify
+    // IVehicleHardware if new subscriptions are required or sample rate is updated.
+    //
+    // For example:
+    // 1. VHAL initially have no subscriber for speed.
+    // 2. A new subscriber is subscribing speed for 10 times/s, 'subscribe' is called
+    //    with sampleRate as 10. The impl is now polling vehicle speed from bus 10 times/s.
+    // 3. A new subscriber is subscribing speed for 5 times/s, because it is less than 10
+    //    times/sec, 'subscribe' is not called.
+    // 4. The initial subscriber is removed, 'subscribe' is called with sampleRate as
+    //    5, because now it only needs to report event 5times/sec. The impl can now poll vehicle
+    //    speed 5 times/s. If the impl is still polling at 10 times/s, that is okay as long as
+    //    the polling rate is larger than 5times/s. DefaultVehicleHal would ignore the additional
+    //    events.
+    // 5. The second subscriber is removed, 'unsubscribe' is called.
+    //    The impl can optionally disable the polling for vehicle speed.
+    //
+    // It is recommended to only deliver the subscribed property events to DefaultVehicleHal to
+    // improve performance. However, even if unsubscribed property events are delivered, they
+    // will be filtered out by DefaultVehicleHal.
+    //
+    // For continuous property, if the impl is always polling at {@code maxSampleRate} as specified
+    // in config, then this function can be a no-op.
+    //
+    // For on-change property, if the impl is always subscribing to all on-change properties, then
+    // this function can be no-op.
+    virtual aidl::android::hardware::automotive::vehicle::StatusCode subscribe(
+            [[maybe_unused]] aidl::android::hardware::automotive::vehicle::SubscribeOptions
+                    options) {
+        return aidl::android::hardware::automotive::vehicle::StatusCode::OK;
+    }
+
+    // A [propId, areaId] is unsubscribed. This applies for both continuous or on-change property.
+    virtual aidl::android::hardware::automotive::vehicle::StatusCode unsubscribe(
+            [[maybe_unused]] int32_t propId, [[maybe_unused]] int32_t areaId) {
+        return aidl::android::hardware::automotive::vehicle::StatusCode::OK;
+    }
+
+    // This function is deprecated, subscribe/unsubscribe should be used instead.
+    //
+    // Update the sampling rate for the specified property and the specified areaId (0 for global
+    // property) if server supports it. The property must be a continuous property.
+    // {@code sampleRate} means that for this specific property, the server must generate at least
+    // this many OnPropertyChange events per seconds.
+    // A sampleRate of 0 means the property is no longer subscribed and server does not need to
+    // generate any onPropertyEvent for this property.
+    // This would be called if sample rate is updated for a subscriber, a new subscriber is added
+    // or an existing subscriber is removed. For example:
+    // 1. We have no subscriber for speed.
+    // 2. A new subscriber is subscribing speed for 10 times/s, updateSampleRate would be called
+    //    with sampleRate as 10. The impl is now polling vehicle speed from bus 10 times/s.
+    // 3. A new subscriber is subscribing speed for 5 times/s, because it is less than 10
+    //    times/sec, updateSampleRate would not be called.
+    // 4. The initial subscriber is removed, updateSampleRate would be called with sampleRate as
+    //    5, because now it only needs to report event 5times/sec. The impl can now poll vehicle
+    //    speed 5 times/s. If the impl is still polling at 10 times/s, that is okay as long as
+    //    the polling rate is larger than 5times/s. DefaultVehicleHal would ignore the additional
+    //    events.
+    // 5. The second subscriber is removed, updateSampleRate would be called with sampleRate as 0.
+    //    The impl can optionally disable the polling for vehicle speed.
+    //
+    // If the impl is always polling at {@code maxSampleRate} as specified in config, then this
+    // function can be a no-op.
+    virtual aidl::android::hardware::automotive::vehicle::StatusCode updateSampleRate(
+            [[maybe_unused]] int32_t propId, [[maybe_unused]] int32_t areaId,
+            [[maybe_unused]] float sampleRate) {
+        return aidl::android::hardware::automotive::vehicle::StatusCode::OK;
+    }
 };
 
 }  // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
index c94bad6..546421e 100644
--- a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
+++ b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
@@ -329,6 +329,11 @@
     }
 };
 
+inline std::string propIdToString(int32_t propId) {
+    return toString(
+            static_cast<aidl::android::hardware::automotive::vehicle::VehicleProperty>(propId));
+}
+
 }  // namespace vehicle
 }  // namespace automotive
 }  // namespace hardware
diff --git a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
index b91895e..057da35 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
@@ -119,7 +119,7 @@
     mutable std::mutex mLock;
     std::unordered_map<PropIdAreaId, std::unordered_map<ClientIdType, CallbackType>,
                        PropIdAreaIdHash>
-            mClientsByPropIdArea GUARDED_BY(mLock);
+            mClientsByPropIdAreaId GUARDED_BY(mLock);
     std::unordered_map<ClientIdType, std::unordered_set<PropIdAreaId, PropIdAreaIdHash>>
             mSubscribedPropsByClient GUARDED_BY(mLock);
     std::unordered_map<PropIdAreaId, ContSubConfigs, PropIdAreaIdHash> mContSubConfigsByPropIdArea
@@ -128,12 +128,21 @@
     VhalResult<void> addContinuousSubscriberLocked(const ClientIdType& clientId,
                                                    const PropIdAreaId& propIdAreaId,
                                                    float sampleRateHz) REQUIRES(mLock);
+    VhalResult<void> addOnChangeSubscriberLocked(const PropIdAreaId& propIdAreaId) REQUIRES(mLock);
+    // Removes the subscription client for the continuous [propId, areaId].
     VhalResult<void> removeContinuousSubscriberLocked(const ClientIdType& clientId,
                                                       const PropIdAreaId& propIdAreaId)
             REQUIRES(mLock);
+    // Removes one subscription client for the on-change [propId, areaId].
+    VhalResult<void> removeOnChangeSubscriberLocked(const PropIdAreaId& propIdAreaId)
+            REQUIRES(mLock);
 
-    VhalResult<void> updateContSubConfigs(const PropIdAreaId& PropIdAreaId,
-                                          const ContSubConfigs& newConfig) REQUIRES(mLock);
+    VhalResult<void> updateContSubConfigsLocked(const PropIdAreaId& PropIdAreaId,
+                                                const ContSubConfigs& newConfig) REQUIRES(mLock);
+
+    VhalResult<void> unsubscribePropIdAreaIdLocked(SubscriptionManager::ClientIdType clientId,
+                                                   const PropIdAreaId& propIdAreaId)
+            REQUIRES(mLock);
 
     // Checks whether the manager is empty. For testing purpose.
     bool isEmpty();
diff --git a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
index b3c2693..a7c797b 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
@@ -16,6 +16,7 @@
 
 #include "SubscriptionManager.h"
 
+#include <VehicleUtils.h>
 #include <android-base/stringprintf.h>
 #include <utils/Log.h>
 #include <utils/SystemClock.h>
@@ -29,10 +30,6 @@
 
 namespace {
 
-constexpr float ONE_SECOND_IN_NANO = 1'000'000'000.;
-
-}  // namespace
-
 using ::aidl::android::hardware::automotive::vehicle::IVehicleCallback;
 using ::aidl::android::hardware::automotive::vehicle::StatusCode;
 using ::aidl::android::hardware::automotive::vehicle::SubscribeOptions;
@@ -43,13 +40,26 @@
 using ::android::base::StringPrintf;
 using ::ndk::ScopedAStatus;
 
+constexpr float ONE_SECOND_IN_NANOS = 1'000'000'000.;
+
+SubscribeOptions newSubscribeOptions(int32_t propId, int32_t areaId, float sampleRateHz) {
+    SubscribeOptions subscribedOptions;
+    subscribedOptions.propId = propId;
+    subscribedOptions.areaIds = {areaId};
+    subscribedOptions.sampleRate = sampleRateHz;
+
+    return subscribedOptions;
+}
+
+}  // namespace
+
 SubscriptionManager::SubscriptionManager(IVehicleHardware* vehicleHardware)
     : mVehicleHardware(vehicleHardware) {}
 
 SubscriptionManager::~SubscriptionManager() {
     std::scoped_lock<std::mutex> lockGuard(mLock);
 
-    mClientsByPropIdArea.clear();
+    mClientsByPropIdAreaId.clear();
     mSubscribedPropsByClient.clear();
 }
 
@@ -62,10 +72,10 @@
     if (sampleRateHz <= 0) {
         return Error() << "invalid sample rate, must be a positive number";
     }
-    if (sampleRateHz <= (ONE_SECOND_IN_NANO / static_cast<float>(INT64_MAX))) {
+    if (sampleRateHz <= (ONE_SECOND_IN_NANOS / static_cast<float>(INT64_MAX))) {
         return Error() << "invalid sample rate: " << sampleRateHz << ", too small";
     }
-    intervalNanos = static_cast<int64_t>(ONE_SECOND_IN_NANO / sampleRateHz);
+    intervalNanos = static_cast<int64_t>(ONE_SECOND_IN_NANOS / sampleRateHz);
     return intervalNanos;
 }
 
@@ -95,12 +105,31 @@
     return mMaxSampleRateHz;
 }
 
+VhalResult<void> SubscriptionManager::addOnChangeSubscriberLocked(
+        const PropIdAreaId& propIdAreaId) {
+    if (mClientsByPropIdAreaId.find(propIdAreaId) != mClientsByPropIdAreaId.end()) {
+        // This propId, areaId is already subscribed, ignore the request.
+        return {};
+    }
+
+    int32_t propId = propIdAreaId.propId;
+    int32_t areaId = propIdAreaId.areaId;
+    if (auto status = mVehicleHardware->subscribe(
+                newSubscribeOptions(propId, areaId, /*updateRateHz=*/0));
+        status != StatusCode::OK) {
+        return StatusError(status)
+               << StringPrintf("failed subscribe for prop: %s, areaId: %" PRId32,
+                               propIdToString(propId).c_str(), areaId);
+    }
+    return {};
+}
+
 VhalResult<void> SubscriptionManager::addContinuousSubscriberLocked(
         const ClientIdType& clientId, const PropIdAreaId& propIdAreaId, float sampleRateHz) {
     // Make a copy so that we don't modify 'mContSubConfigsByPropIdArea' on failure cases.
     ContSubConfigs newConfig = mContSubConfigsByPropIdArea[propIdAreaId];
     newConfig.addClient(clientId, sampleRateHz);
-    return updateContSubConfigs(propIdAreaId, newConfig);
+    return updateContSubConfigsLocked(propIdAreaId, newConfig);
 }
 
 VhalResult<void> SubscriptionManager::removeContinuousSubscriberLocked(
@@ -108,11 +137,28 @@
     // Make a copy so that we don't modify 'mContSubConfigsByPropIdArea' on failure cases.
     ContSubConfigs newConfig = mContSubConfigsByPropIdArea[propIdAreaId];
     newConfig.removeClient(clientId);
-    return updateContSubConfigs(propIdAreaId, newConfig);
+    return updateContSubConfigsLocked(propIdAreaId, newConfig);
 }
 
-VhalResult<void> SubscriptionManager::updateContSubConfigs(const PropIdAreaId& propIdAreaId,
-                                                           const ContSubConfigs& newConfig) {
+VhalResult<void> SubscriptionManager::removeOnChangeSubscriberLocked(
+        const PropIdAreaId& propIdAreaId) {
+    if (mClientsByPropIdAreaId[propIdAreaId].size() > 1) {
+        // After unsubscribing this client, there is still client subscribed, so do nothing.
+        return {};
+    }
+
+    int32_t propId = propIdAreaId.propId;
+    int32_t areaId = propIdAreaId.areaId;
+    if (auto status = mVehicleHardware->unsubscribe(propId, areaId); status != StatusCode::OK) {
+        return StatusError(status)
+               << StringPrintf("failed unsubscribe for prop: %s, areaId: %" PRId32,
+                               propIdToString(propId).c_str(), areaId);
+    }
+    return {};
+}
+
+VhalResult<void> SubscriptionManager::updateContSubConfigsLocked(const PropIdAreaId& propIdAreaId,
+                                                                 const ContSubConfigs& newConfig) {
     if (newConfig.getMaxSampleRateHz() ==
         mContSubConfigsByPropIdArea[propIdAreaId].getMaxSampleRateHz()) {
         mContSubConfigsByPropIdArea[propIdAreaId] = newConfig;
@@ -123,10 +169,27 @@
     int32_t areaId = propIdAreaId.areaId;
     if (auto status = mVehicleHardware->updateSampleRate(propId, areaId, newRateHz);
         status != StatusCode::OK) {
-        return StatusError(status) << StringPrintf("failed to update sample rate for prop: %" PRId32
-                                                   ", area"
-                                                   ": %" PRId32 ", sample rate: %f HZ",
-                                                   propId, areaId, newRateHz);
+        return StatusError(status)
+               << StringPrintf("failed to update sample rate for prop: %s, areaId: %" PRId32
+                               ", sample rate: %f HZ",
+                               propIdToString(propId).c_str(), areaId, newRateHz);
+    }
+    if (newRateHz != 0) {
+        if (auto status =
+                    mVehicleHardware->subscribe(newSubscribeOptions(propId, areaId, newRateHz));
+            status != StatusCode::OK) {
+            return StatusError(status) << StringPrintf(
+                           "failed subscribe for prop: %s, areaId"
+                           ": %" PRId32 ", sample rate: %f HZ",
+                           propIdToString(propId).c_str(), areaId, newRateHz);
+        }
+    } else {
+        if (auto status = mVehicleHardware->unsubscribe(propId, areaId); status != StatusCode::OK) {
+            return StatusError(status) << StringPrintf(
+                           "failed unsubscribe for prop: %s, areaId"
+                           ": %" PRId32,
+                           propIdToString(propId).c_str(), areaId);
+        }
     }
     mContSubConfigsByPropIdArea[propIdAreaId] = newConfig;
     return {};
@@ -163,21 +226,53 @@
                     .propId = propId,
                     .areaId = areaId,
             };
+            VhalResult<void> result;
             if (isContinuousProperty) {
-                if (auto result = addContinuousSubscriberLocked(clientId, propIdAreaId,
-                                                                option.sampleRate);
-                    !result.ok()) {
-                    return result;
-                }
+                result = addContinuousSubscriberLocked(clientId, propIdAreaId, option.sampleRate);
+            } else {
+                result = addOnChangeSubscriberLocked(propIdAreaId);
+            }
+
+            if (!result.ok()) {
+                return result;
             }
 
             mSubscribedPropsByClient[clientId].insert(propIdAreaId);
-            mClientsByPropIdArea[propIdAreaId][clientId] = callback;
+            mClientsByPropIdAreaId[propIdAreaId][clientId] = callback;
         }
     }
     return {};
 }
 
+VhalResult<void> SubscriptionManager::unsubscribePropIdAreaIdLocked(
+        SubscriptionManager::ClientIdType clientId, const PropIdAreaId& propIdAreaId) {
+    if (mContSubConfigsByPropIdArea.find(propIdAreaId) != mContSubConfigsByPropIdArea.end()) {
+        // This is a subscribed continuous property.
+        if (auto result = removeContinuousSubscriberLocked(clientId, propIdAreaId); !result.ok()) {
+            return result;
+        }
+    } else {
+        if (mClientsByPropIdAreaId.find(propIdAreaId) == mClientsByPropIdAreaId.end()) {
+            ALOGW("Unsubscribe: The property: %s, areaId: %" PRId32
+                  " was not previously subscribed, do nothing",
+                  propIdToString(propIdAreaId.propId).c_str(), propIdAreaId.areaId);
+            return {};
+        }
+        // This is an on-change property.
+        if (auto result = removeOnChangeSubscriberLocked(propIdAreaId); !result.ok()) {
+            return result;
+        }
+    }
+
+    auto& clients = mClientsByPropIdAreaId[propIdAreaId];
+    clients.erase(clientId);
+    if (clients.empty()) {
+        mClientsByPropIdAreaId.erase(propIdAreaId);
+        mContSubConfigsByPropIdArea.erase(propIdAreaId);
+    }
+    return {};
+}
+
 VhalResult<void> SubscriptionManager::unsubscribe(SubscriptionManager::ClientIdType clientId,
                                                   const std::vector<int32_t>& propIds) {
     std::scoped_lock<std::mutex> lockGuard(mLock);
@@ -186,39 +281,27 @@
         return StatusError(StatusCode::INVALID_ARG)
                << "No property was subscribed for the callback";
     }
-    std::unordered_set<int32_t> subscribedPropIds;
-    for (auto const& propIdAreaId : mSubscribedPropsByClient[clientId]) {
-        subscribedPropIds.insert(propIdAreaId.propId);
-    }
 
+    std::vector<PropIdAreaId> propIdAreaIdsToUnsubscribe;
+    std::unordered_set<int32_t> propIdSet;
     for (int32_t propId : propIds) {
-        if (subscribedPropIds.find(propId) == subscribedPropIds.end()) {
-            return StatusError(StatusCode::INVALID_ARG)
-                   << "property ID: " << propId << " is not subscribed";
+        propIdSet.insert(propId);
+    }
+    auto& subscribedPropIdsAreaIds = mSubscribedPropsByClient[clientId];
+    for (const auto& propIdAreaId : subscribedPropIdsAreaIds) {
+        if (propIdSet.find(propIdAreaId.propId) != propIdSet.end()) {
+            propIdAreaIdsToUnsubscribe.push_back(propIdAreaId);
         }
     }
 
-    auto& propIdAreaIds = mSubscribedPropsByClient[clientId];
-    auto it = propIdAreaIds.begin();
-    while (it != propIdAreaIds.end()) {
-        int32_t propId = it->propId;
-        if (std::find(propIds.begin(), propIds.end(), propId) != propIds.end()) {
-            if (auto result = removeContinuousSubscriberLocked(clientId, *it); !result.ok()) {
-                return result;
-            }
-
-            auto& clients = mClientsByPropIdArea[*it];
-            clients.erase(clientId);
-            if (clients.empty()) {
-                mClientsByPropIdArea.erase(*it);
-                mContSubConfigsByPropIdArea.erase(*it);
-            }
-            it = propIdAreaIds.erase(it);
-        } else {
-            it++;
+    for (const auto& propIdAreaId : propIdAreaIdsToUnsubscribe) {
+        if (auto result = unsubscribePropIdAreaIdLocked(clientId, propIdAreaId); !result.ok()) {
+            return result;
         }
+        subscribedPropIdsAreaIds.erase(propIdAreaId);
     }
-    if (propIdAreaIds.empty()) {
+
+    if (subscribedPropIdsAreaIds.empty()) {
         mSubscribedPropsByClient.erase(clientId);
     }
     return {};
@@ -233,16 +316,9 @@
 
     auto& subscriptions = mSubscribedPropsByClient[clientId];
     for (auto const& propIdAreaId : subscriptions) {
-        if (auto result = removeContinuousSubscriberLocked(clientId, propIdAreaId); !result.ok()) {
+        if (auto result = unsubscribePropIdAreaIdLocked(clientId, propIdAreaId); !result.ok()) {
             return result;
         }
-
-        auto& clients = mClientsByPropIdArea[propIdAreaId];
-        clients.erase(clientId);
-        if (clients.empty()) {
-            mClientsByPropIdArea.erase(propIdAreaId);
-            mContSubConfigsByPropIdArea.erase(propIdAreaId);
-        }
     }
     mSubscribedPropsByClient.erase(clientId);
     return {};
@@ -258,11 +334,11 @@
                 .propId = value.prop,
                 .areaId = value.areaId,
         };
-        if (mClientsByPropIdArea.find(propIdAreaId) == mClientsByPropIdArea.end()) {
+        if (mClientsByPropIdAreaId.find(propIdAreaId) == mClientsByPropIdAreaId.end()) {
             continue;
         }
 
-        for (const auto& [_, client] : mClientsByPropIdArea[propIdAreaId]) {
+        for (const auto& [_, client] : mClientsByPropIdAreaId[propIdAreaId]) {
             clients[client].push_back(value);
         }
     }
@@ -280,11 +356,11 @@
                 .propId = errorEvent.propId,
                 .areaId = errorEvent.areaId,
         };
-        if (mClientsByPropIdArea.find(propIdAreaId) == mClientsByPropIdArea.end()) {
+        if (mClientsByPropIdAreaId.find(propIdAreaId) == mClientsByPropIdAreaId.end()) {
             continue;
         }
 
-        for (const auto& [_, client] : mClientsByPropIdArea[propIdAreaId]) {
+        for (const auto& [_, client] : mClientsByPropIdAreaId[propIdAreaId]) {
             clients[client].push_back({
                     .propId = errorEvent.propId,
                     .areaId = errorEvent.areaId,
@@ -297,7 +373,7 @@
 
 bool SubscriptionManager::isEmpty() {
     std::scoped_lock<std::mutex> lockGuard(mLock);
-    return mSubscribedPropsByClient.empty() && mClientsByPropIdArea.empty();
+    return mSubscribedPropsByClient.empty() && mClientsByPropIdAreaId.empty();
 }
 
 size_t SubscriptionManager::countClients() {
diff --git a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp
index 3fae596..b64c0d7 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp
@@ -29,6 +29,7 @@
 using ::aidl::android::hardware::automotive::vehicle::SetValueRequest;
 using ::aidl::android::hardware::automotive::vehicle::SetValueResult;
 using ::aidl::android::hardware::automotive::vehicle::StatusCode;
+using ::aidl::android::hardware::automotive::vehicle::SubscribeOptions;
 using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
 using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
 
@@ -88,7 +89,26 @@
     return StatusCode::OK;
 }
 
-StatusCode MockVehicleHardware::updateSampleRate(int32_t propId, int32_t areaId, float sampleRate) {
+StatusCode MockVehicleHardware::subscribe(SubscribeOptions options) {
+    for (int32_t areaId : options.areaIds) {
+        if (auto status = subscribePropIdAreaId(options.propId, areaId, options.sampleRate);
+            status != StatusCode::OK) {
+            return status;
+        }
+    }
+    return StatusCode::OK;
+}
+
+StatusCode MockVehicleHardware::subscribePropIdAreaId(int32_t propId, int32_t areaId,
+                                                      float sampleRateHz) {
+    if (sampleRateHz == 0) {
+        // on-change property.
+        std::scoped_lock<std::mutex> lockGuard(mLock);
+        mSubOnChangePropIdAreaIds.insert(std::pair<int32_t, int32_t>(propId, areaId));
+        return StatusCode::OK;
+    }
+
+    // continuous property.
     std::shared_ptr<std::function<void()>> action;
 
     {
@@ -97,9 +117,6 @@
             // Remove the previous action register for this [propId, areaId].
             mRecurrentTimer->unregisterTimerCallback(mRecurrentActions[propId][areaId]);
         }
-        if (sampleRate == 0) {
-            return StatusCode::OK;
-        }
 
         // We are sure 'propertyChangeCallback' would be alive because we would unregister timer
         // before destroying 'this' which owns mPropertyChangeCallback.
@@ -107,8 +124,8 @@
         action = std::make_shared<std::function<void()>>([propertyChangeCallback, propId, areaId] {
             std::vector<VehiclePropValue> values = {
                     {
-                            .prop = propId,
                             .areaId = areaId,
+                            .prop = propId,
                     },
             };
             (*propertyChangeCallback)(values);
@@ -119,11 +136,45 @@
 
     // In mock implementation, we generate a new property change event for this property at sample
     // rate.
-    int64_t interval = static_cast<int64_t>(1'000'000'000. / sampleRate);
+    int64_t interval = static_cast<int64_t>(1'000'000'000. / sampleRateHz);
     mRecurrentTimer->registerTimerCallback(interval, action);
     return StatusCode::OK;
 }
 
+StatusCode MockVehicleHardware::unsubscribe(int32_t propId, int32_t areaId) {
+    std::scoped_lock<std::mutex> lockGuard(mLock);
+    // For on-change property.
+    mSubOnChangePropIdAreaIds.erase(std::make_pair(propId, areaId));
+    // for continuous property.
+    if (mRecurrentActions[propId][areaId] != nullptr) {
+        // Remove the previous action register for this [propId, areaId].
+        mRecurrentTimer->unregisterTimerCallback(mRecurrentActions[propId][areaId]);
+        mRecurrentActions[propId].erase(areaId);
+        if (mRecurrentActions[propId].empty()) {
+            mRecurrentActions.erase(propId);
+        }
+    }
+    return StatusCode::OK;
+}
+
+std::set<std::pair<int32_t, int32_t>> MockVehicleHardware::getSubscribedOnChangePropIdAreaIds() {
+    std::scoped_lock<std::mutex> lockGuard(mLock);
+    std::set<std::pair<int32_t, int32_t>> propIdAreaIds;
+    propIdAreaIds = mSubOnChangePropIdAreaIds;
+    return propIdAreaIds;
+}
+
+std::set<std::pair<int32_t, int32_t>> MockVehicleHardware::getSubscribedContinuousPropIdAreaIds() {
+    std::scoped_lock<std::mutex> lockGuard(mLock);
+    std::set<std::pair<int32_t, int32_t>> propIdAreaIds;
+    for (const auto& [propId, actionByAreaId] : mRecurrentActions) {
+        for (const auto& [areaId, _] : actionByAreaId) {
+            propIdAreaIds.insert(std::make_pair(propId, areaId));
+        }
+    }
+    return propIdAreaIds;
+}
+
 void MockVehicleHardware::registerOnPropertyChangeEvent(
         std::unique_ptr<const PropertyChangeCallback> callback) {
     std::scoped_lock<std::mutex> lockGuard(mLock);
diff --git a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h
index 3ce18c5..e0d2d66 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h
@@ -29,6 +29,7 @@
 #include <list>
 #include <memory>
 #include <mutex>
+#include <set>
 #include <thread>
 #include <unordered_map>
 #include <vector>
@@ -59,8 +60,10 @@
     void registerOnPropertyChangeEvent(
             std::unique_ptr<const PropertyChangeCallback> callback) override;
     void registerOnPropertySetErrorEvent(std::unique_ptr<const PropertySetErrorCallback>) override;
-    aidl::android::hardware::automotive::vehicle::StatusCode updateSampleRate(
-            int32_t propId, int32_t areaId, float sampleRate) override;
+    aidl::android::hardware::automotive::vehicle::StatusCode subscribe(
+            aidl::android::hardware::automotive::vehicle::SubscribeOptions options) override;
+    aidl::android::hardware::automotive::vehicle::StatusCode unsubscribe(int32_t propId,
+                                                                         int32_t areaId) override;
     std::chrono::nanoseconds getPropertyOnChangeEventBatchingWindow() override;
 
     // Test functions.
@@ -90,6 +93,9 @@
     void sendOnPropertySetErrorEvent(const std::vector<SetValueErrorEvent>& errorEvents);
     void setPropertyOnChangeEventBatchingWindow(std::chrono::nanoseconds window);
 
+    std::set<std::pair<int32_t, int32_t>> getSubscribedOnChangePropIdAreaIds();
+    std::set<std::pair<int32_t, int32_t>> getSubscribedContinuousPropIdAreaIds();
+
   private:
     mutable std::mutex mLock;
     mutable std::condition_variable mCv;
@@ -114,6 +120,7 @@
             const std::vector<aidl::android::hardware::automotive::vehicle::GetValueRequest>&)>
             mGetValueResponder GUARDED_BY(mLock);
     std::chrono::nanoseconds mEventBatchingWindow GUARDED_BY(mLock) = std::chrono::nanoseconds(0);
+    std::set<std::pair<int32_t, int32_t>> mSubOnChangePropIdAreaIds GUARDED_BY(mLock);
 
     template <class ResultType>
     aidl::android::hardware::automotive::vehicle::StatusCode returnResponse(
@@ -126,6 +133,8 @@
             const std::vector<RequestType>& requests,
             std::list<std::vector<RequestType>>* storedRequests,
             std::list<std::vector<ResultType>>* storedResponses) const REQUIRES(mLock);
+    aidl::android::hardware::automotive::vehicle::StatusCode subscribePropIdAreaId(
+            int32_t propId, int32_t areaId, float sampleRateHz);
 
     DumpResult mDumpResult;
 
diff --git a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
index 5464304..049ca8b 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
@@ -114,6 +114,8 @@
 
     void clearEvents() { return getCallback()->clearEvents(); }
 
+    std::shared_ptr<MockVehicleHardware> getHardware() { return mHardware; }
+
   private:
     std::unique_ptr<SubscriptionManager> mManager;
     std::shared_ptr<PropertyCallback> mCallback;
@@ -132,6 +134,9 @@
     auto result = getManager()->subscribe(getCallbackClient(), options, true);
     ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
 
+    ASSERT_THAT(getHardware()->getSubscribedContinuousPropIdAreaIds(),
+                UnorderedElementsAre(std::pair<int32_t, int32_t>(0, 0)));
+
     std::this_thread::sleep_for(std::chrono::seconds(1));
 
     // Theoretically trigger 10 times, but check for at least 9 times to be stable.
@@ -240,6 +245,8 @@
     result = getManager()->unsubscribe(getCallbackClient()->asBinder().get());
     ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();
 
+    ASSERT_EQ(getHardware()->getSubscribedContinuousPropIdAreaIds().size(), 0u);
+
     // Wait for the last events to come.
     std::this_thread::sleep_for(std::chrono::milliseconds(100));
 
@@ -316,7 +323,7 @@
     EXPECT_TRUE(getEvents().empty());
 }
 
-TEST_F(SubscriptionManagerTest, testUnsubscribeFailure) {
+TEST_F(SubscriptionManagerTest, testUnsubscribeUnsubscribedPropId) {
     std::vector<SubscribeOptions> options = {
             {
                     .propId = 0,
@@ -334,14 +341,21 @@
     // Property ID: 2 was not subscribed.
     result = getManager()->unsubscribe(getCallbackClient()->asBinder().get(),
                                        std::vector<int32_t>({0, 1, 2}));
-    ASSERT_FALSE(result.ok()) << "unsubscribe an unsubscribed property must fail";
+    ASSERT_TRUE(result.ok()) << "unsubscribe an unsubscribed property must do nothing";
 
-    // Since property 0 and property 1 was not unsubscribed successfully, we should be able to
-    // unsubscribe them again.
-    result = getManager()->unsubscribe(getCallbackClient()->asBinder().get(),
-                                       std::vector<int32_t>({0, 1}));
-    ASSERT_TRUE(result.ok()) << "a failed unsubscription must not unsubscribe any properties"
-                             << result.error().message();
+    std::vector<VehiclePropValue> updatedValues = {
+            {
+                    .prop = 0,
+                    .areaId = 0,
+            },
+            {
+                    .prop = 1,
+                    .areaId = 0,
+            },
+    };
+    auto clients = getManager()->getSubscribedClients(std::vector<VehiclePropValue>(updatedValues));
+
+    ASSERT_EQ(clients.size(), 0u) << "all subscribed properties must be unsubscribed";
 }
 
 TEST_F(SubscriptionManagerTest, testSubscribeOnchange) {
@@ -370,6 +384,11 @@
     ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
     result = getManager()->subscribe(client2, options2, false);
     ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
+    ASSERT_THAT(getHardware()->getSubscribedOnChangePropIdAreaIds(),
+                UnorderedElementsAre(std::pair<int32_t, int32_t>(0, 0),
+                                     std::pair<int32_t, int32_t>(0, 1),
+                                     std::pair<int32_t, int32_t>(1, 0)));
+    ASSERT_EQ(getHardware()->getSubscribedContinuousPropIdAreaIds().size(), 0u);
 
     std::vector<VehiclePropValue> updatedValues = {
             {
@@ -483,6 +502,8 @@
     auto clients = getManager()->getSubscribedClients(std::vector<VehiclePropValue>(updatedValues));
 
     ASSERT_THAT(clients[getCallbackClient()], ElementsAre(updatedValues[1]));
+    ASSERT_THAT(getHardware()->getSubscribedOnChangePropIdAreaIds(),
+                UnorderedElementsAre(std::pair<int32_t, int32_t>(1, 0)));
 }
 
 TEST_F(SubscriptionManagerTest, testCheckSampleRateHzValid) {