Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b8a2afe4 authored by Tomasz Wasilczyk's avatar Tomasz Wasilczyk
Browse files

Address Broadcast Radio HAL review notes. 

Bug: b/64229617
Test: instrumentation, VTS
Change-Id: I4009b33eaea6df585f514711f22dfb7fec5ad379
parent 084351dc

broadcastradio/1.1/IBroadcastRadio.hal

+13 −0
Original line number Original line Diff line number Diff line
@@ -40,6 +40,19 @@ interface IBroadcastRadio extends @1.0::IBroadcastRadio { 
     * The data should be a valid PNG, JPEG, GIF or BMP file.

     * The data should be a valid PNG, JPEG, GIF or BMP file.

     * Invalid format must be handled gracefully as if the image was missing.

     * Invalid format must be handled gracefully as if the image was missing.

     *

     *

     * The image identifier may become invalid after some time from passing it

     * with metadata struct (due to resource cleanup at the HAL implementation).

     * However, it must remain valid for a currently tuned program at least

     * until currentProgramInfoChanged or programListChanged is called and

     * metadata changes for the current program.

     *

     * There is still a race condition possible (if the HAL deletes the old

     * image immediately after notifying about the new one) between

     * currentProgramInfoChanged callback propagating through the framework and

     * the HAL implementation removing previous image. In such case, client

     * application may expect the new currentProgramInfoChanged callback with

     * updated image identifier.

     *

     * @param id Identifier of an image;

     * @param id Identifier of an image;

     *           value of 0 is reserved and should be treated as invalid image.

     *           value of 0 is reserved and should be treated as invalid image.

     * @return image A binary blob with image data

     * @return image A binary blob with image data

broadcastradio/1.1/ITuner.hal

+14 −14
Original line number Original line Diff line number Diff line
@@ -39,7 +39,7 @@ interface ITuner extends @1.0::ITuner { 
     *                INVALID_ARGUMENTS if invalid arguments are passed.

     *                INVALID_ARGUMENTS if invalid arguments are passed.

     *                NOT_INITIALIZED if another error occurs.

     *                NOT_INITIALIZED if another error occurs.

     */

     */

    tune_1_1(ProgramSelector program) generates (Result result);

    tuneByProgramSelector(ProgramSelector program) generates (Result result);





    /**

    /**

     * Cancels announcement.

     * Cancels announcement.
@@ -121,19 +121,6 @@ interface ITuner extends @1.0::ITuner { 
    getProgramList(string filter)

    getProgramList(string filter)

        generates (ProgramListResult result, vec<ProgramInfo> programList);

        generates (ProgramListResult result, vec<ProgramInfo> programList);





    /**

     * Checks, if the analog playback is forced, see setAnalogForced.

     *

     * The isForced value is only valid if result was OK.

     *

     * @return result OK if the call succeeded and isForced is valid.

     *                INVALID_STATE if the switch is not supported at current

     *                configuration.

     *                NOT_INITIALIZED if any other error occurs.

     * @return isForced true if analog is forced, false otherwise.

     */

    isAnalogForced() generates (Result result, bool isForced);



    /**

    /**

     * Forces the analog playback for the supporting radio technology.

     * Forces the analog playback for the supporting radio technology.

     *

     *
@@ -150,4 +137,17 @@ interface ITuner extends @1.0::ITuner { 
     *                NOT_INITIALIZED if any other error occurs.

     *                NOT_INITIALIZED if any other error occurs.

     */

     */

    setAnalogForced(bool isForced) generates (Result result);

    setAnalogForced(bool isForced) generates (Result result);



    /**

     * Checks, if the analog playback is forced, see setAnalogForced.

     *

     * The isForced value is only valid if result was OK.

     *

     * @return result OK if the call succeeded and isForced is valid.

     *                INVALID_STATE if the switch is not supported at current

     *                configuration.

     *                NOT_INITIALIZED if any other error occurs.

     * @return isForced true if analog is forced, false otherwise.

     */

    isAnalogForced() generates (Result result, bool isForced);

};
 
};

