Fix comments and internal names about SEEK operation.

Legacy radio HAL named it SCAN and the name slipped through HAL 1.x into HAL 2.0. I finally noticed it after HAL 2.0 was locked down. Bug: 109740376 Test: build ow Change-Id: I5ca4147227a7dd0c32248a279f03fb632d97258e
2018-06-05 10:10:39 -07:00 · 2018-06-05 10:10:39 -07:00 · b557e0b748
commit b557e0b748
parent 62fd03d8e6
7 changed files with 42 additions and 33 deletions
--- a/broadcastradio/2.0/ITunerCallback.hal
+++ b/broadcastradio/2.0/ITunerCallback.hal
@ -18,7 +18,8 @@ package android.hardware.broadcastradio@2.0;
 interface ITunerCallback {
    /**
     * Method called by the HAL when a tuning operation fails asynchronously
-     * following a step(), scan() or tune() command.
+     * following ITunerSession::tune(), ITunerSession::scan() or
+     * ITunerSession::step().
     *
     * This callback is only called when the step(), scan() or tune() command
     * returned OK at first.
--- a/broadcastradio/2.0/ITunerSession.hal
+++ b/broadcastradio/2.0/ITunerSession.hal
@ -19,7 +19,7 @@ interface ITunerSession {
    /**
     * Tune to a specified program.
     *
-     * Automatically cancels pending scan, step or tune.
+     * Automatically cancels pending tune(), scan() or step().
     * If the method returns OK, tuneFailed or currentProgramInfoChanged
     * callback must be called.
     *
@ -33,9 +33,16 @@ interface ITunerSession {
    tune(ProgramSelector program) generates (Result result);

    /**
-     * Tune to the next valid program.
+     * Tune (seek) to the next valid program on the "air".
     *
-     * Automatically cancels pending scan, step or tune.
+     * This might more naturally be called "seek" but for legacy reasons, the
+     * entry point remains "scan". This should not be confused with the actual
+     * scan operation (where the radio seeks through programs in a loop until
+     * user chooses to stay on one of them) nor background scan operation (that
+     * a tuner may do in order to locate all available programs.  This function
+     * is meant to advance to the next detected program and stay there.
+     *
+     * Automatically cancels pending tune(), scan() or step().
     * If the method returns OK, tuneFailed or currentProgramInfoChanged
     * callback must be called.
     *
@ -44,20 +51,20 @@ interface ITunerSession {
     *  - DAB secondary service.
     *
     * As an implementation detail, the HAL has the option to perform an actual
-     * scan or select the next program from the list retrieved in the
+     * seek or select the next program from the list retrieved in the
     * background, if one is not stale.
     *
     * @param directionUp True to change towards higher numeric values
     *                    (frequency, channel number), false towards lower.
     * @param skipSubChannel Don't tune to subchannels.
-     * @return result OK if the scan has successfully started.
+     * @return result OK if the operation has successfully started.
     */
    scan(bool directionUp, bool skipSubChannel) generates (Result result);

    /**
     * Tune to the adjacent channel, which may not be occupied by any program.
     *
-     * Automatically cancels pending scan, step or tune.
+     * Automatically cancels pending tune(), scan() or step().
     * If the method returns OK, tuneFailed or currentProgramInfoChanged
     * callback must be called.
     *
@ -70,7 +77,7 @@ interface ITunerSession {
    step(bool directionUp) generates (Result result);

    /**
-     * Cancel a scan, step or tune operation.
+     * Cancel a pending tune(), scan() or step().
     *
     * If there is no such operation running, the call must be ignored.
     */
--- a/broadcastradio/2.0/default/TunerSession.cpp
+++ b/broadcastradio/2.0/default/TunerSession.cpp
@ -42,7 +42,7 @@ using std::vector;

 namespace delay {

-static constexpr auto scan = 200ms;
+static constexpr auto seek = 200ms;
 static constexpr auto step = 100ms;
 static constexpr auto tune = 150ms;
 static constexpr auto list = 1s;
@ -131,11 +131,11 @@ Return<Result> TunerSession::scan(bool directionUp, bool /* skipSubChannel */) {
    if (list.empty()) {
        mIsTuneCompleted = false;
        auto task = [this, directionUp]() {
-            ALOGI("Performing failed scan up=%d", directionUp);
+            ALOGI("Performing failed seek up=%d", directionUp);

            mCallback->onTuneFailed(Result::TIMEOUT, {});
        };
-        mThread.schedule(task, delay::scan);
+        mThread.schedule(task, delay::seek);

        return Result::OK;
    }
@ -162,12 +162,12 @@ Return<Result> TunerSession::scan(bool directionUp, bool /* skipSubChannel */) {

    mIsTuneCompleted = false;
    auto task = [this, tuneTo, directionUp]() {
-        ALOGI("Performing scan up=%d", directionUp);
+        ALOGI("Performing seek up=%d", directionUp);

        lock_guard<mutex> lk(mMut);
        tuneInternalLocked(tuneTo);
    };
-    mThread.schedule(task, delay::scan);
+    mThread.schedule(task, delay::seek);

    return Result::OK;
 }
--- a/broadcastradio/2.0/default/VirtualProgram.h
+++ b/broadcastradio/2.0/default/VirtualProgram.h
@ -40,8 +40,8 @@ struct VirtualProgram {
    operator ProgramInfo() const;

    /**
-     * Defines order on how virtual programs appear on the "air" with
-     * ITunerSession::scan operation.
+     * Defines order in which virtual programs appear on the "air" with
+     * ITunerSession::scan().
     *
     * It's for default implementation purposes, may not be complete or correct.
     */
--- a/broadcastradio/2.0/types.hal
+++ b/broadcastradio/2.0/types.hal
@ -184,18 +184,20 @@ struct AmFmRegionConfig {
 * lowerBound + channelNumber * spacing, up to upperBound.
 */
 struct AmFmBandRange {
-    /** The frequency of the first channel within the range. */
+    /** The frequency (in kHz) of the first channel within the range. */
    uint32_t lowerBound;

-    /** The frequency of the last channel within the range. */
+    /** The frequency (in kHz) of the last channel within the range. */
    uint32_t upperBound;

-    /** Channel grid resolution, how far apart are the channels. */
+    /** Channel grid resolution (in kHz), how far apart are the channels. */
    uint32_t spacing;

    /**
-     * Spacing used when scanning for channels. It's a multiply of spacing and
-     * allows to skip some channels when scanning to make it faster.
+     * Channel spacing (in kHz) used to speed up seeking to the next station
+     * via the ITunerSession::scan() operation.
+     *
+     * It must be a multiple of channel grid resolution.
     *
     * Tuner may first quickly check every n-th channel and if it detects echo
     * from a station, it fine-tunes to find the exact frequency.
@ -415,9 +417,10 @@ enum ProgramInfoFlags : uint32_t {
    TRAFFIC_ANNOUNCEMENT = 1 << 3,

    /**
-     * Tuned to a program (not playing a static).
+     * Tuned to a program (not playing static).
     *
-     * It's the same condition that would stop scan() operation.
+     * It's the same condition that would stop a seek operation
+     * (ie: ITunerSession::scan()).
     *
     * By definition, this flag must be set for all items on the program list.
     */
--- a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
+++ b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp
@ -308,7 +308,7 @@ static bool supportsFM(const AmFmRegionConfig& config) {
 *  - there is at least one AM/FM band configured;
 *  - FM Deemphasis and RDS are correctly configured for FM-capable radio;
 *  - all channel grids (frequency ranges and spacings) are valid;
- *  - scan spacing is a multiply of manual spacing value.
+ *  - seek spacing is a multiple of the manual spacing value.
 */
 TEST_F(BroadcastRadioHalTest, GetAmFmRegionConfig) {
    AmFmRegionConfig config;
@ -341,7 +341,7 @@ TEST_F(BroadcastRadioHalTest, GetAmFmRegionConfig) {
 *  - there is at least one AM/FM range supported;
 *  - there is at least one de-emphasis filter mode supported for FM-capable radio;
 *  - all channel grids (frequency ranges and spacings) are valid;
- *  - scan spacing is not set.
+ *  - seek spacing is not set.
 */
 TEST_F(BroadcastRadioHalTest, GetAmFmRegionConfigCapabilities) {
    AmFmRegionConfig config;
@ -501,14 +501,14 @@ TEST_F(BroadcastRadioHalTest, TuneFailsWithEmpty) {
 }

 /**
- * Test scanning to next/prev station.
+ * Test seeking to next/prev station via ITunerSession::scan().
 *
 * Verifies that:
 *  - the method succeeds;
 *  - the program info is changed within timeout::tune;
 *  - works both directions and with or without skipping sub-channel.
 */
-TEST_F(BroadcastRadioHalTest, Scan) {
+TEST_F(BroadcastRadioHalTest, Seek) {
    ASSERT_TRUE(openSession());

    // TODO(b/69958777): see FmTune workaround
@ -564,8 +564,8 @@ TEST_F(BroadcastRadioHalTest, Cancel) {
    ASSERT_TRUE(openSession());

    for (int i = 0; i < 10; i++) {
-        auto scanResult = mSession->scan(true /* up */, true /* skip subchannel */);
-        ASSERT_EQ(Result::OK, scanResult);
+        auto result = mSession->scan(true /* up */, true /* skip subchannel */);
+        ASSERT_EQ(Result::OK, result);

        auto cancelResult = mSession->cancel();
        ASSERT_TRUE(cancelResult.isOk());
--- a/current.txt
+++ b/current.txt
@ -251,8 +251,6 @@ c8bc853546dd55584611def2a9fa1d99f657e3366c976d2f60fe6b8aa6d2cb87 android.hardwar

 # ABI preserving changes to HALs during Android P
 9e7a0b650d0e461ece2cfec0e1072abf8676f592b41a7fb48f01e88fc3c8f780 android.hardware.broadcastradio@1.0::types
-eaeb3e4f3237430a7fdc206bffdf844713f5682990b2d49ea24392e15b5d343f android.hardware.broadcastradio@2.0::ITunerCallback
-2804120c1f8522ad15feb7695fe5eece527d399b406c671ea99618194118c316 android.hardware.broadcastradio@2.0::types
 cf72ff5a52bfa4d08e9e1000cf3ab5952a2d280c7f13cdad5ab7905c08050766 android.hardware.camera.metadata@3.2::types
 3902efc42097cba55f0655aa389e052ea70164e99ced1a6d1ef53dafc13f7650 android.hardware.camera.provider@2.4::ICameraProvider
 6fa9804a17a8bb7923a56bd10493a5483c20007e4c9026fd04287bee7c945a8c android.hardware.gnss@1.0::IGnssCallback
@ -308,9 +306,9 @@ ff4be64d7992f8bec97dff37f35450e79b3430c61f85f54322ce45bef229dc3b android.hardwar
 3d8ed67d807e9f15d0708390a416bee00920f6a22196c104cc9e443c8d217df8 android.hardware.broadcastradio@2.0::IAnnouncementListener
 44017c42e6f4d8cb30f07eb1da04540a98736a336ac28c7e0ed2e69e1589f8d1 android.hardware.broadcastradio@2.0::IBroadcastRadio
 e5f4960290b4f3089163dd43251e1a032c81e9bdb796e75a87fc7c5810c262b3 android.hardware.broadcastradio@2.0::ICloseHandle
-7357516e8f4585e211b9b6f271af6fb82d6e9cc208df01851e63118404621e6d android.hardware.broadcastradio@2.0::ITunerCallback
-eacf4e7491fc52c4db90898faddf25ec7bc72501b07ae8737434c47cb845128c android.hardware.broadcastradio@2.0::ITunerSession
-34fe4601072aa5051eb8e41d776a9f6d43f716687de3f5c0031a12e7912ff3d6 android.hardware.broadcastradio@2.0::types
+af24b87ca8b8f02fcde205e47db6a9609fc7e9d77d73e694ec8f9c508ca19575 android.hardware.broadcastradio@2.0::ITunerCallback
+d70464c517a4a1b5167730843775a97f455102919e945b04f717b9da390c0f39 android.hardware.broadcastradio@2.0::ITunerSession
+2afa59ebf8145e7fbc090cf49605c27280c07d4178d47cd7f9d82b3b95a47af0 android.hardware.broadcastradio@2.0::types
 4fb0725c36ed4f77a42b42e3f18d8b5f7919cb62b90098b23143a555aa7dd96d android.hardware.camera.device@3.4::ICameraDeviceCallback
 812fa66aa10ba0cba27cfddc2fd7f0ee27a8ab65a1f15aa79fdad97d403e6a14 android.hardware.camera.device@3.4::ICameraDeviceSession
 cc288f1f78d1e643eb3d3dbc16e1401d44033d8e6856761f5156814a29986ec7 android.hardware.camera.device@3.4::types