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

Commit fa3b4a93 authored by Kevin Rocard's avatar Kevin Rocard
Browse files

Audio HAL VTS: differentiate getParam success/failure/not_implemented 



When sending parameters to the HAL (and some getters are implemented
with getParameters), the client expect a status consistent
with the other HIDL methods. Ie: not implemented or success and failure.

Unfortunately, the legacy get_parameter interface, which currently most
Audio HIDL implementation are a wrapper around, do not return such error
code.

Get parameters return a list of key values.
 - If a requested key does not return a key value pair, consider it not
   implemented
 - If a requested key returns a key not followed by a correct value,
   consider it a failure
 - otherwise it is a success

Test: vts-tradefed run vts --module VtsHalAudioV2_0Target
Test: call/play music/record/video...
Bug: 36311550
Change-Id: Id6711e9c1974fe5a336b6de83a9b6d14f74437c9
Signed-off-by: default avatarKevin Rocard <krocard@google.com>
parent 72e50e2e

audio/2.0/default/ParametersUtil.cpp

+34 −19
Original line number Diff line number Diff line
@@ -22,11 +22,31 @@ namespace audio { 
namespace V2_0 {

namespace implementation {



// Static method and not private method to avoid leaking status_t dependency

static Result getHalStatusToResult(status_t status) {

    switch (status) {

        case OK:

            return Result::OK;

        case BAD_VALUE:  // Nothing was returned, probably because the HAL does

                         // not handle it

            return Result::NOT_SUPPORTED;

        case INVALID_OPERATION:  // Conversion from string to the requested type

                                 // failed

            return Result::INVALID_ARGUMENTS;

        default:  // Should not happen

            ALOGW("Unexpected status returned by getParam: %u", status);

            return Result::INVALID_ARGUMENTS;

    }

}



Result ParametersUtil::getParam(const char* name, bool* value) {

    String8 halValue;

    Result retval = getParam(name, &halValue);

    *value = false;

    if (retval == Result::OK) {

        if (halValue.empty()) {

            return Result::NOT_SUPPORTED;

        }

        *value = !(halValue == AudioParameter::valueOff);

    }

    return retval;
@@ -37,8 +57,7 @@ Result ParametersUtil::getParam(const char* name, int* value) { 
    AudioParameter keys;

    keys.addKey(halName);

    std::unique_ptr<AudioParameter> params = getParams(keys);

    status_t halStatus = params->getInt(halName, *value);

    return halStatus == OK ? Result::OK : Result::INVALID_ARGUMENTS;

    return getHalStatusToResult(params->getInt(halName, *value));

}



Result ParametersUtil::getParam(const char* name, String8* value) {
@@ -46,8 +65,7 @@ Result ParametersUtil::getParam(const char* name, String8* value) { 
    AudioParameter keys;

    keys.addKey(halName);

    std::unique_ptr<AudioParameter> params = getParams(keys);

    status_t halStatus = params->get(halName, *value);

    return halStatus == OK ? Result::OK : Result::INVALID_ARGUMENTS;

    return getHalStatusToResult(params->get(halName, *value));

}



void ParametersUtil::getParametersImpl(
@@ -60,24 +78,21 @@ void ParametersUtil::getParametersImpl( 
        halKeys.addKey(String8(keys[i].c_str()));

    }

    std::unique_ptr<AudioParameter> halValues = getParams(halKeys);

    Result retval(Result::INVALID_ARGUMENTS);

    Result retval =

        halValues->size() == keys.size() ? Result::OK : Result::NOT_SUPPORTED;

    hidl_vec<ParameterValue> result;

    if (halValues->size() > 0) {

    result.resize(halValues->size());

    String8 halKey, halValue;

    for (size_t i = 0; i < halValues->size(); ++i) {

        status_t status = halValues->getAt(i, halKey, halValue);

        if (status != OK) {

            result.resize(0);

            retval = getHalStatusToResult(status);

            break;

        }

        result[i].key = halKey.string();

        result[i].value = halValue.string();

    }

        if (result.size() != 0) {

            retval = Result::OK;

        }

    }

    cb(retval, result);

}

audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp

+6 −7
Original line number Diff line number Diff line
@@ -878,7 +878,7 @@ TEST_IO_STREAM(SetHwAvSync, "Try to set hardware sync to an invalid value", 
TEST_IO_STREAM(GetHwAvSync, "Get hardware sync can not fail",

               ASSERT_TRUE(device->getHwAvSync().isOk()))



static void checkGetParameter(IStream* stream, hidl_vec<hidl_string> keys,

static void checkGetNoParameter(IStream* stream, hidl_vec<hidl_string> keys,

                                vector<Result> expectedResults) {

    hidl_vec<ParameterValue> parameters;

    Result res;
@@ -892,16 +892,15 @@ static void checkGetParameter(IStream* stream, hidl_vec<hidl_string> keys, 
/* Get/Set parameter is intended to be an opaque channel between vendors app and

 * their HALs.

 * Thus can not be meaningfully tested.

 * TODO: Doc missing. Should asking for an empty set of params raise an error ?

 */

TEST_IO_STREAM(getEmptySetParameter, "Retrieve the values of an empty set",

               checkGetParameter(stream.get(), {} /* keys */,

                                 {Result::OK, Result::INVALID_ARGUMENTS}))

               checkGetNoParameter(stream.get(), {} /* keys */, {Result::OK}))



TEST_IO_STREAM(getNonExistingParameter,

               "Retrieve the values of an non existing parameter",

               checkGetParameter(stream.get(), {"Non existing key"} /* keys */,

                                 {Result::INVALID_ARGUMENTS}))

               checkGetNoParameter(stream.get(),

                                   {"Non existing key"} /* keys */,

                                   {Result::NOT_SUPPORTED}))



static vector<Result> okOrInvalidArguments = {Result::OK,

                                              Result::INVALID_ARGUMENTS};