From dfaa9c5e5c719dfd81aed83f12d398b2983eda52 Mon Sep 17 00:00:00 2001 From: Mickey Keeley Date: Thu, 26 Apr 2018 15:05:25 -0700 Subject: [PATCH] BootParameters: Use new JSON reader * Update JSON blob that BootParameters reads from. * Change JSON reader to allow custom keys. Bug: 78524407 Test: Builds. Unit tests pass. End to end flow with RebootActivity passes values as expected. Change-Id: I2966f560de4aaf045125946c7fbe1becd47354be --- cmds/bootanimation/Android.mk | 3 + cmds/bootanimation/iot/Android.mk | 3 + cmds/bootanimation/iot/BootParameters.cpp | 78 +++++++++++--------------- cmds/bootanimation/iot/BootParameters.h | 19 +------ cmds/bootanimation/iot/BootParameters_test.cpp | 30 ++++++---- 5 files changed, 61 insertions(+), 72 deletions(-) diff --git a/cmds/bootanimation/Android.mk b/cmds/bootanimation/Android.mk index a7349eebea21..453fb727bab8 100644 --- a/cmds/bootanimation/Android.mk +++ b/cmds/bootanimation/Android.mk @@ -29,6 +29,9 @@ LOCAL_SHARED_LIBRARIES += \ libandroidthings \ libchrome \ +LOCAL_STATIC_LIBRARIES += \ + libjsoncpp + LOCAL_SRC_FILES += \ iot/iotbootanimation_main.cpp \ iot/BootAction.cpp \ diff --git a/cmds/bootanimation/iot/Android.mk b/cmds/bootanimation/iot/Android.mk index 8f5cfb371576..8b475d3f3c95 100644 --- a/cmds/bootanimation/iot/Android.mk +++ b/cmds/bootanimation/iot/Android.mk @@ -29,6 +29,9 @@ LOCAL_SHARED_LIBRARIES := \ libchrome \ liblog \ +LOCAL_STATIC_LIBRARIES += \ + libjsoncpp + LOCAL_SRC_FILES := \ BootParameters.cpp \ BootParameters_test.cpp \ diff --git a/cmds/bootanimation/iot/BootParameters.cpp b/cmds/bootanimation/iot/BootParameters.cpp index 06cdbf8c33bb..995a3aafae1c 100644 --- a/cmds/bootanimation/iot/BootParameters.cpp +++ b/cmds/bootanimation/iot/BootParameters.cpp @@ -18,29 +18,25 @@ #define LOG_TAG "BootParameters" +#include #include #include -#include -#include -#include #include using android::base::RemoveFileIfExists; using android::base::ReadFileToString; -using base::JSONReader; -using base::JSONValueConverter; -using base::Value; +using Json::Reader; +using Json::Value; namespace android { namespace { -// Brightness and volume are stored as integer strings in next_boot.json. -// They are divided by this constant to produce the actual float values in -// range [0.0, 1.0]. This constant must match its counterpart in -// DeviceManager. -constexpr const float kFloatScaleFactor = 1000.0f; +// Keys for volume, brightness, and user-defined parameters. +constexpr const char* kKeyVolume = "volume"; +constexpr const char* kKeyBrightness = "brightness"; +constexpr const char* kKeyParams = "params"; constexpr const char* kNextBootFile = "/data/misc/bootanimation/next_boot.json"; constexpr const char* kLastBootFile = "/data/misc/bootanimation/last_boot.json"; @@ -69,19 +65,6 @@ void swapBootConfigs() { } // namespace -BootParameters::SavedBootParameters::SavedBootParameters() - : brightness(-kFloatScaleFactor), volume(-kFloatScaleFactor) {} - -void BootParameters::SavedBootParameters::RegisterJSONConverter( - JSONValueConverter* converter) { - converter->RegisterIntField("brightness", &SavedBootParameters::brightness); - converter->RegisterIntField("volume", &SavedBootParameters::volume); - converter->RegisterRepeatedString("param_names", - &SavedBootParameters::param_names); - converter->RegisterRepeatedString("param_values", - &SavedBootParameters::param_values); -} - BootParameters::BootParameters() { swapBootConfigs(); loadParameters(); @@ -100,27 +83,34 @@ void BootParameters::loadParameters() { } void BootParameters::loadParameters(const std::string& raw_json) { - std::unique_ptr json = JSONReader::Read(raw_json); - if (json.get() == nullptr) { - return; - } - - JSONValueConverter converter; - if (converter.Convert(*(json.get()), &mRawParameters)) { - mBrightness = mRawParameters.brightness / kFloatScaleFactor; - mVolume = mRawParameters.volume / kFloatScaleFactor; - - if (mRawParameters.param_names.size() == mRawParameters.param_values.size()) { - for (size_t i = 0; i < mRawParameters.param_names.size(); i++) { - mParameters.push_back({ - .key = mRawParameters.param_names[i]->c_str(), - .value = mRawParameters.param_values[i]->c_str() - }); - } - } else { - ALOGW("Parameter names and values size mismatch"); - } + if (!Reader().parse(raw_json, mJson)) { + return; + } + + // A missing key returns a safe, missing value. + // Ignore invalid or missing JSON parameters. + Value& jsonValue = mJson[kKeyVolume]; + if (jsonValue.isDouble()) { + mVolume = jsonValue.asFloat(); + } + + jsonValue = mJson[kKeyBrightness]; + if (jsonValue.isDouble()) { + mBrightness = jsonValue.asFloat(); + } + + jsonValue = mJson[kKeyParams]; + if (jsonValue.isObject()) { + for (auto &key : jsonValue.getMemberNames()) { + Value& value = jsonValue[key]; + if (value.isString()) { + mParameters.push_back({ + .key = key.c_str(), + .value = value.asCString() + }); + } } + } } } // namespace android diff --git a/cmds/bootanimation/iot/BootParameters.h b/cmds/bootanimation/iot/BootParameters.h index 50e5d570b628..f421790ab321 100644 --- a/cmds/bootanimation/iot/BootParameters.h +++ b/cmds/bootanimation/iot/BootParameters.h @@ -21,8 +21,8 @@ #include #include -#include #include // libandroidthings native API. +#include namespace android { @@ -47,27 +47,14 @@ public: // Exposed for testing. Updates the parameters with new JSON values. void loadParameters(const std::string& raw_json); private: - // Raw boot saved_parameters loaded from .json. - struct SavedBootParameters { - int brightness; - int volume; - std::vector> param_names; - std::vector> param_values; - - SavedBootParameters(); - static void RegisterJSONConverter( - ::base::JSONValueConverter* converter); - }; - void loadParameters(); float mVolume = -1.f; float mBrightness = -1.f; std::vector mParameters; - // ABootActionParameter is just a raw pointer so we need to keep the - // original strings around to avoid losing them. - SavedBootParameters mRawParameters; + // Store parsed JSON because mParameters makes a shallow copy. + Json::Value mJson; }; } // namespace android diff --git a/cmds/bootanimation/iot/BootParameters_test.cpp b/cmds/bootanimation/iot/BootParameters_test.cpp index 431563cdd173..9a0311318812 100644 --- a/cmds/bootanimation/iot/BootParameters_test.cpp +++ b/cmds/bootanimation/iot/BootParameters_test.cpp @@ -26,10 +26,12 @@ TEST(BootParametersTest, TestParseValidParameters) { BootParameters boot_parameters = BootParameters(); boot_parameters.loadParameters(R"( { - "brightness":200, - "volume":100, - "param_names":["key1","key2"], - "param_values":["value1","value2"] + "brightness":0.2, + "volume":0.1, + "params":{ + "key1":"value1", + "key2":"value2" + } } )"); @@ -46,14 +48,16 @@ TEST(BootParametersTest, TestParseValidParameters) { ASSERT_STREQ(parameters[1].value, "value2"); } -TEST(BootParametersTest, TestMismatchedParameters) { +TEST(BootParametersTest, TestMalformedParametersAreSkipped) { BootParameters boot_parameters = BootParameters(); boot_parameters.loadParameters(R"( { - "brightness":500, - "volume":500, - "param_names":["key1","key2"], - "param_values":["value1"] + "brightness":0.5, + "volume":0.5, + "params": { + "key1":1, + "key2":"value2" + } } )"); @@ -63,14 +67,16 @@ TEST(BootParametersTest, TestMismatchedParameters) { EXPECT_FLOAT_EQ(0.5f, boot_parameters.getVolume()); auto parameters = boot_parameters.getParameters(); - ASSERT_EQ(0u, parameters.size()); + ASSERT_EQ(1u, parameters.size()); + ASSERT_STREQ(parameters[0].key, "key2"); + ASSERT_STREQ(parameters[0].value, "value2"); } -TEST(BootParametersTest, TestMissingParameters) { +TEST(BootParametersTest, TestMissingParametersHaveDefaults) { BootParameters boot_parameters = BootParameters(); boot_parameters.loadParameters(R"( { - "brightness":500 + "brightness":0.5 } )"); -- 2.11.0