From 5fdf6ef4f3cb229ffdba1a68b745586c9308e6be Mon Sep 17 00:00:00 2001 From: James Turner Date: Sun, 24 Mar 2019 14:13:42 +0000 Subject: [PATCH] Simgear parts of queued command execution Extend the command API to allow posting commands from arbitrary threads, for deferred execution on the main thread. Additional pieces on the FG side to follow shortly. --- simgear/structure/CMakeLists.txt | 6 +- simgear/structure/commands.cxx | 115 ++++++++++++++++++++----- simgear/structure/commands.hxx | 53 ++++++++---- simgear/structure/test_commands.cxx | 128 ++++++++++++++++++++++++++++ 4 files changed, 266 insertions(+), 36 deletions(-) create mode 100644 simgear/structure/test_commands.cxx 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; +}