diff options
author | Tomasz Wasilczyk <twasilczyk@google.com> | 2018-01-04 20:17:28 +0000 |
---|---|---|
committer | Android (Google) Code Review <android-gerrit@google.com> | 2018-01-04 20:17:28 +0000 |
commit | 38b51669e46addd07aa53a4847c2fa7aae9374df (patch) | |
tree | c1199aad65b5a8fb2b307d5667c194994a7f0a0c /broadcastradio | |
parent | 3fe18f6c0190bc46997e7d53793fd6b2afac48f9 (diff) | |
parent | c71624f8ed57962fe85656aac5b687d8394dfdc0 (diff) | |
download | android_hardware_interfaces-38b51669e46addd07aa53a4847c2fa7aae9374df.tar.gz android_hardware_interfaces-38b51669e46addd07aa53a4847c2fa7aae9374df.tar.bz2 android_hardware_interfaces-38b51669e46addd07aa53a4847c2fa7aae9374df.zip |
Merge "Prepare a best-effort workaround for HD Radio station id abuse."
Diffstat (limited to 'broadcastradio')
-rw-r--r-- | broadcastradio/2.0/types.hal | 25 | ||||
-rw-r--r-- | broadcastradio/2.0/vts/functional/Android.bp | 3 | ||||
-rw-r--r-- | broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp | 57 | ||||
-rw-r--r-- | broadcastradio/common/tests/Android.bp | 7 | ||||
-rw-r--r-- | broadcastradio/common/tests/ProgramIdentifier_test.cpp | 40 | ||||
-rw-r--r-- | broadcastradio/common/utils2x/Android.bp | 3 | ||||
-rw-r--r-- | broadcastradio/common/utils2x/Utils.cpp | 43 | ||||
-rw-r--r-- | broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h | 6 |
8 files changed, 172 insertions, 12 deletions
diff --git a/broadcastradio/2.0/types.hal b/broadcastradio/2.0/types.hal index d77e8c12f..c5880e27e 100644 --- a/broadcastradio/2.0/types.hal +++ b/broadcastradio/2.0/types.hal @@ -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), @@ -478,6 +483,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. * * Consists of (from the LSB): @@ -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, diff --git a/broadcastradio/2.0/vts/functional/Android.bp b/broadcastradio/2.0/vts/functional/Android.bp index 6017b1529..6940bca49 100644 --- a/broadcastradio/2.0/vts/functional/Android.bp +++ b/broadcastradio/2.0/vts/functional/Android.bp @@ -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", diff --git a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp index aa75442f0..111147871 100644 --- a/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp +++ b/broadcastradio/2.0/vts/functional/VtsHalBroadcastradioV2_0TargetTest.cpp @@ -30,6 +30,7 @@ #include <gmock/gmock.h> #include <chrono> +#include <optional> #include <regex> namespace android { @@ -100,6 +101,7 @@ class BroadcastRadioHalTest : public ::testing::VtsHalHidlTargetTestBase { bool openSession(); bool getAmFmRegionConfig(bool full, AmFmRegionConfig* config); + std::optional<utils::ProgramInfoSet> getProgramList(); sp<IBroadcastRadio> mModule; Properties mProperties; @@ -182,6 +184,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. * @@ -649,19 +670,35 @@ TEST_F(BroadcastRadioHalTest, SetConfigFlags) { TEST_F(BroadcastRadioHalTest, GetProgramList) { ASSERT_TRUE(openSession()); - EXPECT_TIMEOUT_CALL(*mCallback, onProgramListReady).Times(AnyNumber()); + getProgramList(); +} - auto startResult = mSession->startProgramListUpdates({}); - if (startResult == Result::NOT_SUPPORTED) { - printSkipped("Program list not supported"); - return; - } - ASSERT_EQ(Result::OK, startResult); +/** + * 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()); - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onProgramListReady, timeout::programListScan); + auto list = getProgramList(); + if (!list) return; - auto stopResult = mSession->stopProgramListUpdates(); - EXPECT_TRUE(stopResult.isOk()); + 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]); + } } /** diff --git a/broadcastradio/common/tests/Android.bp b/broadcastradio/common/tests/Android.bp index 512c02e49..f6a3b6f4d 100644 --- a/broadcastradio/common/tests/Android.bp +++ b/broadcastradio/common/tests/Android.bp @@ -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", diff --git a/broadcastradio/common/tests/ProgramIdentifier_test.cpp b/broadcastradio/common/tests/ProgramIdentifier_test.cpp new file mode 100644 index 000000000..51ad0145e --- /dev/null +++ b/broadcastradio/common/tests/ProgramIdentifier_test.cpp @@ -0,0 +1,40 @@ +/* + * 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 diff --git a/broadcastradio/common/utils2x/Android.bp b/broadcastradio/common/utils2x/Android.bp index c6b94afb0..aab94f2a1 100644 --- a/broadcastradio/common/utils2x/Android.bp +++ b/broadcastradio/common/utils2x/Android.bp @@ -23,6 +23,9 @@ cc_library_static { "-Wextra", "-Werror", ], + cppflags: [ + "-std=c++1z", + ], srcs: [ "Utils.cpp", ], diff --git a/broadcastradio/common/utils2x/Utils.cpp b/broadcastradio/common/utils2x/Utils.cpp index d825a7a00..6fe95549d 100644 --- a/broadcastradio/common/utils2x/Utils.cpp +++ b/broadcastradio/common/utils2x/Utils.cpp @@ -245,6 +245,15 @@ bool isValid(const ProgramIdentifier& id) { expect(freq < 10000000u, "f < 10GHz"); break; } + case IdentifierType::HD_STATION_NAME: { + while (val > 0) { + auto ch = static_cast<char>(val & 0xFF); + val >>= 8; + expect((ch >= '0' && ch <= '9') || (ch >= 'A' && ch <= 'Z'), + "HD_STATION_NAME does not match [A-Z0-9]+"); + } + break; + } case IdentifierType::DAB_SID_EXT: { auto sid = val & 0xFFFF; // 16bit val >>= 16; @@ -367,6 +376,40 @@ void updateProgramList(ProgramInfoSet& list, const ProgramListChunk& chunk) { } } +std::optional<std::string> getMetadataString(const V2_0::ProgramInfo& info, + const V2_0::MetadataKey key) { + auto isKey = [key](const V2_0::Metadata& item) { + return static_cast<V2_0::MetadataKey>(item.key) == key; + }; + + auto it = std::find_if(info.metadata.begin(), info.metadata.end(), isKey); + if (it == info.metadata.end()) return std::nullopt; + + return it->stringValue; +} + +V2_0::ProgramIdentifier make_hdradio_station_name(const std::string& name) { + constexpr size_t maxlen = 8; + + std::string shortName; + shortName.reserve(maxlen); + + auto&& loc = std::locale::classic(); + for (char ch : name) { + if (!std::isalnum(ch, loc)) continue; + shortName.push_back(std::toupper(ch, loc)); + if (shortName.length() >= maxlen) break; + } + + uint64_t val = 0; + for (auto rit = shortName.rbegin(); rit != shortName.rend(); ++rit) { + val <<= 8; + val |= static_cast<uint8_t>(*rit); + } + + return make_identifier(IdentifierType::HD_STATION_NAME, val); +} + } // namespace utils } // namespace broadcastradio } // namespace hardware diff --git a/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h b/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h index e3134f7b8..5e5194163 100644 --- a/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h +++ b/broadcastradio/common/utils2x/include/broadcastradio-utils-2x/Utils.h @@ -18,6 +18,7 @@ #include <android/hardware/broadcastradio/2.0/types.h> #include <chrono> +#include <optional> #include <queue> #include <thread> #include <unordered_set> @@ -146,6 +147,11 @@ typedef std::unordered_set<V2_0::ProgramInfo, ProgramInfoHasher, ProgramInfoKeyE void updateProgramList(ProgramInfoSet& list, const V2_0::ProgramListChunk& chunk); +std::optional<std::string> getMetadataString(const V2_0::ProgramInfo& info, + const V2_0::MetadataKey key); + +V2_0::ProgramIdentifier make_hdradio_station_name(const std::string& name); + } // namespace utils } // namespace broadcastradio } // namespace hardware |