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

Commit 0cac55bd authored by Shrikar Amirisetty's avatar Shrikar Amirisetty Committed by Android (Google) Code Review
Browse files

Merge "Updated JsonConfigLoader to parse access for area configs" into main

parents 3f0bccf1 6a4f3a87

automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/include/JsonConfigLoader.h

+7 −8
Original line number Diff line number Diff line
@@ -130,11 +130,8 @@ class JsonConfigParser { 
                                     std::vector<T>* outPtr, std::vector<std::string>* errors);

    // Parses a JSON field to VehiclePropertyAccess or VehiclePropertyChangeMode.

    template <class T>

    void parseAccessChangeMode(

            const Json::Value& parentJsonNode, const std::string& fieldName, int propId,

            const std::string& propStr,

            const std::unordered_map<aidl::android::hardware::automotive::vehicle::VehicleProperty,

                                     T>& defaultMap,

    void parseAccessChangeMode(const Json::Value& parentJsonNode, const std::string& fieldName,

                               const std::string& propStr, const T* defaultAccessChangeModePtr,

                               T* outPtr, std::vector<std::string>* errors);



    // Parses a JSON field to RawPropValues.
@@ -145,8 +142,10 @@ class JsonConfigParser { 
                         std::vector<std::string>* errors);



    // Prase a JSON field as an array of area configs.

    void parseAreas(const Json::Value& parentJsonNode, const std::string& fieldName,

                    ConfigDeclaration* outPtr, std::vector<std::string>* errors);

    void parseAreas(

            const Json::Value& parentJsonNode, const std::string& fieldName,

            ConfigDeclaration* outPtr, std::vector<std::string>* errors,

            aidl::android::hardware::automotive::vehicle::VehiclePropertyAccess defaultAccess);

};



}  // namespace jsonconfigloader_impl

automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/src/JsonConfigLoader.cpp

+33 −18
Original line number Diff line number Diff line
@@ -487,10 +487,11 @@ bool JsonConfigParser::tryParseJsonArrayToVariable(const Json::Value& parentJson 
}



template <class T>

