diff --git a/simgear/structure/CMakeLists.txt b/simgear/structure/CMakeLists.txt index 5d81ad67..fb13114d 100644 --- a/simgear/structure/CMakeLists.txt +++ b/simgear/structure/CMakeLists.txt @@ -39,7 +39,7 @@ set(SOURCES commands.cxx event_mgr.cxx exception.cxx - subsystem_mgr.cxx + subsystem_mgr.cxx StateMachine.cxx ) @@ -63,6 +63,10 @@ add_executable(test_shared_ptr shared_ptr_test.cpp) target_link_libraries(test_shared_ptr ${TEST_LIBS}) add_test(shared_ptr ${EXECUTABLE_OUTPUT_PATH}/test_shared_ptr) +add_executable(test_commands test_commands.cxx) +target_link_libraries(test_commands ${TEST_LIBS}) +add_test(subsystems ${EXECUTABLE_OUTPUT_PATH}/test_commands) + endif(ENABLE_TESTS) add_boost_test(function_list diff --git a/simgear/structure/commands.cxx b/simgear/structure/commands.cxx index 150e16aa..5621c8d8 100644 --- a/simgear/structure/commands.cxx +++ b/simgear/structure/commands.cxx @@ -4,9 +4,7 @@ // // $Id$ -#ifdef HAVE_CONFIG_H -# include -#endif +#include #include #include @@ -20,23 +18,46 @@ #include #include +struct Invocation +{ + std::string command; + SGPropertyNode_ptr args; +}; + +class SGCommandMgr::Private +{ +public: + SGPropertyNode_ptr _rootNode; + + using InvocactionVec = std::vector; + InvocactionVec _queue; + + SGMutex _mutex; + + using command_map = std::map ; + command_map _commands; + + long _mainThreadId = 0; +}; //////////////////////////////////////////////////////////////////////// // Implementation of SGCommandMgr class. //////////////////////////////////////////////////////////////////////// -static SGCommandMgr* static_instance = NULL; +static SGCommandMgr* static_instance = nullptr; -SGCommandMgr::SGCommandMgr () +SGCommandMgr::SGCommandMgr () : + d(new Private) { - assert(static_instance == NULL); + d->_mainThreadId = SGThread::current(); + assert(static_instance == nullptr); static_instance = this; } SGCommandMgr::~SGCommandMgr () { assert(static_instance == this); - static_instance = NULL; + static_instance = nullptr; } SGCommandMgr* @@ -45,28 +66,37 @@ SGCommandMgr::instance() return static_instance ? static_instance : new SGCommandMgr; } +void SGCommandMgr::setImplicitRoot(SGPropertyNode *root) +{ + assert(root); + d->_rootNode = root; +} + void SGCommandMgr::addCommandObject (const std::string &name, Command* command) { - if (_commands.find(name) != _commands.end()) - throw sg_exception("duplicate command name:" + name); - - _commands[name] = command; +#if !defined(NDEBUG) + assert(SGThread::current() == d->_mainThreadId); +#endif + if (d->_commands.find(name) != d->_commands.end()) + throw sg_exception("duplicate command name:" + name); + + d->_commands[name] = command; } SGCommandMgr::Command* SGCommandMgr::getCommand (const std::string &name) const { - const command_map::const_iterator it = _commands.find(name); - return (it != _commands.end() ? it->second : 0); + const auto it = d->_commands.find(name); + return (it != d->_commands.end() ? it->second : nullptr); } string_list SGCommandMgr::getCommandNames () const { string_list names; - command_map::const_iterator it = _commands.begin(); - command_map::const_iterator last = _commands.end(); + auto it = d->_commands.begin(); + auto last = d->_commands.end(); while (it != last) { names.push_back(it->first); ++it; @@ -77,13 +107,24 @@ SGCommandMgr::getCommandNames () const bool SGCommandMgr::execute (const std::string &name, const SGPropertyNode * arg, SGPropertyNode *root) const { +#if !defined(NDEBUG) + if (SGThread::current() != d->_mainThreadId) { + SG_LOG(SG_GENERAL, SG_WARN, "calling SGCommandMgr::execute from a different thread than expected. Command is:" << name); + } + // assert(SGThread::current() == d->_mainThreadId); +#endif Command* command = getCommand(name); - if (command == 0) + if (command == nullptr) { SG_LOG(SG_GENERAL, SG_WARN, "command not found: '" << name << "'"); return false; } + // use implicit root node if caller did not define one explicitly + if (root == nullptr) { + root = d->_rootNode.get(); + } + try { return (*command)(arg, root); @@ -122,13 +163,47 @@ SGCommandMgr::execute (const std::string &name, const SGPropertyNode * arg, SGPr bool SGCommandMgr::removeCommand(const std::string& name) { - command_map::iterator it = _commands.find(name); - if (it == _commands.end()) +#if !defined(NDEBUG) + assert(SGThread::current() == d->_mainThreadId); +#endif + auto it = d->_commands.find(name); + if (it == d->_commands.end()) return false; - + delete it->second; - _commands.erase(it); + d->_commands.erase(it); return true; } +void SGCommandMgr::queuedExecute(const std::string &name, const SGPropertyNode* arg) +{ + Invocation invoke = {name, new SGPropertyNode}; + copyProperties(arg, invoke.args); + SGGuard g(d->_mutex); + d->_queue.push_back(invoke); +} + +void SGCommandMgr::executedQueuedCommands() +{ +#if !defined(NDEBUG) + assert(SGThread::current() == d->_mainThreadId); +#endif + + // locked swap with the shared queue + Private::InvocactionVec q; + { + SGGuard g(d->_mutex); + d->_queue.swap(q); + } + + for (auto i : q) { + bool ok = execute(i.command, i.args); + if (!ok) { + SG_LOG(SG_GENERAL, SG_WARN, "queued execute of command " << i.command + << "failed"); + } + } +} + + // end of commands.cxx diff --git a/simgear/structure/commands.hxx b/simgear/structure/commands.hxx index 58d8c281..e8bf4e96 100644 --- a/simgear/structure/commands.hxx +++ b/simgear/structure/commands.hxx @@ -15,12 +15,13 @@ #include #include +#include #include // forward decls class SGPropertyNode; - + /** * Manage commands. * @@ -44,7 +45,7 @@ public: virtual bool operator()(const SGPropertyNode * arg, SGPropertyNode *root) = 0; }; - + typedef bool (*command_t) (const SGPropertyNode * arg, SGPropertyNode *root); private: @@ -74,7 +75,7 @@ private: ObjPtr pObj_; MemFn pMemFn_; }; - + /** * Helper template functions. */ @@ -84,7 +85,7 @@ private: { return new MethodCommand(pObj, pMemFn ); } - + public: /** * Default constructor (sets instance to created item) @@ -98,6 +99,12 @@ public: static SGCommandMgr* instance(); + /** + * specify the root node to use if one is not explicitly provided when + * executing a command. + */ + void setImplicitRoot(SGPropertyNode* root); + /** * Register a new command with the manager. * @@ -109,15 +116,15 @@ public: */ void addCommand(const std::string& name, command_t f) { addCommandObject(name, new FunctionCommand(f)); } - + void addCommandObject (const std::string &name, Command* command); template void addCommand(const std::string& name, const OBJ& o, METHOD m) - { + { addCommandObject(name, make_functor(o,m)); } - + /** * Look up an existing command. * @@ -147,21 +154,37 @@ public: * @return true if the command is present and executes successfully, * false otherwise. */ - virtual bool execute (const std::string &name, const SGPropertyNode * arg, SGPropertyNode *root) const; + virtual bool execute (const std::string &name, const SGPropertyNode * arg, SGPropertyNode *root = nullptr) const; + + /** + * Queue a command for execution on the main thread / thread owning + * this command manager. (In practice in FlightGear, this means the same thing) + * The argument node will be copied immediately, so changes made after this call + * will not be reflected when the command is executed. + * + * Note there is no way to unqueue a command, or find out when the execution + * actually occurs, at present. Queued ommands are executed in the order submitted, + * however, which can be used to infer completion. + * + * Queued execution always uses the implicit root node. + */ + void queuedExecute(const std::string &name, const SGPropertyNode* arg); + + /** + * Dispatch queued command. FlightGear calls this once per frame + * on the main thread. + * + */ + void executedQueuedCommands(); /** * Remove a command registration */ bool removeCommand(const std::string& name); -protected: - - private: - - typedef std::map command_map; - command_map _commands; - + class Private; + std::unique_ptr d; }; #endif // __COMMANDS_HXX diff --git a/simgear/structure/test_commands.cxx b/simgear/structure/test_commands.cxx new file mode 100644 index 00000000..a107e8b0 --- /dev/null +++ b/simgear/structure/test_commands.cxx @@ -0,0 +1,128 @@ +#include + +#include +#include + +#include +#include +#include +#include +#include + +using std::string; +using std::cout; +using std::cerr; +using std::endl; + +SGPropertyNode_ptr test_rootNode; + +bool commandAFunc(const SGPropertyNode* args, SGPropertyNode* root) +{ + SG_VERIFY(root == test_rootNode.get()); + root->setIntValue("propA", args->getIntValue("argA")); + return true; +} + +bool commandBFunc(const SGPropertyNode* args, SGPropertyNode* root) +{ + SG_VERIFY(root == test_rootNode.get()); + root->setIntValue("propB", args->getIntValue("argB")); + return args->getBoolValue("result"); +} + +void testBasicCommands() +{ + test_rootNode.reset(new SGPropertyNode); + SGCommandMgr::instance()->setImplicitRoot(test_rootNode.get()); + + SGCommandMgr::instance()->addCommand("cmd-a", commandAFunc); + SGCommandMgr::instance()->addCommand("cmd-b", commandBFunc); + + auto names = SGCommandMgr::instance()->getCommandNames(); + SG_CHECK_EQUAL(names.size(), 2); + SG_VERIFY(std::count(names.begin(), names.end(), "cmd-a")); + SG_VERIFY(std::count(names.begin(), names.end(), "cmd-b")); + + { + SGPropertyNode_ptr args(new SGPropertyNode); + args->setIntValue("argA", 42); + bool ok = SGCommandMgr::instance()->execute("cmd-a", args); + SG_VERIFY(ok); + SG_CHECK_EQUAL(test_rootNode->getIntValue("propA"), 42); + } + + { + SGPropertyNode_ptr args(new SGPropertyNode); + args->setIntValue("argB", 99); + args->setBoolValue("result", true); + bool ok = SGCommandMgr::instance()->execute("cmd-b", args); + SG_VERIFY(ok); + SG_CHECK_EQUAL(test_rootNode->getIntValue("propB"), 99); + } + + SGCommandMgr::instance()->removeCommand("cmd-a"); + names = SGCommandMgr::instance()->getCommandNames(); + SG_CHECK_EQUAL(names.size(), 1); + SG_VERIFY(std::count(names.begin(), names.end(), "cmd-b")); + + // check we can't execute a removed command + { + SGPropertyNode_ptr args(new SGPropertyNode); + args->setIntValue("argA", 66); + bool ok = SGCommandMgr::instance()->execute("cmd-a", args); + SG_VERIFY(!ok); + SG_CHECK_EQUAL(test_rootNode->getIntValue("propA"), 42); + } +} + +/////////////////////////////////////////////////////////////////////////////// + +void testQueuedExec() +{ + // delete the previous instance, so next call re-creates + delete SGCommandMgr::instance(); + + test_rootNode.reset(new SGPropertyNode); + test_rootNode->setIntValue("propA", 71); + + SGCommandMgr::instance()->setImplicitRoot(test_rootNode.get()); + + SGCommandMgr::instance()->addCommand("cmd-a", commandAFunc); + SGCommandMgr::instance()->addCommand("cmd-b", commandBFunc); + + { + SGPropertyNode_ptr args(new SGPropertyNode); + args->setIntValue("argA", 99); + SGCommandMgr::instance()->queuedExecute("cmd-a", args); + + // ensure args are cpatured during enqueue call + args->setIntValue("argA", 101); + } + + // not executed yet + SG_CHECK_EQUAL(test_rootNode->getIntValue("propA"), 71); + + { + SGPropertyNode_ptr args(new SGPropertyNode); + args->setIntValue("argB", 1234); + args->setBoolValue("result", true); + SGCommandMgr::instance()->queuedExecute("cmd-b", args); + } + + SGCommandMgr::instance()->executedQueuedCommands(); + + SG_CHECK_EQUAL(test_rootNode->getIntValue("propA"), 99); + SG_CHECK_EQUAL(test_rootNode->getIntValue("propB"), 1234); +} + +/////////////////////////////////////////////////////////////////////////////// + + +int main(int argc, char* argv[]) +{ + testBasicCommands(); + testQueuedExec(); + + cout << __FILE__ << ": All tests passed" << endl; + return EXIT_SUCCESS; +}