Skip to content
This repository was archived by the owner on Aug 5, 2022. It is now read-only.

Criterion rework #96

Merged
dawagner merged 14 commits into from
Jun 15, 2015
Merged

Criterion rework #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
02d0486
libcriterion: Rework Inclusive Criterion numerical value definition
clero Apr 15, 2015
23f2fdf
python bindings: Add criterion state modification sample
clero Jun 4, 2015
81fb7d3
libcriterion: Rework Value handling
clero Apr 17, 2015
ba301cf
libcriterion: Remove getNumericalValue specialization for inclusive c…
clero Apr 22, 2015
54fd115
libcriterion: Rework state handling
clero Apr 21, 2015
e35e432
libcriterion: Enhance default value handling
clero Apr 23, 2015
021a731
libcriterion: Remove litteral/numerical value handling
clero May 28, 2015
2d88fd1
libcriterion: Add missing documentation
clero May 18, 2015
335f30a
libcriterion: unify API name
clero May 28, 2015
7914ddc
libcriterion: remove isInclusive API
clero May 29, 2015
d11e7ba
libcriterion: Remove getFormattedState from public API
clero Jun 1, 2015
2ac73a4
libcriterion: Finish the shift to the new naming convention
clero Jun 1, 2015
60ec0ec
libcriterion: Rename CriterionInterface Criterion
clero Jun 1, 2015
5947963
test-platform: Rework criterion handling
clero Jun 1, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 62 additions & 45 deletions bindings/c/ParameterFramework.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include <ParameterMgrPlatformConnector.h>

#include <iostream>
#include <limits>
#include <string>
#include <map>

Expand All @@ -41,12 +40,12 @@
#include <cstdlib>

using std::string;
using core::criterion::CriterionInterface;
using core::criterion::Criterion;

/** Rename long pfw types to short ones in pfw namespace. */
namespace pfw
{
typedef std::map<string, CriterionInterface *> Criteria;
typedef std::map<string, Criterion*> Criteria;
typedef CParameterMgrPlatformConnector Pfw;
}

Expand Down Expand Up @@ -187,30 +186,19 @@ bool PfwHandler::createCriteria(const PfwCriterion criteriaArray[], size_t crite
"\" already exist");
}

// Create criterion
CriterionInterface *newCriterion = (criterion.inclusive ?
pfw->createInclusiveCriterion(criterion.name) :
pfw->createExclusiveCriterion(criterion.name));
assert(newCriterion != NULL);
// Add criterion values
// Create criterion values
core::criterion::Values values;
for (size_t valueIndex = 0; criterion.values[valueIndex] != NULL; ++valueIndex) {
int value;
if (criterion.inclusive) {
// Check that (int)1 << valueIndex would not overflow (UB)
if(std::numeric_limits<int>::max() >> valueIndex == 0) {
return status.failure("Too many values for criterion " +
string(criterion.name));
}
value = 1 << valueIndex;
} else {
value = valueIndex;
}
const char * valueName = criterion.values[valueIndex];
string error;
if(not newCriterion->addValuePair(value, valueName, error)) {
return status.failure("Could not add value " + string(valueName) +
" to criterion " + criterion.name + ": " + error);
}
values.emplace_back(criterion.values[valueIndex]);
}
// Create criterion
string error;
Criterion *newCriterion = (criterion.inclusive ?
pfw->createInclusiveCriterion(criterion.name, values, error) :
pfw->createExclusiveCriterion(criterion.name, values, error));
if (newCriterion == nullptr) {
return status.failure("Could not create criterion '" + string(criterion.name) +
"': " + error);
}
// Add new criterion to criteria list
criteria[criterion.name] = newCriterion;
Expand Down Expand Up @@ -256,13 +244,13 @@ const char *pfwGetLastError(const PfwHandler *handle)
return handle == NULL ? NULL : handle->lastStatus.msg().c_str();
}

static CriterionInterface *getCriterion(const pfw::Criteria &criteria, const string &name)
static Criterion *getCriterion(const pfw::Criteria &criteria, const string &name)
{
pfw::Criteria::const_iterator it = criteria.find(name);
return it == criteria.end() ? NULL : it->second;
}