void JsonConfigParser::parseAccessChangeMode(

        const Json::Value& parentJsonNode, const std::string& fieldName, int propId,

        const std::string& propStr, const std::unordered_map<VehicleProperty, T>& defaultMap,

        T* outPtr, std::vector<std::string>* errors) {

void JsonConfigParser::parseAccessChangeMode(const Json::Value& parentJsonNode,

                                             const std::string& fieldName,

                                             const std::string& propStr,

                                             const T* defaultAccessChangeModeValuePtr, T* outPtr,

                                             std::vector<std::string>* errors) {

    if (!parentJsonNode.isObject()) {

        errors->push_back("Node: " + parentJsonNode.toStyledString() + " is not an object");

        return;
@@ -504,12 +505,11 @@ void JsonConfigParser::parseAccessChangeMode( 
        *outPtr = static_cast<T>(result.value());

        return;

    }

    auto it = defaultMap.find(static_cast<VehicleProperty>(propId));

    if (it == defaultMap.end()) {

    if (defaultAccessChangeModeValuePtr == NULL) {

        errors->push_back("No " + fieldName + " specified for property: " + propStr);

        return;

    }

    *outPtr = it->second;

    *outPtr = *defaultAccessChangeModeValuePtr;

    return;

}
@@ -538,7 +538,8 @@ bool JsonConfigParser::parsePropValues(const Json::Value& parentJsonNode, 
}



void JsonConfigParser::parseAreas(const Json::Value& parentJsonNode, const std::string& fieldName,

                                  ConfigDeclaration* config, std::vector<std::string>* errors) {

                                  ConfigDeclaration* config, std::vector<std::string>* errors,

                                  VehiclePropertyAccess defaultAccess) {

    if (!parentJsonNode.isObject()) {

        errors->push_back("Node: " + parentJsonNode.toStyledString() + " is not an object");

        return;
@@ -546,6 +547,7 @@ void JsonConfigParser::parseAreas(const Json::Value& parentJsonNode, const std:: 
    if (!parentJsonNode.isMember(fieldName)) {

        return;

    }

    std::string propStr = parentJsonNode["property"].toStyledString();

    const Json::Value& jsonValue = parentJsonNode[fieldName];



    if (!jsonValue.isArray()) {
@@ -561,6 +563,8 @@ void JsonConfigParser::parseAreas(const Json::Value& parentJsonNode, const std:: 
        }

        VehicleAreaConfig areaConfig = {};

        areaConfig.areaId = areaId;

        parseAccessChangeMode(jsonAreaConfig, "access", propStr, &defaultAccess, &areaConfig.access,

                              errors);

        tryParseJsonValueToVariable(jsonAreaConfig, "minInt32Value", /*optional=*/true,

                                    &areaConfig.minInt32Value, errors);

        tryParseJsonValueToVariable(jsonAreaConfig, "maxInt32Value", /*optional=*/true,
@@ -608,12 +612,21 @@ std::optional<ConfigDeclaration> JsonConfigParser::parseEachProperty( 


    configDecl.config.prop = propId;

    std::string propStr = propJsonValue["property"].toStyledString();

    VehiclePropertyAccess* defaultAccessMode = NULL;

    auto itAccess = AccessForVehicleProperty.find(static_cast<VehicleProperty>(propId));

    if (itAccess != AccessForVehicleProperty.end()) {

        defaultAccessMode = &itAccess->second;

    }

    VehiclePropertyChangeMode* defaultChangeMode = NULL;

    auto itChangeMode = ChangeModeForVehicleProperty.find(static_cast<VehicleProperty>(propId));

    if (itChangeMode != ChangeModeForVehicleProperty.end()) {

        defaultChangeMode = &itChangeMode->second;

    }

    VehiclePropertyAccess access = VehiclePropertyAccess::NONE;

    parseAccessChangeMode(propJsonValue, "access", propStr, defaultAccessMode, &access, errors);



    parseAccessChangeMode(propJsonValue, "access", propId, propStr, AccessForVehicleProperty,

                          &configDecl.config.access, errors);



    parseAccessChangeMode(propJsonValue, "changeMode", propId, propStr,

                          ChangeModeForVehicleProperty, &configDecl.config.changeMode, errors);

    parseAccessChangeMode(propJsonValue, "changeMode", propStr, defaultChangeMode,

                          &configDecl.config.changeMode, errors);



    tryParseJsonValueToVariable(propJsonValue, "configString", /*optional=*/true,

                                &configDecl.config.configString, errors);
@@ -629,21 +642,23 @@ std::optional<ConfigDeclaration> JsonConfigParser::parseEachProperty( 
    tryParseJsonValueToVariable(propJsonValue, "maxSampleRate", /*optional=*/true,

                                &configDecl.config.maxSampleRate, errors);



    parseAreas(propJsonValue, "areas", &configDecl, errors);



    if (errors->size() != initialErrorCount) {

        return std::nullopt;

    }

    parseAreas(propJsonValue, "areas", &configDecl, errors, access);



    // If there is no area config, by default we allow variable update rate, so we have to add

    // a global area config.

    if (configDecl.config.areaConfigs.size() == 0) {

        VehicleAreaConfig areaConfig = {

                .areaId = 0,

                .access = access,

                .supportVariableUpdateRate = true,

        };

        configDecl.config.areaConfigs.push_back(std::move(areaConfig));

    }



    if (errors->size() != initialErrorCount) {

        return std::nullopt;

    }



    return configDecl;

}

automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/test/JsonConfigLoaderUnitTest.cpp

+108 −4
Original line number Diff line number Diff line
@@ -286,7 +286,8 @@ TEST_F(JsonConfigLoaderUnitTest, testCheckDefaultAccessChangeMode) { 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& propConfig = configs.begin()->second.config;

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::READ);

    ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::STATIC);

}
@@ -307,7 +308,8 @@ TEST_F(JsonConfigLoaderUnitTest, testAccessOverride) { 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& propConfig = configs.begin()->second.config;

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::WRITE);

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::WRITE);

    ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::STATIC);

}
@@ -328,7 +330,8 @@ TEST_F(JsonConfigLoaderUnitTest, testChangeModeOverride) { 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& propConfig = configs.begin()->second.config;

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::READ);

    ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::ON_CHANGE);

}
@@ -350,7 +353,8 @@ TEST_F(JsonConfigLoaderUnitTest, testCustomProp) { 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& propConfig = configs.begin()->second.config;

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::WRITE);

    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::WRITE);

    ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::ON_CHANGE);

}
@@ -550,10 +554,12 @@ TEST_F(JsonConfigLoaderUnitTest, testAreas_Simple) { 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& config = configs.begin()->second.config;

    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(config.areaConfigs.size(), 1u);

    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];

    ASSERT_EQ(areaConfig.minInt32Value, 1);

    ASSERT_EQ(areaConfig.maxInt32Value, 7);

    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(areaConfig.areaId, HVAC_ALL);

}
@@ -635,9 +641,11 @@ TEST_F(JsonConfigLoaderUnitTest, testAreas_HandlesNoSupportedEnumValuesDeclared) 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& config = configs.begin()->second.config;

    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(config.areaConfigs.size(), 1u);



    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];

    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(areaConfig.areaId, 0);

    ASSERT_FALSE(areaConfig.supportedEnumValues);

}
@@ -662,9 +670,11 @@ TEST_F(JsonConfigLoaderUnitTest, testAreas_HandlesSupportedEnumValues) { 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& config = configs.begin()->second.config;

    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(config.areaConfigs.size(), 1u);



    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];

    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(areaConfig.areaId, 0);

    ASSERT_TRUE(areaConfig.supportedEnumValues);

    ASSERT_EQ(areaConfig.supportedEnumValues.value().size(), 2u);
