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

@@ -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

simgear/structure/commands.cxx

@@ -4,9 +4,7 @@
 //
 // $Id$
 
-#ifdef HAVE_CONFIG_H
+#include <simgear_config.h>
-#  include <simgear_config.h>
-#endif
 
 #include <memory>
 #include <cassert>
@@ -20,23 +18,46 @@
 #include <simgear/threads/SGGuard.hxx>
 #include <simgear/debug/logstream.hxx>
 
+struct Invocation
+{
+  std::string command;
+  SGPropertyNode_ptr args;
+};
+
+class SGCommandMgr::Private
+{
+public:
+  SGPropertyNode_ptr _rootNode;
+  
+  using InvocactionVec = std::vector<Invocation>;
+  InvocactionVec _queue;
+
+  SGMutex _mutex;
+
+  using command_map = std::map<std::string,Command*> ;
+  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())
+#if !defined(NDEBUG)
-        throw sg_exception("duplicate command name:" + name);
+  assert(SGThread::current() == d->_mainThreadId);
+#endif
+  if (d->_commands.find(name) != d->_commands.end())
+    throw sg_exception("duplicate command name:" + name);
 
-  _commands[name] = command;
+  d->_commands[name] = command;
 }
 
 SGCommandMgr::Command*
 SGCommandMgr::getCommand (const std::string &name) const
 {
-  const command_map::const_iterator it = _commands.find(name);
+  const auto it = d->_commands.find(name);
-  return (it != _commands.end() ? it->second : 0);
+  return (it != d->_commands.end() ? it->second : nullptr);
 }
 
 string_list
 SGCommandMgr::getCommandNames () const
 {
   string_list names;
-  command_map::const_iterator it = _commands.begin();
+  auto it = d->_commands.begin();
-  command_map::const_iterator last = _commands.end();
+  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 !defined(NDEBUG)
-    if (it == _commands.end())
+  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<SGMutex> 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<SGMutex> 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

simgear/structure/commands.hxx

@@ -15,6 +15,7 @@
 
 #include <string>
 #include <map>
+#include <memory>
 
 #include <simgear/math/sg_types.hxx>
 
@@ -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.
    *
@@ -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:
-
+    class Private;
-  typedef std::map<std::string,Command*> command_map;
+    std::unique_ptr<Private> d;
-  command_map _commands;
-
 };
 
 #endif // __COMMANDS_HXX

simgear/structure/test_commands.cxx

@@ -0,0 +1,128 @@
+#include <simgear_config.h>
+
+#include <cstdio>
+#include <algorithm>
+
+#include <simgear/compiler.h>
+#include <simgear/constants.h>
+#include <simgear/structure/commands.hxx>
+#include <simgear/misc/test_macros.hxx>
+#include <simgear/props/props.hxx>
+
+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;
+}