bool pfwSetCriterion(PfwHandler *handle, const char name[], int value)
bool pfwSetCriterion(PfwHandler *handle, const char name[], const char **values)
{
if (handle == NULL) { return Status::failure(); }
Status &status = handle->lastStatus;
Expand All @@ -274,35 +262,64 @@ bool pfwSetCriterion(PfwHandler *handle, const char name[], int value)
return status.failure("Can not set criterion \"" + string(name) +
"\" as the parameter framework is not started.");
}
CriterionInterface *criterion = getCriterion(handle->criteria, name);
Criterion *criterion = getCriterion(handle->criteria, name);
if (criterion == NULL) {
return status.failure("Can not set criterion " + string(name) + " as does not exist");
}
criterion->setCriterionState(value);
return status.success();
core::criterion::State state{};
for (size_t valueIndex = 0; values[valueIndex] != NULL; ++valueIndex) {
state.emplace(values[valueIndex]);
}
return status.forward(criterion->setState(state, status.msg()));
}
bool pfwGetCriterion(const PfwHandler *handle, const char name[], int *value)
const char **pfwGetCriterion(const PfwHandler *handle, const char name[])
{
if (handle == NULL) { return Status::failure(); }
if (handle == NULL) { return NULL; }
Status &status = handle->lastStatus;
if (name == NULL) {
return status.failure("char *name of the criterion is NULL, "
"while getting a criterion.");
status.failure("char *name of the criterion is NULL, while getting a criterion.");
return NULL;
}
if (handle->pfw == NULL) {
return status.failure("Can not get criterion \"" + string(name) +
"\" as the parameter framework is not started.");
}
if (value == NULL) {
return status.failure("Can not get criterion \"" + string(name) +
"\" as the out value is NULL.");
status.failure("Can not get criterion \"" + string(name) +
"\" as the parameter framework is not started.");
return NULL;
}
CriterionInterface *criterion = getCriterion(handle->criteria, name);
Criterion *criterion = getCriterion(handle->criteria, name);
if (criterion == NULL) {
return status.failure("Can not get criterion " + string(name) + " as it does not exist");
status.failure("Can not get criterion " + string(name) + " as it does not exist");
return NULL;
}
*value = criterion->getCriterionState();
return status.success();
core::criterion::State state = criterion->getState();

// Allocating one slot for each criterion value plus one for the NULL end delimiter
const char **values = static_cast<const char**>(malloc((state.size() + 1) * sizeof(char*)));
if (values == nullptr) {
status.failure("Can not get criterion " + string(name) + " as there is no memory left");
return NULL;
}

size_t i = 0;
for (auto &critValue : state) {
values[i] = strdup(critValue.c_str());
if (values[i] == nullptr) {
status.failure("Can not get criterion " + string(name) +
" as there is no memory left (errno: " + strerror(errno) + ")");
return NULL;
}
++i;
}
values[i] = NULL;
return values;
}

void pfwCriterionValueFree(const char **value)
{
const char **valueSav = value;
while(*value != NULL) {
std::free(*const_cast<char**>(value++));
}
std::free(valueSav);
}

bool pfwApplyConfigurations(const PfwHandler *handle)
Expand Down
27 changes: 19 additions & 8 deletions bindings/c/ParameterFramework.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,25 +156,36 @@ const char *pfwGetLastError(const PfwHandler *handle) NONNULL;
/** Set a criterion value given its name and value.
* @param handle[in] @see PfwHandler
* @param name[in] The name of the criterion that need to be changed.
* @param value If the criterion is exclusive, the index of the new value.
* If the criterion is inclusive, a bit field where each bit
* correspond to the value index.
* @param value If the criterion is exclusive, an array containing the new value.
* If the criterion is inclusive, an array containing all new values.
* For an inclusive criterion defined as such: { "Red", "Green", "Blue", NULL }
* to set the value Green and Blue, value has to be 1<<1 | 1<<2 = 0b110 = 6.
* to set the value Green and Blue, value has to be ["Green", "Blue", NULL].
* For an exclusive criterion defined as such: { "Car", "Boat", "Plane", NULL }
* to set the value Plane, value has to be 2.
* to set the value Plane, value has to be ["Plane", NULL].
*
* Criterion change do not have impact on the parameters value
* (no configuration applied) until the changes are committed using pfwApplyConfigurations.
*
* @return true on success and false on failure.
*/
bool pfwSetCriterion(PfwHandler *handle, const char name[], int value) NONNULL USERESULT;
bool pfwSetCriterion(PfwHandler *handle, const char name[], const char **values) NONNULL USERESULT;
/** Get a criterion value given its name.
* Same usage as pfwSetCriterion except that value is an out param.
* @param handle[in] @see PfwHandler
* @param name[in] The name of the criterion that need to be changed.
* @return an NULL terminated array of char* which store criterion values if success
* NULL otherwise
* Get criterion will return the last value setted with pfwSetCriterion independantly of pfwCommitCritenio.
* The callee MUST free the returned value array using pfwCriterionValueFree after use.
*/
const char **pfwGetCriterion(const PfwHandler *handle, const char name[]) NONNULL USERESULT;