@@ -692,13 +702,107 @@ TEST_F(JsonConfigLoaderUnitTest, testAreas_HandlesEmptySupportedEnumValues) { 
    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& config = configs.begin()->second.config;

    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(config.areaConfigs.size(), 1u);



    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];

    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(areaConfig.areaId, 0);

    ASSERT_FALSE(areaConfig.supportedEnumValues);

}



TEST_F(JsonConfigLoaderUnitTest, testAccess_areaOverrideGlobalDefault) {

    std::istringstream iss(R"(

    {

        "properties": [{

            "property": "VehicleProperty::CABIN_LIGHTS_SWITCH",

            "areas": [{

                "access": "VehiclePropertyAccess::READ",

                "areaId": 0

            }]

        }]

    }

    )");



    auto result = mLoader.loadPropConfig(iss);

    ASSERT_TRUE(result.ok());



    auto configs = result.value();

    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& config = configs.begin()->second.config;

    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(config.areaConfigs.size(), 1u);



    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];

    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(areaConfig.areaId, 0);

}



TEST_F(JsonConfigLoaderUnitTest, testAccess_globalOverrideDefault) {

    std::istringstream iss(R"(

    {

        "properties": [{

            "property": "VehicleProperty::CABIN_LIGHTS_SWITCH",

            "areas": [{

                "areaId": 0

            }],

            "access": "VehiclePropertyAccess::READ"

        }]

    }

    )");



    auto result = mLoader.loadPropConfig(iss);

    ASSERT_TRUE(result.ok());



    auto configs = result.value();

    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& config = configs.begin()->second.config;

    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(config.areaConfigs.size(), 1u);



    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];

    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(areaConfig.areaId, 0);

}



