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: Kevin Rocard <krocard@google.com>
diff --git a/audio/2.0/default/ParametersUtil.cpp b/audio/2.0/default/ParametersUtil.cpp
index 0511557..5cc60db 100644
--- a/audio/2.0/default/ParametersUtil.cpp
+++ b/audio/2.0/default/ParametersUtil.cpp
@@ -22,11 +22,31 @@
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 @@
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 @@
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,23 +78,20 @@
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);
- break;
- }
- result[i].key = halKey.string();
- result[i].value = halValue.string();
+ 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;
}
- if (result.size() != 0) {
- retval = Result::OK;
- }
+ result[i].key = halKey.string();
+ result[i].value = halValue.string();
}
cb(retval, result);
}
diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
index e1c9549..b8e3454 100644
--- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
+++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
@@ -878,8 +878,8 @@
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,
- vector<Result> expectedResults) {
+static void checkGetNoParameter(IStream* stream, hidl_vec<hidl_string> keys,
+ vector<Result> expectedResults) {
hidl_vec<ParameterValue> parameters;
Result res;
ASSERT_OK(stream->getParameters(keys, returnIn(res, parameters)));
@@ -892,16 +892,15 @@
/* 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};