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

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

Prepare a best-effort workaround for HD Radio station id abuse. 

In theory, 32bit HD Radio station ID + subchannel index (parts of
HD_STATION_ID_EXT) is a globally unique identifier. It allows broadcast
radio framework to determine which programs are the same and allow the
application to match entries from favourite list and the program list
provided by the tuner.

However, some broadcasters don't perform equipment setup correctly and
don't set station ID. As a result, there are some stations with
conflicting IDs.

As a workaround to treat these stations separately in a given location,
FM frequency was added as a part of HD_STATION_ID_EXT. This still doesn't
solve the global uniqueness problem: user might save KCQW 105.5 (sid=0) in
California, travel to Nevada and find KNAB 105.5 (sid=0). It turns out
there is no reliable identifier that might identify the station globally.

As a workaround, shortened station name is added for double-checking.
This is a best-effort fix, so it's not required for such misbehaving
stations to get correctly identified in every corner case.

Bug: 69958777
Test: VTS
Change-Id: Id11243096f1cde7fdda5cb70a7248d1831985cdd
parent 0914a946

broadcastradio/2.0/types.hal

+23 −2
Original line number Diff line number Diff line
@@ -466,7 +466,12 @@ enum IdentifierType : uint32_t { 
     * Consists of (from the LSB):

     * - 32bit: Station ID number;

     * - 4bit: HD Radio subchannel;

     * - 18bit: AMFM_FREQUENCY. // TODO(b/69958777): is it necessary?

     * - 18bit: AMFM_FREQUENCY.

     *

     * While station ID number should be unique globally, it sometimes get

     * abused by broadcasters (i.e. not being set at all). To ensure local

     * uniqueness, AMFM_FREQUENCY was added here. Global uniqueness is

     * a best-effort - see HD_STATION_NAME.

     *

     * HD Radio subchannel is a value in range 0-7.

     * This index is 0-based (where 0 is MPS and 1..7 are SPS),
@@ -477,6 +482,22 @@ enum IdentifierType : uint32_t { 
     */

    HD_STATION_ID_EXT,



    /**

     * 64bit additional identifier for HD Radio.

     *

     * Due to Station ID abuse, some HD_STATION_ID_EXT identifiers may be not

     * globally unique. To provide a best-effort solution, a short version of

     * station name may be carried as additional identifier and may be used

     * by the tuner hardware to double-check tuning.

     *

     * The name is limited to the first 8 A-Z0-9 characters (lowercase letters

     * must be converted to uppercase). Encoded in little-endian ASCII:

     * the first character of the name is the LSB.

     *

     * For example: "Abc" is encoded as 0x434241.

     */

    HD_STATION_NAME,



    /**

     * 28bit compound primary identifier for Digital Audio Broadcasting.

     *
@@ -492,7 +513,7 @@ enum IdentifierType : uint32_t { 
     * The remaining bits should be set to zeros when writing on the chip side

     * and ignored when read.

     */

    DAB_SID_EXT = HD_STATION_ID_EXT + 2,

    DAB_SID_EXT,



    /** 16bit */

    DAB_ENSEMBLE,

broadcastradio/2.0/vts/functional/Android.bp

+3 −0
Original line number Diff line number Diff line
@@ -17,6 +17,9 @@ 
cc_test {

    name: "VtsHalBroadcastradioV2_0TargetTest",

    defaults: ["VtsHalTargetTestDefaults"],

    cppflags: [

        "-std=c++1z",

    ],

    srcs: ["VtsHalBroadcastradioV2_0TargetTest.cpp"],

    static_libs: [

        "android.hardware.broadcastradio@2.0",

broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp

+47 −10
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 
#include <gmock/gmock.h>



#include <chrono>

#include <optional>

#include <regex>



namespace android {
@@ -96,6 +97,7 @@ class BroadcastRadioHalTest : public ::testing::VtsHalHidlTargetTestBase { 


    bool openSession();

    bool getAmFmRegionConfig(bool full, AmFmRegionConfig* config);

    std::optional<utils::ProgramInfoSet> getProgramList();



    sp<IBroadcastRadio> mModule;

    Properties mProperties;
@@ -178,6 +180,25 @@ bool BroadcastRadioHalTest::getAmFmRegionConfig(bool full, AmFmRegionConfig* con 
    return halResult == Result::OK;

}



std::optional<utils::ProgramInfoSet> BroadcastRadioHalTest::getProgramList() {

    EXPECT_TIMEOUT_CALL(*mCallback, onProgramListReady).Times(AnyNumber());



    auto startResult = mSession->startProgramListUpdates({});

    if (startResult == Result::NOT_SUPPORTED) {

        printSkipped("Program list not supported");

        return nullopt;

    }

    EXPECT_EQ(Result::OK, startResult);

    if (startResult != Result::OK) return nullopt;



    EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onProgramListReady, timeout::programListScan);



    auto stopResult = mSession->stopProgramListUpdates();

    EXPECT_TRUE(stopResult.isOk());



    return mCallback->mProgramList;

}



/**

 * Test session opening.

 *
@@ -645,19 +666,35 @@ TEST_F(BroadcastRadioHalTest, SetConfigFlags) { 
TEST_F(BroadcastRadioHalTest, GetProgramList) {

    ASSERT_TRUE(openSession());



    EXPECT_TIMEOUT_CALL(*mCallback, onProgramListReady).Times(AnyNumber());



    auto startResult = mSession->startProgramListUpdates({});

    if (startResult == Result::NOT_SUPPORTED) {

        printSkipped("Program list not supported");

        return;

    getProgramList();

}

    ASSERT_EQ(Result::OK, startResult);



    EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onProgramListReady, timeout::programListScan);

/**

 * Test HD_STATION_NAME correctness.

 *

 * Verifies that if a program on the list contains HD_STATION_NAME identifier:

 *  - the program provides station name in its metadata;

 *  - the identifier matches the name;

 *  - there is only one identifier of that type.

 */

TEST_F(BroadcastRadioHalTest, HdRadioStationNameId) {

    ASSERT_TRUE(openSession());



    auto stopResult = mSession->stopProgramListUpdates();

    EXPECT_TRUE(stopResult.isOk());

    auto list = getProgramList();

    if (!list) return;



    for (auto&& program : *list) {

        auto nameIds = utils::getAllIds(program.selector, IdentifierType::HD_STATION_NAME);

        EXPECT_LE(nameIds.size(), 1u);

        if (nameIds.size() == 0) continue;



        auto name = utils::getMetadataString(program, MetadataKey::PROGRAM_NAME);

        if (!name) name = utils::getMetadataString(program, MetadataKey::RDS_PS);

        ASSERT_TRUE(name.has_value());



        auto expectedId = utils::make_hdradio_station_name(*name);

        EXPECT_EQ(expectedId.value, nameIds[0]);

    }

}



// TODO(b/70939328): test ProgramInfo's currentlyTunedId and

broadcastradio/common/tests/Android.bp

+7 −0
Original line number Diff line number Diff line
@@ -22,6 +22,9 @@ cc_test { 
        "-Wextra",

        "-Werror",

    ],

    cppflags: [

        "-std=c++1z",

    ],

    srcs: [

        "CommonXX_test.cpp",

    ],
@@ -43,8 +46,12 @@ cc_test { 
        "-Wextra",

        "-Werror",

    ],

    cppflags: [

        "-std=c++1z",

    ],

    srcs: [

        "IdentifierIterator_test.cpp",

        "ProgramIdentifier_test.cpp",

    ],

    static_libs: [

        "android.hardware.broadcastradio@common-utils-2x-lib",

broadcastradio/common/tests/ProgramIdentifier_test.cpp

0 → 100644
+40 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2017 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#include <broadcastradio-utils-2x/Utils.h>

#include <gtest/gtest.h>



#include <optional>



namespace {



namespace utils = android::hardware::broadcastradio::utils;



TEST(ProgramIdentifierTest, hdRadioStationName) {

    auto verify = [](std::string name, uint64_t nameId) {

        auto id = utils::make_hdradio_station_name(name);

        EXPECT_EQ(nameId, id.value) << "Failed to convert '" << name << "'";

    };



    verify("", 0);

    verify("Abc", 0x434241);

    verify("Some Station 1", 0x54415453454d4f53);

    verify("Station1", 0x314e4f4954415453);

    verify("!@#$%^&*()_+", 0);

    verify("-=[]{};':\"0", 0x30);

}



}  // anonymous namespace