Fix ANAPIC review comments (hardware/interfaces)

* Use "GnssAgc[] gnssAgcs = {};" for backwards compatibility
* Add comments on AgnssTypes
* Rename SetIDType -> SetIdType
* Update the comment on NetworkAttributes -> networkHandle
* Remove the “DELETE_” prefix in the GnssAidingData
* Make a parcelable for setPositionMode parameters

Bug: 215566115
Test: atest VtsHalGnssTargetTest
Change-Id: Ifc2de451a43cd1f32267e74dc288a3821b75f0cb
diff --git a/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/GnssData.aidl b/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/GnssData.aidl
index aa514da..01a3b3a 100644
--- a/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/GnssData.aidl
+++ b/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/GnssData.aidl
@@ -37,7 +37,7 @@
   android.hardware.gnss.GnssMeasurement[] measurements;
   android.hardware.gnss.GnssClock clock;
   android.hardware.gnss.ElapsedRealtime elapsedRealtime;
-  @nullable android.hardware.gnss.GnssData.GnssAgc[] gnssAgcs;
+  android.hardware.gnss.GnssData.GnssAgc[] gnssAgcs = {};
   @VintfStability
   parcelable GnssAgc {
     double agcLevelDb;
diff --git a/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IAGnssRil.aidl b/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IAGnssRil.aidl
index 73df195..69fa32b 100644
--- a/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IAGnssRil.aidl
+++ b/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IAGnssRil.aidl
@@ -36,7 +36,7 @@
 interface IAGnssRil {
   void setCallback(in android.hardware.gnss.IAGnssRilCallback callback);
   void setRefLocation(in android.hardware.gnss.IAGnssRil.AGnssRefLocation agnssReflocation);
-  void setSetId(in android.hardware.gnss.IAGnssRil.SetIDType type, in @utf8InCpp String setid);
+  void setSetId(in android.hardware.gnss.IAGnssRil.SetIdType type, in @utf8InCpp String setid);
   void updateNetworkState(in android.hardware.gnss.IAGnssRil.NetworkAttributes attributes);
   const int NETWORK_CAPABILITY_NOT_METERED = 1;
   const int NETWORK_CAPABILITY_NOT_ROAMING = 2;
@@ -48,7 +48,7 @@
     NR_CELLID = 8,
   }
   @Backing(type="int") @VintfStability
-  enum SetIDType {
+  enum SetIdType {
     NONE = 0,
     IMSI = 1,
     MSISDM = 2,
diff --git a/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IGnss.aidl b/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IGnss.aidl
index a16d27b..affef2b 100644
--- a/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IGnss.aidl
+++ b/gnss/aidl/aidl_api/android.hardware.gnss/current/android/hardware/gnss/IGnss.aidl
@@ -53,7 +53,7 @@
   void injectLocation(in android.hardware.gnss.GnssLocation location);
   void injectBestLocation(in android.hardware.gnss.GnssLocation location);
   void deleteAidingData(in android.hardware.gnss.IGnss.GnssAidingData aidingDataFlags);
-  void setPositionMode(in android.hardware.gnss.IGnss.GnssPositionMode mode, in android.hardware.gnss.IGnss.GnssPositionRecurrence recurrence, in int minIntervalMs, in int preferredAccuracyMeters, in int preferredTimeMs, in boolean lowPowerMode);
+  void setPositionMode(in android.hardware.gnss.IGnss.PositionModeOptions options);
   android.hardware.gnss.IGnssAntennaInfo getExtensionGnssAntennaInfo();
   @nullable android.hardware.gnss.measurement_corrections.IMeasurementCorrectionsInterface getExtensionMeasurementCorrections();
   const int ERROR_INVALID_ARGUMENT = 1;
@@ -72,18 +72,27 @@
   }
   @Backing(type="int") @VintfStability
   enum GnssAidingData {
-    DELETE_EPHEMERIS = 1,
-    DELETE_ALMANAC = 2,
-    DELETE_POSITION = 4,
-    DELETE_TIME = 8,
-    DELETE_IONO = 16,
-    DELETE_UTC = 32,
-    DELETE_HEALTH = 64,
-    DELETE_SVDIR = 128,
-    DELETE_SVSTEER = 256,
-    DELETE_SADATA = 512,
-    DELETE_RTI = 1024,
-    DELETE_CELLDB_INFO = 32768,
-    DELETE_ALL = 65535,
+    EPHEMERIS = 1,
+    ALMANAC = 2,
+    POSITION = 4,
+    TIME = 8,
+    IONO = 16,
+    UTC = 32,
+    HEALTH = 64,
+    SVDIR = 128,
+    SVSTEER = 256,
+    SADATA = 512,
+    RTI = 1024,
+    CELLDB_INFO = 32768,
+    ALL = 65535,
+  }
+  @VintfStability
+  parcelable PositionModeOptions {
+    android.hardware.gnss.IGnss.GnssPositionMode mode;
+    android.hardware.gnss.IGnss.GnssPositionRecurrence recurrence;
+    int minIntervalMs;
+    int preferredAccuracyMeters;
+    int preferredTimeMs;
+    boolean lowPowerMode;
   }
 }
diff --git a/gnss/aidl/android/hardware/gnss/GnssData.aidl b/gnss/aidl/android/hardware/gnss/GnssData.aidl
index 204eb65..6b2068e 100644
--- a/gnss/aidl/android/hardware/gnss/GnssData.aidl
+++ b/gnss/aidl/android/hardware/gnss/GnssData.aidl
@@ -76,6 +76,10 @@
          * is the primary common use central frequency, e.g. L1 = 1575.45 MHz
          * for GPS.
          *
+         * If all the GLO frequencies have a common AGC, the FC0 (frequency
+         * channel number 0) of the individual GLO bands is used to represent
+         * all the GLO frequencies.
+         *
          * For an L1, L5 receiver tracking a satellite on L1 and L5 at the same
          * time, two raw measurement structs must be reported for this same
          * satellite, in one of the measurement structs, all the values related
@@ -92,5 +96,5 @@
      * GnssMeasurement or GnssClock fields are not reported yet. E.g., when a GNSS signal is too
      * weak to be acquired, the AGC value must still be reported.
      */
-    @nullable GnssAgc[] gnssAgcs;
+    GnssAgc[] gnssAgcs = {};
 }