broadcastradio/1.1/ITunerCallback.hal

+14 −11
Original line number Original line Diff line number Diff line
@@ -29,9 +29,9 @@ interface ITunerCallback extends @1.0::ITunerCallback { 
     * Method called by the HAL when a tuning operation completes

     * Method called by the HAL when a tuning operation completes

     * following a step(), scan() or tune() command.

     * following a step(), scan() or tune() command.

     *

     *

     * This callback supersedes V1_0::tuneComplete. For performance reasons,

     * This callback supersedes V1_0::tuneComplete.

     * the 1.0 callback may not be called when HAL implementation detects 1.1

     * The 1.0 callback must not be called when HAL implementation detects

     * client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).

     * 1.1 client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).

     *

     *

     * @param result OK if tune succeeded or TIMEOUT in case of time out.

     * @param result OK if tune succeeded or TIMEOUT in case of time out.

     * @param selector A ProgramSelector structure describing the tuned station.

     * @param selector A ProgramSelector structure describing the tuned station.
@@ -41,9 +41,9 @@ interface ITunerCallback extends @1.0::ITunerCallback { 
    /**

    /**

     * Method called by the HAL when a frequency switch occurs.

     * Method called by the HAL when a frequency switch occurs.

     *

     *

     * This callback supersedes V1_0::afSwitch. For performance reasons,

     * This callback supersedes V1_0::afSwitch.

     * the 1.0 callback may not be called when HAL implementation detects 1.1

     * The 1.0 callback must not be called when HAL implementation detects

     * client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).

     * 1.1 client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).

     *

     *

     * @param selector A ProgramSelector structure describing the tuned station.

     * @param selector A ProgramSelector structure describing the tuned station.

     */

     */
@@ -73,6 +73,9 @@ interface ITunerCallback extends @1.0::ITunerCallback { 
     * call it immediately, ie. it may wait for a short time to accumulate

     * call it immediately, ie. it may wait for a short time to accumulate

     * multiple list change notifications into a single event.

     * multiple list change notifications into a single event.

     *

     *

     * This callback is only for notifying about insertions and deletions,

     * not about metadata changes.

     *

     * It may be triggered either by an explicitly issued background scan,

     * It may be triggered either by an explicitly issued background scan,

     * or a scan issued by the device internally.

     * or a scan issued by the device internally.

     *

     *
@@ -89,10 +92,10 @@ interface ITunerCallback extends @1.0::ITunerCallback { 
     *

     *

     * This may be called together with tuneComplete_1_1 or afSwitch_1_1.

     * This may be called together with tuneComplete_1_1 or afSwitch_1_1.

     *

     *

     * This callback supersedes V1_0::tuneComplete, V1_0::afSwitch and

     * This callback supersedes V1_0::newMetadata and partly V1_0::tuneComplete

     * newMetadata. For performance reasons, these callbacks may not be called

     * and V1_0::afSwitch.

     * when HAL implementation detects 1.1 client (by casting

     * 1.0 callbacks must not be called when HAL implementation detects

     * V1_0::ITunerCallback to V1_1::ITunerCallback).

     * 1.1 client (by casting V1_0::ITunerCallback to V1_1::ITunerCallback).

     */

     */

    oneway programInfoChanged();

    oneway currentProgramInfoChanged();

};
 
};

broadcastradio/1.1/default/Tuner.cpp

+11 −11
Original line number Original line Diff line number Diff line
@@ -248,10 +248,10 @@ Return<Result> Tuner::tune(uint32_t channel, uint32_t subChannel) { 
        lock_guard<mutex> lk(mMut);

        lock_guard<mutex> lk(mMut);

        band = mAmfmConfig.type;

        band = mAmfmConfig.type;

    }

    }

    return tune_1_1(utils::make_selector(band, channel, subChannel));

    return tuneByProgramSelector(utils::make_selector(band, channel, subChannel));

}

}