/** Frees the memory space of criterion value array,
* which must have been returned by a previous call to pfwGetCriterion.
*
* @param value[in] pointer to the memory to free.
* @see man 3 free for usage.
*/
bool pfwGetCriterion(const PfwHandler *handle, const char name[], int *value) NONNULL USERESULT;
void pfwCriterionValueFree(const char **value);

/** Commit criteria change and change parameters according to the configurations.
* Criterion do not have impact on the parameters value when changed,
Expand Down
80 changes: 36 additions & 44 deletions bindings/c/Test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,6 @@ TEST_CASE_METHOD(Test, "Parameter-framework c api use") {
};
REQUIRE_FAILURE(pfwStart(pfw, config, duplicatedCriteria, 2, &logger));
}
WHEN("The pfw is started with duplicated criterion value state") {
const char * values[] = {"a", "a", NULL};
const PfwCriterion duplicatedCriteria[] = {{"name", true, values}};

WHEN("Using test logger") {
REQUIRE_FAILURE(pfwStart(pfw, config, duplicatedCriteria, 1, &logger));
}
WHEN("Using default logger") {
// Test coverage of default logger warning
REQUIRE_FAILURE(pfwStart(pfw, config, duplicatedCriteria, 1, NULL));
}
}
WHEN("The pfw is started with NULL name criterion") {
const PfwCriterion duplicatedCriteria[] = {{NULL, true, letterList}};
REQUIRE_FAILURE(pfwStart(pfw, config, duplicatedCriteria, 1, &logger));
Expand All @@ -204,8 +192,8 @@ TEST_CASE_METHOD(Test, "Parameter-framework c api use") {
}
GIVEN("A criteria with lots of values")
{
// Build a criterion with as many value as there is bits in int.
std::vector<char> names(sizeof(int) * CHAR_BIT + 1, 'a');
// Build a criterion with a lot of value
std::vector<char> names(300, 'a');
Copy link
Contributor

@krocard krocard May 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 300 ? I think this test section is meaningless now (was testing the criterion value number limitation). Delete it ?

Copy link
Contributor Author

@clero clero May 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it shows all is working but yes we can remove it

names.back() = '\0';
std::vector<const char *> values(names.size());
for(size_t i = 0; i < values.size(); ++i) {
Expand Down Expand Up @@ -240,11 +228,7 @@ TEST_CASE_METHOD(Test, "Parameter-framework c api use") {
*/
const PfwCriterion duplicatedCriteria[] = {{"name", true, &values[0]}};

WHEN("The pfw is started with a too long criterion state list") {
REQUIRE_FAILURE(pfwStart(pfw, config, duplicatedCriteria, 1, &logger));
}
WHEN("The pfw is started with max length criterion state list") {
values[values.size() - 2] = NULL; // Hide last value
WHEN("The pfw is started with a long criterion state list") {
REQUIRE_SUCCESS(pfwStart(pfw, config, duplicatedCriteria, 1, &logger));
}
}
Expand All @@ -267,11 +251,11 @@ TEST_CASE_METHOD(Test, "Parameter-framework c api use") {
}