diff --git a/gnss/aidl/android/hardware/gnss/IAGnssCallback.aidl b/gnss/aidl/android/hardware/gnss/IAGnssCallback.aidl
index 7c25937..8f881b7 100644
--- a/gnss/aidl/android/hardware/gnss/IAGnssCallback.aidl
+++ b/gnss/aidl/android/hardware/gnss/IAGnssCallback.aidl
@@ -23,9 +23,13 @@
     @VintfStability
     @Backing(type="int")
     enum AGnssType {
+        // Secure User Plane Location
         SUPL = 1,
+        // CDMA2000
         C2K = 2,
+        // SUPL, Emergency call over IP Multimedia Subsystem
         SUPL_EIMS = 3,
+        // SUPL, IP Multimedia Subsystem
         SUPL_IMS = 4,
     }
 
diff --git a/gnss/aidl/android/hardware/gnss/IAGnssRil.aidl b/gnss/aidl/android/hardware/gnss/IAGnssRil.aidl
index c506b04..b505d81 100644
--- a/gnss/aidl/android/hardware/gnss/IAGnssRil.aidl
+++ b/gnss/aidl/android/hardware/gnss/IAGnssRil.aidl
@@ -17,7 +17,6 @@
 package android.hardware.gnss;
 
 import android.hardware.gnss.IAGnssRilCallback;
-import android.hardware.gnss.IAGnssRilCallback.SetIDType;
 
 /**
  * Extended interface for AGNSS RIL support. An Assisted GNSS Radio Interface
@@ -46,7 +45,7 @@
     /** SET ID type*/
     @VintfStability
     @Backing(type="int")
