Handling concurrent location & SV status update
Move handling of location and SV status updates to
existing handler thread.
Change SV status JNI output for thread safety.
Synchronize updates to LocationExtras (many entry
points)
Adding some DeleteLocalRef cleanups
Bug: 72481417
Fixes: 72949246
Fixes: 73090867
Test: GPS works on device, and adb shell dumpsys location
shows extras
Change-Id: I3eda8884597ad1cdf5aec6d1b7c33ab49401410f
diff --git a/services/core/jni/com_android_server_location_GnssLocationProvider.cpp b/services/core/jni/com_android_server_location_GnssLocationProvider.cpp
index c2771e4..0c11bc1 100644
--- a/services/core/jni/com_android_server_location_GnssLocationProvider.cpp
+++ b/services/core/jni/com_android_server_location_GnssLocationProvider.cpp
@@ -393,10 +393,7 @@
// New in 1.1
Return<void> gnssNameCb(const android::hardware::hidl_string& name) override;
- static GnssSvInfo sGnssSvList[static_cast<uint32_t>(
- android::hardware::gnss::V1_0::GnssMax::SVS_COUNT)];
- static size_t sGnssSvListSize;
-
+ // TODO(b/73306084): Reconsider allocation cost vs threadsafety on these statics
static const char* sNmeaString;
static size_t sNmeaStringLength;
};
@@ -412,11 +409,8 @@
return Void();
}
-IGnssCallback::GnssSvInfo GnssCallback::sGnssSvList[static_cast<uint32_t>(
- android::hardware::gnss::V1_0::GnssMax::SVS_COUNT)];
const char* GnssCallback::sNmeaString = nullptr;
size_t GnssCallback::sNmeaStringLength = 0;
-size_t GnssCallback::sGnssSvListSize = 0;
Return<void> GnssCallback::gnssLocationCb(const GnssLocation& location) {
JNIEnv* env = getJniEnv();
@@ -430,6 +424,7 @@
boolToJbool(hasLatLong),
jLocation);
checkAndClearExceptionFromCallback(env, __FUNCTION__);
+ env->DeleteLocalRef(jLocation);
return Void();
}
@@ -443,20 +438,55 @@
Return<void> GnssCallback::gnssSvStatusCb(const IGnssCallback::GnssSvStatus& svStatus) {
JNIEnv* env = getJniEnv();
- sGnssSvListSize = svStatus.numSvs;
- if (sGnssSvListSize > static_cast<uint32_t>(
+ uint32_t listSize = svStatus.numSvs;
+ if (listSize > static_cast<uint32_t>(
android::hardware::gnss::V1_0::GnssMax::SVS_COUNT)) {
- ALOGD("Too many satellites %zd. Clamps to %u.", sGnssSvListSize,
+ ALOGD("Too many satellites %u. Clamps to %u.", listSize,
static_cast<uint32_t>(android::hardware::gnss::V1_0::GnssMax::SVS_COUNT));
- sGnssSvListSize = static_cast<uint32_t>(android::hardware::gnss::V1_0::GnssMax::SVS_COUNT);
+ listSize = static_cast<uint32_t>(android::hardware::gnss::V1_0::GnssMax::SVS_COUNT);
}
- // Copy GNSS SV info into sGnssSvList, if any.
- if (svStatus.numSvs > 0) {
- memcpy(sGnssSvList, svStatus.gnssSvList.data(), sizeof(GnssSvInfo) * sGnssSvListSize);
+ jintArray svidWithFlagArray = env->NewIntArray(listSize);
+ jfloatArray cn0Array = env->NewFloatArray(listSize);
+ jfloatArray elevArray = env->NewFloatArray(listSize);
+ jfloatArray azimArray = env->NewFloatArray(listSize);
+ jfloatArray carrierFreqArray = env->NewFloatArray(listSize);
+
+ jint* svidWithFlags = env->GetIntArrayElements(svidWithFlagArray, 0);
+ jfloat* cn0s = env->GetFloatArrayElements(cn0Array, 0);
+ jfloat* elev = env->GetFloatArrayElements(elevArray, 0);
+ jfloat* azim = env->GetFloatArrayElements(azimArray, 0);
+ jfloat* carrierFreq = env->GetFloatArrayElements(carrierFreqArray, 0);
+
+ /*
+ * Read GNSS SV info.
+ */
+ for (size_t i = 0; i < listSize; ++i) {
+ enum ShiftWidth: uint8_t {
+ SVID_SHIFT_WIDTH = 8,
+ CONSTELLATION_TYPE_SHIFT_WIDTH = 4
+ };
+
+ const IGnssCallback::GnssSvInfo& info = svStatus.gnssSvList.data()[i];
+ svidWithFlags[i] = (info.svid << SVID_SHIFT_WIDTH) |
+ (static_cast<uint32_t>(info.constellation) << CONSTELLATION_TYPE_SHIFT_WIDTH) |
+ static_cast<uint32_t>(info.svFlag);
+ cn0s[i] = info.cN0Dbhz;
+ elev[i] = info.elevationDegrees;
+ azim[i] = info.azimuthDegrees;
+ carrierFreq[i] = info.carrierFrequencyHz;
}
- env->CallVoidMethod(mCallbacksObj, method_reportSvStatus);
+ env->ReleaseIntArrayElements(svidWithFlagArray, svidWithFlags, 0);
+ env->ReleaseFloatArrayElements(cn0Array, cn0s, 0);
+ env->ReleaseFloatArrayElements(elevArray, elev, 0);
+ env->ReleaseFloatArrayElements(azimArray, azim, 0);
+ env->ReleaseFloatArrayElements(carrierFreqArray, carrierFreq, 0);
+
+ env->CallVoidMethod(mCallbacksObj, method_reportSvStatus,
+ static_cast<jint>(listSize), svidWithFlagArray, cn0Array, elevArray, azimArray,
+ carrierFreqArray);
+
checkAndClearExceptionFromCallback(env, __FUNCTION__);
return Void();
}
@@ -575,6 +605,7 @@
timestamp);
checkAndClearExceptionFromCallback(env, __FUNCTION__);
+ env->DeleteLocalRef(jLocation);
return Void();
}
@@ -590,6 +621,7 @@
status,
jLocation);
checkAndClearExceptionFromCallback(env, __FUNCTION__);
+ env->DeleteLocalRef(jLocation);
return Void();
}
@@ -1063,7 +1095,7 @@
method_reportLocation = env->GetMethodID(clazz, "reportLocation",
"(ZLandroid/location/Location;)V");
method_reportStatus = env->GetMethodID(clazz, "reportStatus", "(I)V");
- method_reportSvStatus = env->GetMethodID(clazz, "reportSvStatus", "()V");
+ method_reportSvStatus = env->GetMethodID(clazz, "reportSvStatus", "(I[I[F[F[F[F)V");
method_reportAGpsStatus = env->GetMethodID(clazz, "reportAGpsStatus", "(II[B)V");
method_reportNmea = env->GetMethodID(clazz, "reportNmea", "(J)V");
method_setEngineCapabilities = env->GetMethodID(clazz, "setEngineCapabilities", "(I)V");
@@ -1369,49 +1401,6 @@
}
}
-/*
- * This enum is used by the read_sv_status method to combine the svid,
- * constellation and svFlag fields.
- */
-enum ShiftWidth: uint8_t {
- SVID_SHIFT_WIDTH = 8,
- CONSTELLATION_TYPE_SHIFT_WIDTH = 4
-};
-
-static jint android_location_GnssLocationProvider_read_sv_status(JNIEnv* env, jobject /* obj */,
- jintArray svidWithFlagArray, jfloatArray cn0Array, jfloatArray elevArray,
- jfloatArray azumArray, jfloatArray carrierFreqArray) {
- /*
- * This method should only be called from within a call to reportSvStatus.
- */
- jint* svidWithFlags = env->GetIntArrayElements(svidWithFlagArray, 0);
- jfloat* cn0s = env->GetFloatArrayElements(cn0Array, 0);
- jfloat* elev = env->GetFloatArrayElements(elevArray, 0);
- jfloat* azim = env->GetFloatArrayElements(azumArray, 0);
- jfloat* carrierFreq = env->GetFloatArrayElements(carrierFreqArray, 0);
-
- /*
- * Read GNSS SV info.
- */
- for (size_t i = 0; i < GnssCallback::sGnssSvListSize; ++i) {
- const IGnssCallback::GnssSvInfo& info = GnssCallback::sGnssSvList[i];
- svidWithFlags[i] = (info.svid << SVID_SHIFT_WIDTH) |
- (static_cast<uint32_t>(info.constellation) << CONSTELLATION_TYPE_SHIFT_WIDTH) |
- static_cast<uint32_t>(info.svFlag);
- cn0s[i] = info.cN0Dbhz;
- elev[i] = info.elevationDegrees;
- azim[i] = info.azimuthDegrees;
- carrierFreq[i] = info.carrierFrequencyHz;
- }
-
- env->ReleaseIntArrayElements(svidWithFlagArray, svidWithFlags, 0);
- env->ReleaseFloatArrayElements(cn0Array, cn0s, 0);
- env->ReleaseFloatArrayElements(elevArray, elev, 0);
- env->ReleaseFloatArrayElements(azumArray, azim, 0);
- env->ReleaseFloatArrayElements(carrierFreqArray, carrierFreq, 0);
- return static_cast<jint>(GnssCallback::sGnssSvListSize);
-}
-
static void android_location_GnssLocationProvider_agps_set_reference_location_cellid(
JNIEnv* /* env */, jobject /* obj */, jint type, jint mcc, jint mnc, jint lac, jint cid) {
IAGnssRil::AGnssRefLocation location;
@@ -2057,9 +2046,6 @@
{"native_delete_aiding_data",
"(I)V",
reinterpret_cast<void*>(android_location_GnssLocationProvider_delete_aiding_data)},
- {"native_read_sv_status",
- "([I[F[F[F[F)I",
- reinterpret_cast<void *>(android_location_GnssLocationProvider_read_sv_status)},
{"native_read_nmea", "([BI)I", reinterpret_cast<void *>(
android_location_GnssLocationProvider_read_nmea)},
{"native_inject_time", "(JJI)V", reinterpret_cast<void *>(