Batching API batch retrieval changes

Allows one last batch of fixes, if available, be retrieved by HIDL
client after stop() is called. Also, if flush() is called, service
end will always report something, empty if no more fixes in the
batch.

Bug: 222222871
Bug: 222182505
Test: run cts -m CtsLocationFineTestCases -t
android.location.cts.fine.LocationManagerFineTest#testRequestFlush_Gnss
CRs-fixed: 28702232

Change-Id: I2481285c598c04be8b448c88da98756daa8a19d0

3 files changed, 88 insertions, 41 deletions

diff --git a/android/2.1/location_api/BatchingAPIClient.cpp b/android/2.1/location_api/BatchingAPIClient.cpp
index 00d2ed9..986d5c8 100644
--- a/android/2.1/location_api/BatchingAPIClient.cpp
+++ b/android/2.1/location_api/BatchingAPIClient.cpp
@@ -34,6 +34,7 @@
 #include <log_util.h>
 #include <loc_cfg.h>
 
+#include <thread>
 #include "LocationUtil.h"
 #include "BatchingAPIClient.h"
 
@@ -82,10 +83,10 @@ BatchingAPIClient::~BatchingAPIClient()
     LOC_LOGD("%s]: ()", __FUNCTION__);
 }
 
-int BatchingAPIClient::getBatchSize()
-{
-    LOC_LOGD("%s]: ()", __FUNCTION__);
-    return locAPIGetBatchSize();
+int BatchingAPIClient::getBatchSize() {
+    int batchSize = locAPIGetBatchSize();
+    LOC_LOGd("batchSize: %d", batchSize);
+    return batchSize;
 }
 
 void BatchingAPIClient::setCallbacks()
@@ -133,8 +134,10 @@ void BatchingAPIClient::gnssUpdateCallbacks_2_0(const sp<V2_0::IGnssBatchingCall
     }
 }
 