TEST_F(JsonConfigLoaderUnitTest, testAccess_areaOverrideGlobal) {

    std::istringstream iss(R"(

    {

        "properties": [{

            "property": "VehicleProperty::CABIN_LIGHTS_SWITCH",

            "areas": [{

                "access": "VehiclePropertyAccess::WRITE",

                "areaId": 0

            },

            {

                "areaId": 1

            }],

            "access": "VehiclePropertyAccess::READ",

        }]

    }

    )");



    auto result = mLoader.loadPropConfig(iss);

    ASSERT_TRUE(result.ok());



    auto configs = result.value();

    ASSERT_EQ(configs.size(), 1u);



    const VehiclePropConfig& config = configs.begin()->second.config;

    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);

    ASSERT_EQ(config.areaConfigs.size(), 2u);



    const VehicleAreaConfig& areaConfig1 = config.areaConfigs[0];

    ASSERT_EQ(areaConfig1.access, VehiclePropertyAccess::WRITE);

    ASSERT_EQ(areaConfig1.areaId, 0);



    const VehicleAreaConfig& areaConfig2 = config.areaConfigs[1];

    ASSERT_EQ(areaConfig2.access, VehiclePropertyAccess::READ);

    ASSERT_EQ(areaConfig2.areaId, 1);

}



}  // namespace vehicle

}  // namespace automotive

}  // namespace hardware

automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp

+65 −10
Original line number Diff line number Diff line
@@ -135,6 +135,7 @@ class VtsHalAutomotiveVehicleTargetTest : public testing::TestWithParam<ServiceD 
            const VhalClientResult<std::unique_ptr<IHalPropValue>>& result, int32_t value);



  public:

    void verifyAccessMode(int actualAccess, int expectedAccess);

    void verifyProperty(VehicleProperty propId, VehiclePropertyAccess access,

                        VehiclePropertyChangeMode changeMode, VehiclePropertyGroup group,

                        VehicleArea area, VehiclePropertyType propertyType);
