From 95500f8a0f0ff501653d507916f98fc36cefdcc8 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Mon, 26 Feb 2018 19:08:23 -0800 Subject: Fix CDMA Range Checks for SignalStrength -Allow zero as a valid value for CDMA ECIO. Zero is allowed for EVDO ECIO and is equally valid for CDMA. Making them consistent by allowing zero here. -Set EVDO ECIO to -160 if unreported rather than setting it to -1. The "unreported" value is undocumented, and since -1 is well within the range of valid values, makes no sense. Since CDMA ECIO was setting an unreported value to a very low number, again making them the same. -Allow 0 for EVDO SNR. This value has a range that is documented both in the RIL and in SignalStrength to include zero, but we were previously disallowing 0. Making the range check inclusive in line with the existing documentation, which was self-consistent. Bug: 32364031 Test: runtest frameworks-telephony Change-Id: Ie0ca5abb4998d1b0b5abdbff9d51f364fe6db858 --- telephony/java/android/telephony/SignalStrength.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/telephony/java/android/telephony/SignalStrength.java b/telephony/java/android/telephony/SignalStrength.java index fc2ef2782b78..ccb550ee1897 100644 --- a/telephony/java/android/telephony/SignalStrength.java +++ b/telephony/java/android/telephony/SignalStrength.java @@ -311,11 +311,11 @@ public class SignalStrength implements Parcelable { // BER no change; mCdmaDbm = mCdmaDbm > 0 ? -mCdmaDbm : -120; - mCdmaEcio = (mCdmaEcio > 0) ? -mCdmaEcio : -160; + mCdmaEcio = (mCdmaEcio >= 0) ? -mCdmaEcio : -160; mEvdoDbm = (mEvdoDbm > 0) ? -mEvdoDbm : -120; - mEvdoEcio = (mEvdoEcio >= 0) ? -mEvdoEcio : -1; - mEvdoSnr = ((mEvdoSnr > 0) && (mEvdoSnr <= 8)) ? mEvdoSnr : -1; + mEvdoEcio = (mEvdoEcio >= 0) ? -mEvdoEcio : -160; + mEvdoSnr = ((mEvdoSnr >= 0) && (mEvdoSnr <= 8)) ? mEvdoSnr : -1; // TS 36.214 Physical Layer Section 5.1.3, TS 36.331 RRC mLteSignalStrength = (mLteSignalStrength >= 0) ? mLteSignalStrength : 99; -- cgit v1.2.3-59-g8ed1b From 3ffbf86e1e4701ea316146c1686f06193166c2a6 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Mon, 26 Feb 2018 19:14:18 -0800 Subject: Fix Range-Checking in CellSignalStrengthCdma The CellSignalStrengthCdma class previously allowed the values in the class to be kept as negative ints but expected them to be parceled as positive ints. This led to a confusing mess that is best unwound by calling the actual constructor for the class and letting the parcel values be an implementation detail. This CL removes all of the parcel-time coersion and instead expects that the class be constructed using a constructor rather than by manually parceling and then using the class to un-parcel. In addition, the range checking for inputs is now done only once, and values are no longer mutated in the parcel/unparcel process. Bug: 32364031 Test: runtest frameworks-telephony Change-Id: I59ce8c9df1bd99547f3de941a30d6c3cea8f2b8f --- .../android/telephony/CellSignalStrengthCdma.java | 49 ++++++++++++++-------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/telephony/java/android/telephony/CellSignalStrengthCdma.java b/telephony/java/android/telephony/CellSignalStrengthCdma.java index ece1ee378170..183f96ddab0b 100644 --- a/telephony/java/android/telephony/CellSignalStrengthCdma.java +++ b/telephony/java/android/telephony/CellSignalStrengthCdma.java @@ -41,14 +41,36 @@ public final class CellSignalStrengthCdma extends CellSignalStrength implements setDefaultValues(); } - /** @hide */ + /** + * SignalStrength constructor for input from the HAL. + * + * Note that values received from the HAL require coersion to be compatible here. All values + * reported through IRadio are the negative of the actual values (which results in a positive + * input to this method. + * + *

Note that this HAL is inconsistent with UMTS-based radio techs as the value indicating + * that a field is unreported is negative, rather than a large(r) positive number. + *

Also note that to keep the public-facing methods of this class consistent with others, + * unreported values are coerced to Integer.MAX_VALUE rather than left as -1, which is + * a departure from SignalStrength, which is stuck with the values it currently reports. + * + * @param cdmaDbm negative of the CDMA signal strength value or -1 if invalid. + * @param cdmaEcio negative of the CDMA pilot/noise ratio or -1 if invalid. + * @param evdoDbm negative of the EvDO signal strength value or -1 if invalid. + * @param evdoEcio negative of the EvDO pilot/noise ratio or -1 if invalid. + * @param evdoSnr an SNR value 0..8 or -1 if invalid. + * @hide + */ public CellSignalStrengthCdma(int cdmaDbm, int cdmaEcio, int evdoDbm, int evdoEcio, int evdoSnr) { - mCdmaDbm = cdmaDbm; - mCdmaEcio = cdmaEcio; - mEvdoDbm = evdoDbm; - mEvdoEcio = evdoEcio; - mEvdoSnr = evdoSnr; + // The values here were lifted from SignalStrength.validateInput() + // FIXME: Combine all checking and setting logic between this and SignalStrength. + mCdmaDbm = ((cdmaDbm > 0) && (cdmaDbm < 120)) ? -cdmaDbm : Integer.MAX_VALUE; + mCdmaEcio = ((cdmaEcio > 0) && (cdmaEcio < 160)) ? -cdmaEcio : Integer.MAX_VALUE; + + mEvdoDbm = ((evdoDbm > 0) && (evdoDbm < 120)) ? -evdoDbm : Integer.MAX_VALUE; + mEvdoEcio = ((evdoEcio > 0) && (evdoEcio < 160)) ? -evdoEcio : Integer.MAX_VALUE; + mEvdoSnr = ((evdoSnr > 0) && (evdoSnr <= 8)) ? evdoSnr : Integer.MAX_VALUE; } /** @hide */ @@ -303,13 +325,10 @@ public final class CellSignalStrengthCdma extends CellSignalStrength implements @Override public void writeToParcel(Parcel dest, int flags) { if (DBG) log("writeToParcel(Parcel, int): " + toString()); - // Need to multiply CdmaDbm, CdmaEcio, EvdoDbm and EvdoEcio by -1 - // to ensure consistency when reading values written here - // unless the value is invalid - dest.writeInt(mCdmaDbm * (mCdmaDbm != Integer.MAX_VALUE ? -1 : 1)); - dest.writeInt(mCdmaEcio * (mCdmaEcio != Integer.MAX_VALUE ? -1 : 1)); - dest.writeInt(mEvdoDbm * (mEvdoDbm != Integer.MAX_VALUE ? -1 : 1)); - dest.writeInt(mEvdoEcio * (mEvdoEcio != Integer.MAX_VALUE ? -1 : 1)); + dest.writeInt(mCdmaDbm); + dest.writeInt(mCdmaEcio); + dest.writeInt(mEvdoDbm); + dest.writeInt(mEvdoEcio); dest.writeInt(mEvdoSnr); } @@ -322,13 +341,9 @@ public final class CellSignalStrengthCdma extends CellSignalStrength implements // the parcel as positive values. // Need to convert into negative values unless the value is invalid mCdmaDbm = in.readInt(); - if (mCdmaDbm != Integer.MAX_VALUE) mCdmaDbm *= -1; mCdmaEcio = in.readInt(); - if (mCdmaEcio != Integer.MAX_VALUE) mCdmaEcio *= -1; mEvdoDbm = in.readInt(); - if (mEvdoDbm != Integer.MAX_VALUE) mEvdoDbm *= -1; mEvdoEcio = in.readInt(); - if (mEvdoEcio != Integer.MAX_VALUE) mEvdoEcio *= -1; mEvdoSnr = in.readInt(); if (DBG) log("CellSignalStrengthCdma(Parcel): " + toString()); } -- cgit v1.2.3-59-g8ed1b From 78eb48e53600198b04517210bc23977e59484e2c Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Mon, 26 Feb 2018 20:31:04 -0800 Subject: Set CDMA Location to Invalid if on Null Island If the reported CDMA location is ~= (0, 0), which is in the middle of the Gulf of Guinea, assert that there are no CDMA cell towers within range (there are not) and force the location to a saner default value of Integer.MAX_VALUE which is out of the range of valid lats+longs. Bug: 32364031 Test: runtest frameworks-telephony Change-Id: I3f50054dd37cf7cef56b1bd16c3313c02da34c31 --- .../java/android/telephony/CellIdentityCdma.java | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/telephony/java/android/telephony/CellIdentityCdma.java b/telephony/java/android/telephony/CellIdentityCdma.java index 2e1d1dc343cd..105ddb0e8f25 100644 --- a/telephony/java/android/telephony/CellIdentityCdma.java +++ b/telephony/java/android/telephony/CellIdentityCdma.java @@ -103,8 +103,12 @@ public final class CellIdentityCdma extends CellIdentity { mNetworkId = nid; mSystemId = sid; mBasestationId = bid; - mLongitude = lon; - mLatitude = lat; + if (!isNullIsland(lat, lon)) { + mLongitude = lon; + mLatitude = lat; + } else { + mLongitude = mLatitude = Integer.MAX_VALUE; + } mAlphaLong = alphal; mAlphaShort = alphas; } @@ -118,6 +122,18 @@ public final class CellIdentityCdma extends CellIdentity { return new CellIdentityCdma(this); } + /** + * Take the latitude and longitude in 1/4 seconds and see if + * the reported location is on Null Island. + * + * @return whether the reported Lat/Long are for Null Island + * + * @hide + */ + private boolean isNullIsland(int lat, int lon) { + return Math.abs(lat) <= 1 && Math.abs(lon) <= 1; + } + /** * @return Network Id 0..65535, Integer.MAX_VALUE if unknown */ -- cgit v1.2.3-59-g8ed1b