Return<Result> Tuner::tune_1_1(const ProgramSelector& sel) {

Return<Result> Tuner::tuneByProgramSelector(const ProgramSelector& sel) {

    ALOGV("%s(%s)", __func__, toString(sel).c_str());

    ALOGV("%s(%s)", __func__, toString(sel).c_str());

    lock_guard<mutex> lk(mMut);

    lock_guard<mutex> lk(mMut);

    if (mIsClosed) return Result::NOT_INITIALIZED;

    if (mIsClosed) return Result::NOT_INITIALIZED;
@@ -336,6 +336,15 @@ Return<void> Tuner::getProgramList(const hidl_string& filter, getProgramList_cb 
    return {};

    return {};

}

}





Return<Result> Tuner::setAnalogForced(bool isForced) {

    ALOGV("%s", __func__);

    lock_guard<mutex> lk(mMut);

    if (mIsClosed) return Result::NOT_INITIALIZED;



    mIsAnalogForced = isForced;

    return Result::OK;

}



Return<void> Tuner::isAnalogForced(isAnalogForced_cb _hidl_cb) {

Return<void> Tuner::isAnalogForced(isAnalogForced_cb _hidl_cb) {

    ALOGV("%s", __func__);

    ALOGV("%s", __func__);

    lock_guard<mutex> lk(mMut);

    lock_guard<mutex> lk(mMut);
@@ -348,15 +357,6 @@ Return<void> Tuner::isAnalogForced(isAnalogForced_cb _hidl_cb) { 
    return {};

    return {};

}

}





Return<Result> Tuner::setAnalogForced(bool isForced) {

    ALOGV("%s", __func__);

    lock_guard<mutex> lk(mMut);

    if (mIsClosed) return Result::NOT_INITIALIZED;



    mIsAnalogForced = isForced;

    return Result::OK;

}



}  // namespace implementation

}  // namespace implementation

}  // namespace V1_1

}  // namespace V1_1

}  // namespace broadcastradio

}  // namespace broadcastradio

broadcastradio/1.1/default/Tuner.h

+2 −2
Original line number Original line Diff line number Diff line
@@ -39,7 +39,7 @@ struct Tuner : public ITuner { 
    virtual Return<Result> scan(V1_0::Direction direction, bool skipSubChannel) override;

    virtual Return<Result> scan(V1_0::Direction direction, bool skipSubChannel) override;

    virtual Return<Result> step(V1_0::Direction direction, bool skipSubChannel) override;

    virtual Return<Result> step(V1_0::Direction direction, bool skipSubChannel) override;

    virtual Return<Result> tune(uint32_t channel, uint32_t subChannel) override;

    virtual Return<Result> tune(uint32_t channel, uint32_t subChannel) override;

    virtual Return<Result> tune_1_1(const ProgramSelector& program) override;

    virtual Return<Result> tuneByProgramSelector(const ProgramSelector& program) override;

    virtual Return<Result> cancel() override;

    virtual Return<Result> cancel() override;

    virtual Return<Result> cancelAnnouncement() override;

    virtual Return<Result> cancelAnnouncement() override;

    virtual Return<void> getProgramInformation(getProgramInformation_cb _hidl_cb) override;

    virtual Return<void> getProgramInformation(getProgramInformation_cb _hidl_cb) override;
@@ -47,8 +47,8 @@ struct Tuner : public ITuner { 
    virtual Return<ProgramListResult> startBackgroundScan() override;

    virtual Return<ProgramListResult> startBackgroundScan() override;

    virtual Return<void> getProgramList(const hidl_string& filter,

    virtual Return<void> getProgramList(const hidl_string& filter,

                                        getProgramList_cb _hidl_cb) override;

                                        getProgramList_cb _hidl_cb) override;

    virtual Return<void> isAnalogForced(isAnalogForced_cb _hidl_cb) override;

    virtual Return<Result> setAnalogForced(bool isForced) override;

    virtual Return<Result> setAnalogForced(bool isForced) override;

    virtual Return<void> isAnalogForced(isAnalogForced_cb _hidl_cb) override;





   private:

   private:

    std::mutex mMut;

    std::mutex mMut;