@@ -322,8 +323,13 @@ TEST_P(VtsHalAutomotiveVehicleTargetTest, setProp) { 
        const IHalPropConfig& cfg = *cfgPtr;

        int32_t propId = cfg.getPropId();

        // test on boolean and writable property

        if (cfg.getAccess() == toInt(VehiclePropertyAccess::READ_WRITE) &&

            isBooleanGlobalProp(propId) && !hvacProps.count(propId)) {

        bool isReadWrite = (cfg.getAccess() == toInt(VehiclePropertyAccess::READ_WRITE));

        if (cfg.getAreaConfigSize() != 0 &&

            cfg.getAreaConfigs()[0]->getAccess() != toInt(VehiclePropertyAccess::NONE)) {

            isReadWrite = (cfg.getAreaConfigs()[0]->getAccess() ==

                           toInt(VehiclePropertyAccess::READ_WRITE));

        }

        if (isReadWrite && isBooleanGlobalProp(propId) && !hvacProps.count(propId)) {

            auto propToGet = mVhalClient->createHalPropValue(propId);

            auto getValueResult = mVhalClient->getValueSync(*propToGet);
@@ -580,6 +586,53 @@ TEST_P(VtsHalAutomotiveVehicleTargetTest, testGetValuesTimestampAIDL) { 
    }

}



// Test that access mode is populated in exclusively one of the VehiclePropConfig or the

// VehicleAreaConfigs. Either VehiclePropConfig.access must be populated, or all the

// VehicleAreaConfig.access fields should be populated.

TEST_P(VtsHalAutomotiveVehicleTargetTest, testAccessModeExclusivityAIDL) {

    if (!mVhalClient->isAidlVhal()) {

        GTEST_SKIP() << "Skip checking access mode for HIDL because the access mode field is only "

                        "present for AIDL";

    }



    auto result = mVhalClient->getAllPropConfigs();

    ASSERT_TRUE(result.ok());

    for (const auto& cfgPtr : result.value()) {

        const IHalPropConfig& cfg = *cfgPtr;



        bool propAccessIsSet = (cfg.getAccess() != toInt(VehiclePropertyAccess::NONE));

        bool unsetAreaAccessExists = false;

        bool setAreaAccessExists = false;



        for (const auto& areaConfig : cfg.getAreaConfigs()) {

            if (areaConfig->getAccess() == toInt(VehiclePropertyAccess::NONE)) {

                unsetAreaAccessExists = true;

            } else {

                setAreaAccessExists = true;

            }

        }



        ASSERT_FALSE(propAccessIsSet && setAreaAccessExists) << StringPrintf(

                "Both prop and area config access is set for propertyId %d", cfg.getPropId());

        ASSERT_FALSE(!propAccessIsSet && !setAreaAccessExists) << StringPrintf(

                "Neither prop and area config access is set for propertyId %d", cfg.getPropId());

        ASSERT_FALSE(unsetAreaAccessExists && setAreaAccessExists) << StringPrintf(

                "Area access is only set in some configs for propertyId %d", cfg.getPropId());

    }

}



void VtsHalAutomotiveVehicleTargetTest::verifyAccessMode(int actualAccess, int expectedAccess) {

    if (expectedAccess == toInt(VehiclePropertyAccess::READ_WRITE)) {

        ASSERT_TRUE(actualAccess == expectedAccess ||

                    actualAccess == toInt(VehiclePropertyAccess::READ))

                << StringPrintf("Expect to get VehiclePropertyAccess: %i or %i, got %i",

                                expectedAccess, toInt(VehiclePropertyAccess::READ), actualAccess);

        return;

    }

    ASSERT_EQ(actualAccess, expectedAccess) << StringPrintf(

            "Expect to get VehiclePropertyAccess: %i, got %i", expectedAccess, actualAccess);

}



// Helper function to compare actual vs expected property config

void VtsHalAutomotiveVehicleTargetTest::verifyProperty(VehicleProperty propId,

                                                       VehiclePropertyAccess access,
@@ -622,7 +675,6 @@ void VtsHalAutomotiveVehicleTargetTest::verifyProperty(VehicleProperty propId, 


    const auto& config = result.value().at(0);

    int actualPropId = config->getPropId();

    int actualAccess = config->getAccess();

    int actualChangeMode = config->getChangeMode();

    int actualGroup = actualPropId & toInt(VehiclePropertyGroup::MASK);

    int actualArea = actualPropId & toInt(VehicleArea::MASK);
@@ -631,14 +683,17 @@ void VtsHalAutomotiveVehicleTargetTest::verifyProperty(VehicleProperty propId, 
    ASSERT_EQ(actualPropId, expectedPropId)

            << StringPrintf("Expect to get property ID: %i, got %i", expectedPropId, actualPropId);



    if (expectedAccess == toInt(VehiclePropertyAccess::READ_WRITE)) {

        ASSERT_TRUE(actualAccess == expectedAccess ||

                    actualAccess == toInt(VehiclePropertyAccess::READ))

                << StringPrintf("Expect to get VehiclePropertyAccess: %i or %i, got %i",

                                expectedAccess, toInt(VehiclePropertyAccess::READ), actualAccess);

    int globalAccess = config->getAccess();

    if (config->getAreaConfigSize() == 0) {

        verifyAccessMode(globalAccess, expectedAccess);

    } else {

        ASSERT_EQ(actualAccess, expectedAccess) << StringPrintf(

                "Expect to get VehiclePropertyAccess: %i, got %i", expectedAccess, actualAccess);

        for (const auto& areaConfig : config->getAreaConfigs()) {

            int areaConfigAccess = areaConfig->getAccess();

            int actualAccess = (areaConfigAccess != toInt(VehiclePropertyAccess::NONE))

                                       ? areaConfigAccess

                                       : globalAccess;

            verifyAccessMode(actualAccess, expectedAccess);

        }

    }



    ASSERT_EQ(actualChangeMode, expectedChangeMode)