-int BatchingAPIClient::startSession(const IGnssBatching::Options& opts)
-{
+int BatchingAPIClient::startSession(const IGnssBatching::Options& opts) {
+    mMutex.lock();
+    mState = STARTED;
+    mMutex.unlock();
     LOC_LOGD("%s]: (%lld %d)", __FUNCTION__,
             static_cast<long long>(opts.periodNanos), static_cast<uint8_t>(opts.flags));
     int retVal = -1;
@@ -168,10 +171,13 @@ int BatchingAPIClient::updateSessionOptions(const IGnssBatching::Options& opts)
     return retVal;
 }
 
-int BatchingAPIClient::stopSession()
-{
+int BatchingAPIClient::stopSession() {
+    mMutex.lock();
+    mState = STOPPING;
+    mMutex.unlock();
     LOC_LOGD("%s]: ", __FUNCTION__);
     int retVal = -1;
+    locAPIGetBatchedLocations(mDefaultId, SIZE_MAX);
     if (locAPIStopSession(mDefaultId) == LOCATION_ERROR_SUCCESS) {
         retVal = 1;
     }
@@ -184,11 +190,16 @@ void BatchingAPIClient::getBatchedLocation(int last_n_locations)
     locAPIGetBatchedLocations(mDefaultId, last_n_locations);
 }
 
-void BatchingAPIClient::flushBatchedLocations()
-{
+void BatchingAPIClient::flushBatchedLocations() {
     LOC_LOGD("%s]: ()", __FUNCTION__);
-    locAPIGetBatchedLocations(mDefaultId, SIZE_MAX);
-}
+    uint32_t retVal = locAPIGetBatchedLocations(mDefaultId, SIZE_MAX);
+    // when flush a stopped session or one doesn't exist, just report an empty batch.
+    if (LOCATION_ERROR_ID_UNKNOWN == retVal) {
+        BatchingOptions opt = {};
+        ::std::thread thd(&BatchingAPIClient::onBatchingCb, this, 0, nullptr, opt);
+        thd.detach();
+    }
+ }
 
 void BatchingAPIClient::onCapabilitiesCb(LocationCapabilitiesMask capabilitiesMask)
 {
@@ -197,37 +208,68 @@ void BatchingAPIClient::onCapabilitiesCb(LocationCapabilitiesMask capabilitiesMa
 }
 
 void BatchingAPIClient::onBatchingCb(size_t count, Location* location,
-        BatchingOptions /*batchOptions*/)
-{
+        BatchingOptions /*batchOptions*/) {
+    bool processReport = false;
+    LOC_LOGd("(count: %zu)", count);
     mMutex.lock();
-    auto gnssBatchingCbIface(mGnssBatchingCbIface);
-    auto gnssBatchingCbIface_2_0(mGnssBatchingCbIface_2_0);
-    mMutex.unlock();
-
-    LOC_LOGD("%s]: (count: %zu)", __FUNCTION__, count);
-    if (gnssBatchingCbIface_2_0 != nullptr && count > 0) {
-        hidl_vec<V2_0::GnssLocation> locationVec;
-        locationVec.resize(count);
-        for (size_t i = 0; i < count; i++) {
-            convertGnssLocation(location[i], locationVec[i]);
-        }
-        auto r = gnssBatchingCbIface_2_0->gnssLocationBatchCb(locationVec);
-        if (!r.isOk()) {
-            LOC_LOGE("%s] Error from gnssLocationBatchCb 2_0 description=%s",
-                __func__, r.description().c_str());
-        }
-    } else if (gnssBatchingCbIface != nullptr && count > 0) {
-        hidl_vec<V1_0::GnssLocation> locationVec;
-        locationVec.resize(count);
-        for (size_t i = 0; i < count; i++) {
-            convertGnssLocation(location[i], locationVec[i]);
-        }
-        auto r = gnssBatchingCbIface->gnssLocationBatchCb(locationVec);
-        if (!r.isOk()) {
-            LOC_LOGE("%s] Error from gnssLocationBatchCb 1.0 description=%s",
-                __func__, r.description().c_str());
+    // back to back stop() and flush() could bring twice onBatchingCb(). Each one might come first.
+    // Combine them both (the first goes to cache, the second in location*) before report to FW
+    switch (mState) {
+        case STOPPING:
+            mState = STOPPED;
+            for (size_t i = 0; i < count; i++) {
+                mBatchedLocationInCache.push_back(location[i]);
+            }
+            break;
+        case STARTED:
+        case STOPPED: // flush() always trigger report, even on a stopped session
+            processReport = true;
+            break;
+        default:
+            break;
+    }
+    // report location batch when in STARTED state or flush(), combined with cache in last stop()
+    if (processReport) {
+        auto gnssBatchingCbIface(mGnssBatchingCbIface);
+        auto gnssBatchingCbIface_2_0(mGnssBatchingCbIface_2_0);
+        size_t batchCacheCnt = mBatchedLocationInCache.size();
+        LOC_LOGd("(batchCacheCnt: %zu)", batchCacheCnt);
+        if (gnssBatchingCbIface_2_0 != nullptr) {
+            hidl_vec<V2_0::GnssLocation> locationVec;
+            if (count+batchCacheCnt > 0) {
+                locationVec.resize(count+batchCacheCnt);
+                for (size_t i = 0; i < batchCacheCnt; ++i) {
+                    convertGnssLocation(mBatchedLocationInCache[i], locationVec[i]);
+                }
+                for (size_t i = 0; i < count; i++) {
+                    convertGnssLocation(location[i], locationVec[i+batchCacheCnt]);
+                }
+            }
+            auto r = gnssBatchingCbIface_2_0->gnssLocationBatchCb(locationVec);
+            if (!r.isOk()) {
+                LOC_LOGE("%s] Error from gnssLocationBatchCb 2_0 description=%s",
+                        __func__, r.description().c_str());
+            }
+        } else if (gnssBatchingCbIface != nullptr) {
+            hidl_vec<V1_0::GnssLocation> locationVec;
+            if (count+batchCacheCnt > 0) {
+                locationVec.resize(count+batchCacheCnt);
+                for (size_t i = 0; i < batchCacheCnt; ++i) {
+                    convertGnssLocation(mBatchedLocationInCache[i], locationVec[i]);
+                }
+                for (size_t i = 0; i < count; i++) {
+                    convertGnssLocation(location[i], locationVec[i+batchCacheCnt]);
+                }
+            }
+            auto r = gnssBatchingCbIface->gnssLocationBatchCb(locationVec);
+            if (!r.isOk()) {
+                LOC_LOGE("%s] Error from gnssLocationBatchCb 1.0 description=%s",
+                        __func__, r.description().c_str());
+            }
         }
+        mBatchedLocationInCache.clear();
     }
+    mMutex.unlock();
 }
 
 static void convertBatchOption(const IGnssBatching::Options& in, LocationOptions& out,
diff --git a/android/2.1/location_api/BatchingAPIClient.h b/android/2.1/location_api/BatchingAPIClient.h
index 08d7d23..93b4ace 100755
--- a/android/2.1/location_api/BatchingAPIClient.h
+++ b/android/2.1/location_api/BatchingAPIClient.h
@@ -43,6 +43,8 @@ namespace gnss {
 namespace V2_1 {
 namespace implementation {
 
+enum BATCHING_STATE { STARTED, STOPPING, STOPPED };
+
 class BatchingAPIClient : public LocationAPIClientBase
 {
 public:
@@ -72,6 +74,9 @@ private:
     uint32_t mDefaultId;
     LocationCapabilitiesMask mLocationCapabilitiesMask;
     sp<V2_0::IGnssBatchingCallback> mGnssBatchingCbIface_2_0;
+    volatile BATCHING_STATE mState = STOPPED;
+
+    std::vector<Location> mBatchedLocationInCache;
 };
 
 }  // namespace implementation
diff --git a/location/LocationAPIClientBase.cpp b/location/LocationAPIClientBase.cpp
index cf45e6a..ef2a9d0 100644
--- a/location/LocationAPIClientBase.cpp
+++ b/location/LocationAPIClientBase.cpp
@@ -608,7 +608,7 @@ uint32_t LocationAPIClientBase::locAPIUpdateSessionOptions(
             retVal = LOCATION_ERROR_SUCCESS;
         } else {
             retVal = LOCATION_ERROR_ID_UNKNOWN;
-            LOC_LOGE("%s:%d] session %d is not exist.", __FUNCTION__, __LINE__, id);
+            LOC_LOGd("unknown session id: %d, might flush() a stopped session",  id);
         }
     }
     pthread_mutex_unlock(&mMutex);