From 96a1ee3072f481909c2e6c3d243b77bcf70e23aa Mon Sep 17 00:00:00 2001 From: David Wagner Date: Thu, 5 Nov 2015 17:29:25 +0100 Subject: [PATCH 1/3] Mapping: replace an assert with an error message The assert was checking that the mapping key being added to an element's mapping data was not already present. It seemed redundant with the subsequent check that was returning an error if the name of the mapping key being added was already present. Returning an error to the user leads avoid crashing and produces clearer error messages. Signed-off-by: David Wagner --- parameter/MappingContext.cpp | 11 ++--------- parameter/MappingContext.h | 5 ++--- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/parameter/MappingContext.cpp b/parameter/MappingContext.cpp index 0457474c3..04bdc831a 100644 --- a/parameter/MappingContext.cpp +++ b/parameter/MappingContext.cpp @@ -37,12 +37,7 @@ using std::string; // Item access bool CMappingContext::setItem(size_t itemType, const string* pStrKey, const string* pStrItem) { - // Assert that the key hasn't been set before - assert(find_if(begin(mItems), end(mItems), [pStrKey](SItem &item) - { return item.strKey == pStrKey; } - ) == end(mItems)); - - if (mItems[itemType].bSet) { + if (iSet(itemType)) { // Already set! return false; } @@ -53,8 +48,6 @@ bool CMappingContext::setItem(size_t itemType, const string* pStrKey, const stri // Set item value mItems[itemType].strItem = pStrItem; - // Now is set - mItems[itemType].bSet = true; return true; } @@ -83,5 +76,5 @@ const string* CMappingContext::getItem(const string& strKey) const bool CMappingContext::iSet(size_t itemType) const { - return mItems[itemType].bSet; + return mItems[itemType].strItem != nullptr; } diff --git a/parameter/MappingContext.h b/parameter/MappingContext.h index 0269c9515..b24d3303c 100644 --- a/parameter/MappingContext.h +++ b/parameter/MappingContext.h @@ -41,9 +41,8 @@ class PARAMETER_EXPORT CMappingContext private: // Item structure struct SItem { - const std::string* strKey; - const std::string* strItem; - bool bSet; + const std::string* strKey{nullptr}; + const std::string* strItem{nullptr}; }; public: From dfa10b7ff56b81206a3a198cd55233503e4a4ebc Mon Sep 17 00:00:00 2001 From: David Wagner Date: Thu, 19 Nov 2015 15:02:52 +0100 Subject: [PATCH 2/3] InstanceConfigurableElement: delegate getXmlElementName to the type element A instanciated configurable element may or may not need to be exported as XML with a tag name that differs from its kind. Always rely on how the type must be exported for this matter. Signed-off-by: David Wagner --- parameter/Component.h | 7 ------- parameter/ComponentInstance.cpp | 9 ++++++++- parameter/ComponentInstance.h | 1 + parameter/InstanceConfigurableElement.cpp | 6 ++++++ parameter/InstanceConfigurableElement.h | 1 + 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/parameter/Component.h b/parameter/Component.h index 91126db83..4ec1f71ed 100644 --- a/parameter/Component.h +++ b/parameter/Component.h @@ -45,12 +45,5 @@ class CComponent : public CInstanceConfigurableElement { return EComponent; } - - std::string getXmlElementName() const override - { - // Once instantiated components are reflected as parameter blocks - // in XML documents - return "ParameterBlock"; - } }; diff --git a/parameter/ComponentInstance.cpp b/parameter/ComponentInstance.cpp index 491daa46f..97634ff0f 100644 --- a/parameter/ComponentInstance.cpp +++ b/parameter/ComponentInstance.cpp @@ -41,7 +41,14 @@ CComponentInstance::CComponentInstance(const std::string& strName) : base(strNam std::string CComponentInstance::getKind() const { - return "Component"; + return "ComponentInstance"; +} + +std::string CComponentInstance::getXmlElementName() const +{ + // Once instantiated components are reflected as parameter blocks + // in XML documents + return "ParameterBlock"; } bool CComponentInstance::childrenAreDynamic() const diff --git a/parameter/ComponentInstance.h b/parameter/ComponentInstance.h index 5f4c23acd..97d3afc06 100644 --- a/parameter/ComponentInstance.h +++ b/parameter/ComponentInstance.h @@ -55,6 +55,7 @@ class CComponentInstance : public CTypeElement // CElement virtual std::string getKind() const; + std::string getXmlElementName() const override; private: virtual bool childrenAreDynamic() const; virtual CInstanceConfigurableElement* doInstantiate() const; diff --git a/parameter/InstanceConfigurableElement.cpp b/parameter/InstanceConfigurableElement.cpp index 9eb20304e..e0385afe9 100644 --- a/parameter/InstanceConfigurableElement.cpp +++ b/parameter/InstanceConfigurableElement.cpp @@ -47,6 +47,12 @@ std::string CInstanceConfigurableElement::getKind() const return _pTypeElement->getKind(); } +std::string CInstanceConfigurableElement::getXmlElementName() const +{ + // Delegate + return _pTypeElement->getXmlElementName(); +} + // Type element const CTypeElement* CInstanceConfigurableElement::getTypeElement() const { diff --git a/parameter/InstanceConfigurableElement.h b/parameter/InstanceConfigurableElement.h index 9d2271efb..224199ec9 100644 --- a/parameter/InstanceConfigurableElement.h +++ b/parameter/InstanceConfigurableElement.h @@ -70,6 +70,7 @@ class PARAMETER_EXPORT CInstanceConfigurableElement : public CConfigurableElemen // From CElement virtual std::string getKind() const; + std::string getXmlElementName() const override; // Syncer to/from HW void setSyncer(ISyncer* pSyncer); From 416f78424495e97e9cf51b7f42fb16ae67bc5675 Mon Sep 17 00:00:00 2001 From: David Wagner Date: Wed, 4 Nov 2015 17:48:43 +0100 Subject: [PATCH 3/3] Allow ArrayLength attribute in Component tags Numerical parameters as well as ParameterBlocks can have an "ArrayLength" attribute that instantiate as many identical parameters or parameter blocks. This is now also possible for Component instances (not types). When used, a ParameterBlock is created with the name of the arrayed Component and as many Components will be created with their names being "0", "1", "2", ... *** Use with caution! *** You shouldn't define any context or instantiation mapping at or below the level of the "arrayed" component. This also concerns the Component's type (since the Component will inherit its type's mapping). In other words, only use "ArrayLength" on components that do not have any mapping and which content does not have any mapping either. DO: DON'T: Signed-off-by: David Wagner --- parameter/ComponentInstance.cpp | 28 +++++++++++++++++++++++++--- schemas/Parameter.xsd | 1 + test/functional-tests/Handle.cpp | 26 ++++++++++++++++++-------- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/parameter/ComponentInstance.cpp b/parameter/ComponentInstance.cpp index 97634ff0f..995737572 100644 --- a/parameter/ComponentInstance.cpp +++ b/parameter/ComponentInstance.cpp @@ -31,6 +31,7 @@ #include "ComponentLibrary.h" #include "ComponentType.h" #include "Component.h" +#include "ParameterBlock.h" // for "array" instantiation #include "XmlParameterSerializingContext.h" #define base CTypeElement @@ -110,12 +111,33 @@ bool CComponentInstance::fromXml(const CXmlElement& xmlElement, CXmlSerializingC CInstanceConfigurableElement* CComponentInstance::doInstantiate() const { - return new CComponent(getName(), this); + if (isScalar()) { + return new CComponent(getName(), this); + } else { + return new CParameterBlock(getName(), this); + } } void CComponentInstance::populate(CElement* pElement) const { - base::populate(pElement); + size_t arrayLength = getArrayLength(); + + if (arrayLength != 0) { + + // Create child elements + for (size_t child = 0; child < arrayLength; child++) { + + CComponent* pChildComponent = new CComponent(std::to_string(child), this); - _pComponentType->populate(static_cast(pElement)); + pElement->addChild(pChildComponent); + + base::populate(pChildComponent); + + _pComponentType->populate(pChildComponent); + } + } else { + base::populate(pElement); + + _pComponentType->populate(static_cast(pElement)); + } } diff --git a/schemas/Parameter.xsd b/schemas/Parameter.xsd index 23745405b..8afc482a4 100644 --- a/schemas/Parameter.xsd +++ b/schemas/Parameter.xsd @@ -10,6 +10,7 @@ + diff --git a/test/functional-tests/Handle.cpp b/test/functional-tests/Handle.cpp index fef03b71a..d13c0e6d7 100644 --- a/test/functional-tests/Handle.cpp +++ b/test/functional-tests/Handle.cpp @@ -102,7 +102,6 @@ struct AllParamsPF : public ParameterFramework nodeDesc("ParameterBlock", "parameter_block_array", getBasicParams(), "ArrayLength='2'") + nodeDesc("Component", "component_scalar", "", "Type='component_type'") + - // Test that ArrayLength have no effect on components nodeDesc("Component", "component_array", "", "Type='component_type' ArrayLength='2'"); return config; @@ -220,10 +219,12 @@ SCENARIO_METHOD(AllParamsPF, "Export component", "[handler][structure][xml]") SCENARIO_METHOD(AllParamsPF, "Export component array", "[handler][structure][xml]") { - string expected = rootNode("ParameterBlock", "Name='component_array' " - "Description='description_component_array'", - // component array are the same as non array for now - getBasicParams()); + string expected = rootNode("ParameterBlock", + "Name='component_array' Description='description_component_array'", + nodeDesc("ParameterBlock", "0", getBasicParams(), "", + "description_component_array") + + nodeDesc("ParameterBlock", "1", getBasicParams(), "", + "description_component_array")); checkStructure("/test/test/component_array", expected); } @@ -239,7 +240,12 @@ SCENARIO_METHOD(AllParamsPF, "Export all parameters", "[handler][structure][xml] "description_parameter_block_array")) + // Components should be exported as parameterBlock nodeDesc("ParameterBlock", "component_scalar", getBasicParams()) + - nodeDesc("ParameterBlock", "component_array", getBasicParams()); + nodeDesc("ParameterBlock", "component_array", + nodeDesc("ParameterBlock", "0", getBasicParams(), "", + // description is inherited from array + "description_component_array") + + nodeDesc("ParameterBlock", "1", getBasicParams(), "", + "description_component_array")); WHEN("Exporting subsystem") { string expected = rootNode("Subsystem", "Name='test'", paramExpected); @@ -274,7 +280,9 @@ struct SettingsTestPF : public AllParamsPF parameterBlockNode("0", settings) + parameterBlockNode("1", settings)) + parameterBlockNode("component_scalar", settings) + - parameterBlockNode("component_array", settings); + parameterBlockNode("component_array", + parameterBlockNode("0", settings) + + parameterBlockNode("1", settings)); return rootNode("SystemClass", "Name='test'" , node("Subsystem", "test", settings, "")); @@ -283,7 +291,9 @@ struct SettingsTestPF : public AllParamsPF static string fullBytesSettings(const string &basicSettings) { string fullSettings; - for (size_t i = 0; i < 6; ++i) { + // We have the "basic params" repeated 7 times across the test + // structure + for (size_t i = 0; i < 7; ++i) { fullSettings += basicSettings; } return fullSettings;