WHEN("Get criterion of a stopped pfw") {
int value;
REQUIRE_FAILURE(pfwGetCriterion(pfw, criteria[0].name, &value));
REQUIRE_FAILURE(pfwGetCriterion(pfw, criteria[0].name) != NULL);
}
WHEN("Set criterion of a stopped pfw") {
REQUIRE_FAILURE(pfwSetCriterion(pfw, criteria[0].name, 1));
std::vector<const char*> value{"1", NULL};
REQUIRE_FAILURE(pfwSetCriterion(pfw, criteria[0].name, value.data()));
}
WHEN("Commit criteria of a stopped pfw") {
REQUIRE_FAILURE(pfwApplyConfigurations(pfw));
Expand All @@ -284,55 +268,63 @@ TEST_CASE_METHOD(Test, "Parameter-framework c api use") {
WHEN("The pfw is started correctly")
{
REQUIRE_SUCCESS(pfwStart(pfw, config, criteria, criterionNb, &logger));
int value;
int value = 0;

WHEN("Get criterion without an handle") {
REQUIRE(not pfwGetCriterion(NULL, criteria[0].name, &value));
REQUIRE(pfwGetCriterion(NULL, criteria[0].name) == NULL);
}
WHEN("Get criterion without a name") {
REQUIRE_FAILURE(pfwGetCriterion(pfw, NULL, &value));
}
WHEN("Get criterion without an output value") {
REQUIRE_FAILURE(pfwGetCriterion(pfw, criteria[0].name, NULL));
REQUIRE_FAILURE(pfwGetCriterion(pfw, NULL) != NULL);
}
WHEN("Get not existing criterion") {
REQUIRE_FAILURE(pfwGetCriterion(pfw, "Do not exist", &value));
REQUIRE_FAILURE(pfwGetCriterion(pfw, "Do not exist") != NULL);
}
THEN("All criterion should value 0") {
THEN("All criterion should have their default value") {
for(size_t i = 0; i < criterionNb; ++i) {
const char *criterionName = criteria[i].name;
CAPTURE(criterionName);
REQUIRE_SUCCESS(pfwGetCriterion(pfw, criterionName, &value));
REQUIRE(value == 0);
const char **criterionGetValue = pfwGetCriterion(pfw, criterionName);
REQUIRE_SUCCESS(criterionGetValue != NULL);
REQUIRE(criterionGetValue != NULL);
if (criteria[i].inclusive) {
REQUIRE(criterionGetValue[0] == NULL);
} else {
REQUIRE(std::string(criterionGetValue[0]) == "1");
}
pfwCriterionValueFree(criterionGetValue);
}
}

WHEN("Set criterion without an handle") {
REQUIRE(not pfwSetCriterion(NULL, criteria[0].name, 1));
std::vector<const char*> setValue{"1", NULL};
REQUIRE(not pfwSetCriterion(NULL, criteria[0].name, setValue.data()));
}
WHEN("Set criterion without a name") {
REQUIRE_FAILURE(pfwSetCriterion(pfw, NULL, 2));
std::vector<const char*> setValue{"2", NULL};
REQUIRE_FAILURE(pfwSetCriterion(pfw, NULL, setValue.data()));
}
WHEN("Set not existing criterion") {
REQUIRE_FAILURE(pfwSetCriterion(pfw, "Do not exist", 3));
std::vector<const char*> setValue{"3", NULL};
REQUIRE_FAILURE(pfwSetCriterion(pfw, "Do not exist", setValue.data()));
}
WHEN("Set criterion value") {
for(size_t i = 0; i < criterionNb; ++i) {
std::vector<const char*> setValue{(criteria[i].inclusive ? "b" : "2"), NULL};
const char *criterionName = criteria[i].name;
CAPTURE(criterionName);
REQUIRE_SUCCESS(pfwSetCriterion(pfw, criterionName, 3));
}
THEN("Get criterion value should return what was set") {
for(size_t i = 0; i < criterionNb; ++i) {
const char *criterionName = criteria[i].name;
CAPTURE(criterionName);
REQUIRE_SUCCESS(pfwGetCriterion(pfw, criterionName, &value));
REQUIRE(value == 3);
REQUIRE_SUCCESS(pfwSetCriterion(pfw, criterionName, setValue.data()));
THEN("Get criterion value should return what was set") {
const char**criterionGetValue = pfwGetCriterion(pfw, criterionName);
REQUIRE_SUCCESS(criterionGetValue != NULL);
REQUIRE(std::string(criterionGetValue[0]) ==
(criteria[i].inclusive ? "b" : "2"));
pfwCriterionValueFree(criterionGetValue);
}
}
WHEN("Set a new value to a criterion without committing first") {
std::vector<const char*> setValue{"a", NULL};
const char *criterionName = criteria[0].name;
REQUIRE_SUCCESS(pfwSetCriterion(pfw, criterionName, 0));
REQUIRE_SUCCESS(pfwSetCriterion(pfw, criterionName, setValue.data()));
THEN("A warning message should have been displayed") {
INFO("Previous pfw log: \n" + logLines);
size_t logPos = logLines.find("Warning: Selection criterion "
Expand Down
Loading