-    enum SetIDType {
+    enum SetIdType {
         NONE = 0,
         IMSI = 1,
         MSISDM = 2,
@@ -100,7 +99,7 @@
     /** Represents network connection status and capabilities. */
     @VintfStability
     parcelable NetworkAttributes {
-        /** Network handle of the network for use with the NDK API. */
+        /** A handle representing this Network. */
         long networkHandle;
 
         /**
@@ -151,7 +150,7 @@
      * If the type is NONE, then the string must be empty.
      *
      */
-    void setSetId(in SetIDType type, in @utf8InCpp String setid);
+    void setSetId(in SetIdType type, in @utf8InCpp String setid);
 
     /**
      * Notifies GNSS of network status changes.
diff --git a/gnss/aidl/android/hardware/gnss/IAGnssRilCallback.aidl b/gnss/aidl/android/hardware/gnss/IAGnssRilCallback.aidl
index 6fb093e..485626d 100644
--- a/gnss/aidl/android/hardware/gnss/IAGnssRilCallback.aidl
+++ b/gnss/aidl/android/hardware/gnss/IAGnssRilCallback.aidl
@@ -25,7 +25,7 @@
     /**
      * The Hal uses this API to request a SET ID.
      *
-     * @param setIdflag A bitfield of IAGnssRil.SetIDType that is required by
+     * @param setIdflag A bitfield of IAGnssRil.SetIdType that is required by
      * the HAL. The framework will inject an empty SET ID if the flag is NONE.
      *
      */
diff --git a/gnss/aidl/android/hardware/gnss/IGnss.aidl b/gnss/aidl/android/hardware/gnss/IGnss.aidl
index 99f2ee4..79950ad 100644
--- a/gnss/aidl/android/hardware/gnss/IGnss.aidl
+++ b/gnss/aidl/android/hardware/gnss/IGnss.aidl
@@ -81,19 +81,19 @@
     @VintfStability
     @Backing(type="int")
     enum GnssAidingData {
-        DELETE_EPHEMERIS = 0x0001,
-        DELETE_ALMANAC = 0x0002,
-        DELETE_POSITION = 0x0004,
-        DELETE_TIME = 0x0008,
-        DELETE_IONO = 0x0010,
-        DELETE_UTC = 0x0020,
-        DELETE_HEALTH = 0x0040,
-        DELETE_SVDIR = 0x0080,
-        DELETE_SVSTEER = 0x0100,
-        DELETE_SADATA = 0x0200,
-        DELETE_RTI = 0x0400,
-        DELETE_CELLDB_INFO = 0x8000,
-        DELETE_ALL = 0xFFFF
+        EPHEMERIS = 0x0001,
+        ALMANAC = 0x0002,
+        POSITION = 0x0004,
+        TIME = 0x0008,
+        IONO = 0x0010,
+        UTC = 0x0020,
+        HEALTH = 0x0040,
+        SVDIR = 0x0080,
+        SVSTEER = 0x0100,
+        SADATA = 0x0200,
+        RTI = 0x0400,
+        CELLDB_INFO = 0x8000,
+        ALL = 0xFFFF
     }
 
     /**
@@ -256,37 +256,56 @@
 
     /**
      * Specifies that the next call to start will not use the information defined in the flags.
-     * GnssAidingData value of DELETE_ALL is passed for a cold start.
+     * GnssAidingData value of GnssAidingData::ALL is passed for a cold start.
      *
      * @param aidingDataFlags Flags specifying the aiding data to be deleted.
      */
     void deleteAidingData(in GnssAidingData aidingDataFlags);
 
     /**
+     * Options used in the setPositionMode() call for specifying the GNSS engine behavior.
+     */
+    @VintfStability
+    parcelable PositionModeOptions {
+        /**
+         * Must be one of MS_BASED or STANDALONE. It is allowed by the platform (and it is
+         * recommended) to fallback to MS_BASED if MS_ASSISTED is passed in, and MS_BASED is
+         * supported.
+         */
+        GnssPositionMode mode;
+
+        /* Recurrence GNSS position recurrence value, either periodic or single. */
+        GnssPositionRecurrence recurrence;
+
+        /* Represents the time between fixes in milliseconds. */
+        int minIntervalMs;
+
+        /* Represents the requested fix accuracy in meters. */
+        int preferredAccuracyMeters;
+
+        /* Represents the requested time to first fix in milliseconds. */
+        int preferredTimeMs;
+
+        /**
+         * When true, and IGnss is the only client to the GNSS hardware, the GNSS hardware must make
+         * strong tradeoffs to substantially restrict power use. Specifically, in the case of a
+         * several second long minIntervalMs, the GNSS hardware must not, on average, run power
+         * hungry operations like RF and signal searches for more than one second per interval, and
+         * must make exactly one call to gnssSvStatusCb(), and either zero or one call to
+         * GnssLocationCb() at each interval. When false, HAL must operate in the nominal mode and
+         * is expected to make power and performance tradoffs such as duty-cycling when signal
+         * conditions are good and more active searches to reacquire GNSS signals when no signals
+         * are present. When there are additional clients using the GNSS hardware other than IGnss,
+         * the GNSS hardware may operate in a higher power mode, on behalf of those clients.
+         */
+        boolean lowPowerMode;
+    }
+
+    /**
      * Sets the GnssPositionMode parameter, its associated recurrence value, the time between fixes,
      * requested fix accuracy, time to first fix.
-     *
-     * @param mode Parameter must be one of MS_BASED or STANDALONE. It is allowed by the platform
-     *     (and it is recommended) to fallback to MS_BASED if MS_ASSISTED is passed in, and MS_BASED
-     *     is supported.
-     * @param recurrence GNSS position recurrence value, either periodic or single.
-     * @param minIntervalMs Represents the time between fixes in milliseconds.
-     * @param preferredAccuracyMeters Represents the requested fix accuracy in meters.
-     * @param preferredTimeMs Represents the requested time to first fix in milliseconds.
-     * @param lowPowerMode When true, and IGnss is the only client to the GNSS hardware, the GNSS
-     *     hardware must make strong tradeoffs to substantially restrict power use. Specifically, in
-     *     the case of a several second long minIntervalMs, the GNSS hardware must not, on average,
-     *     run power hungry operations like RF and signal searches for more than one second per
-     *     interval, and must make exactly one call to gnssSvStatusCb(), and either zero or one call
-     *     to GnssLocationCb() at each interval. When false, HAL must operate in the nominal mode
-     *     and is expected to make power and performance tradoffs such as duty-cycling when signal
-     *     conditions are good and more active searches to reacquire GNSS signals when no signals
-     *     are present. When there are additional clients using the GNSS hardware other than IGnss,
-     *     the GNSS hardware may operate in a higher power mode, on behalf of those clients.
      */
-    void setPositionMode(in GnssPositionMode mode, in GnssPositionRecurrence recurrence,
-            in int minIntervalMs, in int preferredAccuracyMeters, in int preferredTimeMs,
-            in boolean lowPowerMode);
+    void setPositionMode(in PositionModeOptions options);
 
     /*
      * This method returns the IGnssAntennaInfo.
diff --git a/gnss/aidl/default/AGnssRil.cpp b/gnss/aidl/default/AGnssRil.cpp
index afe0039..2aa1abc 100644
--- a/gnss/aidl/default/AGnssRil.cpp
+++ b/gnss/aidl/default/AGnssRil.cpp
@@ -41,7 +41,7 @@
     return ndk::ScopedAStatus::ok();
 }
 
-ndk::ScopedAStatus AGnssRil::setSetId(SetIDType type, const std::string& setid) {
+ndk::ScopedAStatus AGnssRil::setSetId(SetIdType type, const std::string& setid) {
     ALOGD("AGnssRil::setSetId: type:%s, setid: %s", toString(type).c_str(), setid.c_str());
     return ndk::ScopedAStatus::ok();
 }
diff --git a/gnss/aidl/default/AGnssRil.h b/gnss/aidl/default/AGnssRil.h
index 7e429ee..e205b69 100644
--- a/gnss/aidl/default/AGnssRil.h
+++ b/gnss/aidl/default/AGnssRil.h
@@ -24,7 +24,7 @@
   public:
     ndk::ScopedAStatus setCallback(const std::shared_ptr<IAGnssRilCallback>& callback) override;
     ndk::ScopedAStatus setRefLocation(const AGnssRefLocation& agnssReflocation) override;
-    ndk::ScopedAStatus setSetId(SetIDType type, const std::string& setid) override;
+    ndk::ScopedAStatus setSetId(SetIdType type, const std::string& setid) override;
     ndk::ScopedAStatus updateNetworkState(const NetworkAttributes& attributes) override;
 
   private:
diff --git a/gnss/aidl/default/Gnss.cpp b/gnss/aidl/default/Gnss.cpp
index 6331dfd..150a394 100644
--- a/gnss/aidl/default/Gnss.cpp
+++ b/gnss/aidl/default/Gnss.cpp
@@ -208,11 +208,10 @@
     return ScopedAStatus::ok();
 }
 
-ScopedAStatus Gnss::setPositionMode(GnssPositionMode, GnssPositionRecurrence, int minIntervalMs,
-                                    int /* preferredAccuracyMeters */, int /* preferredTimeMs */,
-                                    bool lowPowerMode) {
-    ALOGD("setPositionMode. minIntervalMs:%d, lowPowerMode:%d", minIntervalMs, (int)lowPowerMode);
-    mMinIntervalMs = minIntervalMs;
+ScopedAStatus Gnss::setPositionMode(const PositionModeOptions& options) {
+    ALOGD("setPositionMode. minIntervalMs:%d, lowPowerMode:%d", options.minIntervalMs,
+          (int)options.lowPowerMode);
+    mMinIntervalMs = options.minIntervalMs;
     return ScopedAStatus::ok();
 }
 
diff --git a/gnss/aidl/default/Gnss.h b/gnss/aidl/default/Gnss.h
index 36874b8..b50a1ae 100644
--- a/gnss/aidl/default/Gnss.h
+++ b/gnss/aidl/default/Gnss.h
@@ -50,9 +50,7 @@
     ndk::ScopedAStatus injectLocation(const GnssLocation& location) override;
     ndk::ScopedAStatus injectBestLocation(const GnssLocation& location) override;
     ndk::ScopedAStatus deleteAidingData(GnssAidingData aidingDataFlags) override;
-    ndk::ScopedAStatus setPositionMode(GnssPositionMode mode, GnssPositionRecurrence recurrence,
-                                       int minIntervalMs, int preferredAccuracyMeters,
-                                       int preferredTimeMs, bool lowPowerMode) override;
+    ndk::ScopedAStatus setPositionMode(const PositionModeOptions& options) override;
 
     ndk::ScopedAStatus getExtensionPsds(std::shared_ptr<IGnssPsds>* iGnssPsds) override;
     ndk::ScopedAStatus getExtensionGnssConfiguration(
diff --git a/gnss/aidl/vts/gnss_hal_test.cpp b/gnss/aidl/vts/gnss_hal_test.cpp
index 13c32ee..4828f19 100644
--- a/gnss/aidl/vts/gnss_hal_test.cpp
+++ b/gnss/aidl/vts/gnss_hal_test.cpp
@@ -79,9 +79,14 @@
     const int kPreferredAccuracy = 0;  // Ideally perfect (matches GnssLocationProvider)
     const int kPreferredTimeMsec = 0;  // Ideally immediate
 
-    auto status = aidl_gnss_hal_->setPositionMode(
-            IGnss::GnssPositionMode::MS_BASED, IGnss::GnssPositionRecurrence::RECURRENCE_PERIODIC,
-            min_interval_msec, kPreferredAccuracy, kPreferredTimeMsec, low_power_mode);
+    IGnss::PositionModeOptions options;
+    options.mode = IGnss::GnssPositionMode::MS_BASED;
+    options.recurrence = IGnss::GnssPositionRecurrence::RECURRENCE_PERIODIC;
+    options.minIntervalMs = min_interval_msec;
+    options.preferredAccuracyMeters = kPreferredAccuracy;
+    options.preferredTimeMs = kPreferredTimeMsec;
+    options.lowPowerMode = low_power_mode;
+    auto status = aidl_gnss_hal_->setPositionMode(options);
 
     ASSERT_TRUE(status.isOk());
 }
diff --git a/gnss/aidl/vts/gnss_hal_test_cases.cpp b/gnss/aidl/vts/gnss_hal_test_cases.cpp
index 1fa6825..1b69488 100644
--- a/gnss/aidl/vts/gnss_hal_test_cases.cpp
+++ b/gnss/aidl/vts/gnss_hal_test_cases.cpp
@@ -1050,10 +1050,9 @@
         // Validity check GnssData fields
         CheckGnssMeasurementClockFields(lastMeasurement);
 
-        ASSERT_TRUE(lastMeasurement.gnssAgcs.has_value());
-        for (const auto& gnssAgc : lastMeasurement.gnssAgcs.value()) {
-            ASSERT_TRUE(gnssAgc.has_value());
-            ASSERT_TRUE(gnssAgc.value().carrierFrequencyHz >= 0);
+        ASSERT_TRUE(lastMeasurement.gnssAgcs.size() > 0);
+        for (const auto& gnssAgc : lastMeasurement.gnssAgcs) {
+            ASSERT_TRUE(gnssAgc.carrierFrequencyHz >= 0);
         }
     }
 
diff --git a/gnss/common/utils/default/Utils.cpp b/gnss/common/utils/default/Utils.cpp
index 1ff84eb..122a293 100644
--- a/gnss/common/utils/default/Utils.cpp
+++ b/gnss/common/utils/default/Utils.cpp
@@ -247,8 +247,7 @@
     GnssData gnssData = {.measurements = {measurement},
                          .clock = clock,
                          .elapsedRealtime = timestamp,
-                         .gnssAgcs = std::make_optional(std::vector(
-                                 {std::make_optional(gnssAgc1), std::make_optional(gnssAgc2)}))};
+                         .gnssAgcs = std::vector({gnssAgc1, gnssAgc2})};
     return gnssData;
 }