diff --git a/src/controller/controller_pythonembedded.cpp b/src/controller/controller_pythonembedded.cpp index 5cb54fb9..7a26597f 100644 --- a/src/controller/controller_pythonembedded.cpp +++ b/src/controller/controller_pythonembedded.cpp @@ -856,7 +856,7 @@ PyObject* PythonEmbedded::pythonAddCustomAttribute(PyObject* /*self*/, PyObject* auto previousValue = that->getObjectAttribute(that->getName(), attributeName); // Add the attribute to the Python interpreter object - // This will replace any previous (or default) attribute (setter and getter included) + // This will replace any previous attribute (setter and getter included) that->addAttribute( attributeName, [=](const Values& args) { diff --git a/src/core/attribute.cpp b/src/core/attribute.cpp index bc59e78b..42dc604a 100644 --- a/src/core/attribute.cpp +++ b/src/core/attribute.cpp @@ -19,20 +19,18 @@ CallbackHandle::~CallbackHandle() /*************/ Attribute::Attribute(const std::string& name, const std::function& setFunc, const std::function& getFunc, const std::vector& types) : _name(name) + , _valuesTypes(types) , _setFunc(setFunc) , _getFunc(getFunc) - , _defaultSetAndGet(false) - , _valuesTypes(types) { } /*************/ Attribute::Attribute(const std::string& name, const std::function& setFunc, const std::vector& types) : _name(name) + , _valuesTypes(types) , _setFunc(setFunc) , _getFunc(nullptr) - , _defaultSetAndGet(false) - , _valuesTypes(types) { } @@ -41,7 +39,6 @@ Attribute::Attribute(const std::string& name, const std::function& get : _name(name) , _setFunc(nullptr) , _getFunc(getFunc) - , _defaultSetAndGet(false) { } @@ -50,17 +47,17 @@ Attribute& Attribute::operator=(Attribute&& a) noexcept { if (this != &a) { - _name = move(a._name); - _objectName = move(a._objectName); - _setFunc = move(a._setFunc); - _getFunc = move(a._getFunc); - _defaultSetAndGet = a._defaultSetAndGet; - _objectName = move(a._objectName); - _description = move(a._description); - _values = std::move(a._values); - _valuesTypes = move(a._valuesTypes); + _name = std::move(a._name); + _objectName = std::move(a._objectName); + _description = std::move(a._description); + _valuesTypes = std::move(a._valuesTypes); + + _setFunc = std::move(a._setFunc); + _getFunc = std::move(a._getFunc); + _syncMethod = a._syncMethod; - _callbacks = move(a._callbacks); + _callbacks = std::move(a._callbacks); + _isLocked = a._isLocked; } return *this; @@ -75,25 +72,12 @@ bool Attribute::operator()(const Values& args) // Run all set callbacks { std::lock_guard lockCb(_callbackMutex); - for (auto& cb : _callbacks) + for (const auto& cb : _callbacks) cb.second(_objectName, _name); } - if (!_setFunc && _defaultSetAndGet) - { - std::lock_guard lock(_defaultFuncMutex); - _values = args; - - _valuesTypes.clear(); - for (const auto& a : args) - _valuesTypes.push_back(a.getTypeAsChar()); - - return true; - } - else if (!_setFunc) - { + if (!_setFunc) return false; - } // Check for arguments correctness. // Some attributes may have an unlimited number of arguments, so we do not test for equality. @@ -108,8 +92,8 @@ bool Attribute::operator()(const Values& args) if (args[i].isConvertibleToType(Value::getTypeFromChar(_valuesTypes[i]))) continue; - auto type = args[i].getTypeAsChar(); - auto expected = _valuesTypes[i]; + const auto type = args[i].getTypeAsChar(); + const auto expected = _valuesTypes[i]; Log::get() << Log::WARNING << _objectName << "~~" << _name << " - Argument " << i << " is of wrong type " << std::string(&type, &type + 1) << ", expected " << std::string(&expected, &expected + 1) << Log::endl; @@ -122,15 +106,8 @@ bool Attribute::operator()(const Values& args) /*************/ Values Attribute::operator()() const { - if (!_getFunc && _defaultSetAndGet) - { - std::lock_guard lock(_defaultFuncMutex); - return _values; - } - else if (!_getFunc) - { + if (!_getFunc) return Values(); - } return _getFunc(); } @@ -168,7 +145,7 @@ CallbackHandle Attribute::registerCallback(std::weak_ptr caller, Cal bool Attribute::unregisterCallback(const CallbackHandle& handle) { std::lock_guard lockCb(_callbackMutex); - auto callback = _callbacks.find(handle.getId()); + const auto callback = _callbacks.find(handle.getId()); if (callback == _callbacks.end()) return false; diff --git a/src/core/attribute.h b/src/core/attribute.h index 26b97acb..f2ff4efe 100644 --- a/src/core/attribute.h +++ b/src/core/attribute.h @@ -132,12 +132,6 @@ class Attribute */ Values operator()() const; - /** - * Tells whether the setter and getters are the default ones or not. - * \return Returns true if the setter and getter are the default ones. - */ - bool isDefault() const { return _defaultSetAndGet; } - /** * Get the types of the wanted arguments. * \return Returns the expected types in a Values. @@ -222,24 +216,19 @@ class Attribute void setSyncMethod(const Sync& method) { _syncMethod = method; } private: - mutable std::mutex _defaultFuncMutex{}; - std::string _name{}; // Name of the attribute - - std::function _setFunc{}; - std::function _getFunc{}; - - bool _defaultSetAndGet{true}; - - std::string _objectName{}; // Name of the object holding this attribute - std::string _description{}; // Attribute description - Values _values{}; // Holds the values for the default set and get functions - std::vector _valuesTypes{}; // List of the types held in _values - Sync _syncMethod{Sync::auto_sync}; //!< Synchronization to consider while setting this attribute - std::mutex _callbackMutex{}; - std::map _callbacks{}; - bool _isLocked{false}; + std::string _name{"noname"}; // Name of the attribute + std::string _objectName{"unknown"}; // Name of the object holding this attribute + std::string _description{}; // Attribute description + std::vector _valuesTypes{}; // List of the types held in _values + + std::function _setFunc{}; // Setter function + std::function _getFunc{}; // Getter function + + Sync _syncMethod{Sync::auto_sync}; // Synchronization to consider while setting this attribute + std::map _callbacks{}; // Callbacks invoked when attribute is modified + bool _isLocked{false}; // If true, the setter can not be invoked }; } // namespace Splash diff --git a/src/core/base_object.cpp b/src/core/base_object.cpp index ffd49432..e6328e2d 100644 --- a/src/core/base_object.cpp +++ b/src/core/base_object.cpp @@ -42,23 +42,11 @@ BaseObject::SetAttrStatus BaseObject::setAttribute(const std::string& attrib, co { std::unique_lock lock(_attribMutex); auto attribFunction = _attribFunctions.find(attrib); - const bool attribNotPresent = (attribFunction == _attribFunctions.end()); + _updatedParams = true; - if (attribNotPresent) - { - const auto result = _attribFunctions.emplace(attrib, Attribute(attrib)); - assert(result.second); - attribFunction = result.first; - } - - // If the attribute is not a default one, signify that the parameters - // have been updated - if (!attribFunction->second.isDefault()) - _updatedParams = true; - - // If the attribute is not a default one, but does not have a setter set, + // If no setter function has been set, // there is no need to try to set a new value - if (!attribFunction->second.isDefault() && !attribFunction->second.hasSetter()) + if (!attribFunction->second.hasSetter()) return SetAttrStatus::no_setter; // Otherwise try setting the value. If this fails, something is wrong diff --git a/tests/data/splashrc.json b/tests/data/splashrc.json index c35ddb14..b8cc776b 100644 --- a/tests/data/splashrc.json +++ b/tests/data/splashrc.json @@ -1,5 +1,5 @@ { "image" : { - "flip" : 1 + "flip" : true } } diff --git a/tests/unit_tests/core/attribute.cpp b/tests/unit_tests/core/attribute.cpp index 166de7b0..2ac0538c 100644 --- a/tests/unit_tests/core/attribute.cpp +++ b/tests/unit_tests/core/attribute.cpp @@ -23,7 +23,6 @@ TEST_CASE("Testing Attribute usage") CHECK_EQ(attr.getSyncMethod(), Attribute::Sync::force_sync); CHECK(attr.hasGetter()); - CHECK_FALSE(attr.isDefault()); CHECK(attr({42})); CHECK_EQ(attr()[0].as(), 42); @@ -59,19 +58,3 @@ TEST_CASE("Testing Attribute usage") CHECK(attr({"A girl has no name"})); CHECK(attr().empty()); } - -/*************/ -TEST_CASE("Testing Attribute constructors") -{ - auto attr = Attribute(); - CHECK_FALSE(attr.hasGetter()); - CHECK(attr.isDefault()); - attr({38}); - CHECK_EQ(attr()[0].as(), 38); - - attr = Attribute("attribute"); - CHECK_FALSE(attr.hasGetter()); - CHECK(attr.isDefault()); - attr({"Flying machine"}); - CHECK_EQ(attr()[0].as(), "Flying machine"); -} diff --git a/tests/unit_tests/core/base_object.cpp b/tests/unit_tests/core/base_object.cpp index 6ffbf7be..f59cd90c 100644 --- a/tests/unit_tests/core/base_object.cpp +++ b/tests/unit_tests/core/base_object.cpp @@ -114,7 +114,6 @@ TEST_CASE("Testing BaseObject class") object->setAttribute("integer", {integer_value}); object->setAttribute("float", {float_value}); object->setAttribute("string", {string_value}); - CHECK(object->setAttribute("newAttribute", array_value) != BaseObject::SetAttrStatus::failure); Values value; CHECK(object->getAttribute("integer", value)); @@ -132,11 +131,6 @@ TEST_CASE("Testing BaseObject class") CHECK_EQ(value[0].as(), string_value); CHECK_EQ(object->getAttribute("string").value()[0].as(), string_value); - CHECK(object->getAttribute("newAttribute", value)); - CHECK(!value.empty()); - CHECK(value == array_value); - CHECK_EQ(object->getAttribute("newAttribute").value(), array_value); - CHECK(object->getAttribute("inexistingAttribute", value) == false); CHECK(value.empty()); @@ -173,19 +167,19 @@ TEST_CASE("Testing BaseObject attribute registering") auto someString = std::string("What are you waiting for? Christmas?"); auto otherString = std::string("Show me the money!"); - CHECK(object->setAttribute("someAttribute", {42}) == BaseObject::SetAttrStatus::success); - auto handle = object->registerCallback("someAttribute", [&](const std::string& obj, const std::string& attr) { someString = otherString; }); + CHECK(object->setAttribute("integer", {42}) == BaseObject::SetAttrStatus::success); + auto handle = object->registerCallback("integer", [&](const std::string& obj, const std::string& attr) { someString = otherString; }); CHECK(static_cast(handle)); - object->setAttribute("someAttribute", {1337}); + object->setAttribute("integer", {1337}); CHECK(someString == otherString); object->unregisterCallback(handle); otherString = "I've got a flying machine!"; - object->setAttribute("someAttribute", {42}); + object->setAttribute("integer", {42}); CHECK(someString != otherString); - object->registerCallback("someAttribute", [&](const std::string& obj, const std::string& attr) { someString = otherString; }); - object->setAttribute("someAttribute", {1337}); + object->registerCallback("integer", [&](const std::string& obj, const std::string& attr) { someString = otherString; }); + object->setAttribute("integer", {1337}); CHECK(someString != otherString); }