ping: Code refactoring

Change-Id: I8b4e2c9dd3ba3adfae8d296a5635b048b35cf593
Refs: #3137

core/common.hpp

@@ -64,6 +64,7 @@
 #include <ndn-cxx/security/signing-helpers.hpp>
 #include <ndn-cxx/security/signing-info.hpp>
 #include <ndn-cxx/util/scheduler.hpp>
+#include <ndn-cxx/util/scheduler-scoped-event-id.hpp>
 #include <ndn-cxx/util/signal.hpp>
 
 namespace ndn {

tests/ping/integrated.t.cpp

@@ -22,14 +22,15 @@
 #include <ndn-cxx/util/dummy-client-face.hpp>
 
 #include "tests/test-common.hpp"
+#include "../identity-management-time-fixture.hpp"
 
 namespace ndn {
 namespace ping {
 namespace tests {
 
 using namespace ndn::tests;
 
-class PingIntegratedFixture : public UnitTestTimeFixture
+class PingIntegratedFixture : public IdentityManagementTimeFixture
 {
 public:
   PingIntegratedFixture()
@@ -93,7 +94,7 @@ BOOST_FIXTURE_TEST_CASE(Normal, PingIntegratedFixture)
   serverOpts.nMaxPings = 4;
   serverOpts.shouldPrintTimestamp = false;
   serverOpts.payloadSize = 0;
-  server.reset(new server::PingServer(*serverFace, serverOpts));
+  server.reset(new server::PingServer(*serverFace, m_keyChain, serverOpts));
   BOOST_REQUIRE_EQUAL(0, server->getNPings());
   server->start();
 
@@ -128,7 +129,7 @@ BOOST_FIXTURE_TEST_CASE(Timeout, PingIntegratedFixture)
   serverOpts.nMaxPings = 4;
   serverOpts.shouldPrintTimestamp = false;
   serverOpts.payloadSize = 0;
-  server.reset(new server::PingServer(*serverFace, serverOpts));
+  server.reset(new server::PingServer(*serverFace, m_keyChain, serverOpts));
   BOOST_REQUIRE_EQUAL(0, server->getNPings());
   server->start();
 

tests/ping/server/ping-server.t.cpp

@@ -21,6 +21,7 @@
 #include <ndn-cxx/util/dummy-client-face.hpp>
 
 #include "tests/test-common.hpp"
+#include "../../identity-management-time-fixture.hpp"
 
 namespace ndn {
 namespace ping {
@@ -31,13 +32,13 @@ using namespace ndn::tests;
 
 BOOST_AUTO_TEST_SUITE(PingServerPingServer)
 
-class CreatePingServerFixture : public UnitTestTimeFixture
+class CreatePingServerFixture : public IdentityManagementTimeFixture
 {
 protected:
   CreatePingServerFixture()
     : face(util::makeDummyClientFace(io, {false, true}))
     , pingOptions(makeOptions())
-    , pingServer(*face, pingOptions)
+    , pingServer(*face, m_keyChain, pingOptions)
   {
   }
 

tools/ping/client/ndn-ping.cpp

@@ -31,6 +31,89 @@ namespace ndn {
 namespace ping {
 namespace client {
 
+class Runner : noncopyable
+{
+public:
+  explicit
+  Runner(const Options& options)
+    : m_ping(m_face, options)
+    , m_statisticsCollector(m_ping, options)
+    , m_tracer(m_ping, options)
+    , m_signalSetInt(m_face.getIoService(), SIGINT)
+    , m_signalSetQuit(m_face.getIoService(), SIGQUIT)
+  {
+    m_signalSetInt.async_wait(bind(&Runner::afterIntSignal, this, _1));
+    m_signalSetQuit.async_wait(bind(&Runner::afterQuitSignal, this, _1));
+
+    m_ping.afterFinish.connect([this] {
+        this->cancel();
+      });
+  }
+
+  int
+  run()
+  {
+    try {
+      m_ping.start();
+      m_face.processEvents();
+    }
+    catch (std::exception& e) {
+      m_tracer.onError(e.what());
+      return 2;
+    }
+
+    Statistics statistics = m_statisticsCollector.computeStatistics();
+
+    std::cout << statistics << std::endl;
+
+    if (statistics.nReceived == statistics.nSent) {
+      return 0;
+    }
+    else {
+      return 1;
+    }
+  }
+
+private:
+  void
+  cancel()
+  {
+    m_signalSetInt.cancel();
+    m_signalSetQuit.cancel();
+    m_ping.stop();
+  }
+
+  void
+  afterIntSignal(const boost::system::error_code& errorCode)
+  {
+    if (errorCode == boost::asio::error::operation_aborted) {
+      return;
+    }
+
+    cancel();
+  }
+
+  void
+  afterQuitSignal(const boost::system::error_code& errorCode)
+  {
+    if (errorCode == boost::asio::error::operation_aborted) {
+      return;
+    }
+
+    m_statisticsCollector.computeStatistics().printSummary(std::cout);
+    m_signalSetQuit.async_wait(bind(&Runner::afterQuitSignal, this, _1));
+  };
+
+private:
+  Face m_face;
+  Ping m_ping;
+  StatisticsCollector m_statisticsCollector;
+  Tracer m_tracer;
+
+  boost::asio::signal_set m_signalSetInt;
+  boost::asio::signal_set m_signalSetQuit;
+};
+
 static time::milliseconds
 getMinimumPingInterval()
 {
@@ -61,34 +144,6 @@ usage(const boost::program_options::options_description& options)
   exit(2);
 }
 
-/**
- * @brief SIGINT handler: print statistics and exit
- */
-static void
-onSigInt(Face& face, StatisticsCollector& statisticsCollector)
-{
-  face.shutdown();
-  Statistics statistics = statisticsCollector.computeStatistics();
-  std::cout << statistics << std::endl;
-
-  if (statistics.nReceived == statistics.nSent) {
-    exit(0);
-  }
-  else {
-    exit(1);
-  }
-}
-
-/**
- * @brief SIGQUIT handler: print statistics summary and continue
- */
-static void
-onSigQuit(StatisticsCollector& statisticsCollector, boost::asio::signal_set& signalSet)
-{
-  statisticsCollector.computeStatistics().printSummary(std::cout);
-  signalSet.async_wait(bind(&onSigQuit, ref(statisticsCollector), ref(signalSet)));
-}
-
 int
 main(int argc, char* argv[])
 {
@@ -205,39 +260,8 @@ main(int argc, char* argv[])
     usage(visibleOptDesc);
   }
 
-  boost::asio::io_service ioService;
-  Face face(ioService);
-  Ping ping(face, options);
-  StatisticsCollector statisticsCollector(ping, options);
-  Tracer tracer(ping, options);
-
-  boost::asio::signal_set signalSetInt(face.getIoService(), SIGINT);
-  signalSetInt.async_wait(bind(&onSigInt, ref(face), ref(statisticsCollector)));
-
-  boost::asio::signal_set signalSetQuit(face.getIoService(), SIGQUIT);
-  signalSetQuit.async_wait(bind(&onSigQuit, ref(statisticsCollector), ref(signalSetQuit)));
-
   std::cout << "PING " << options.prefix << std::endl;
-
-  try {
-    ping.run();
-  }
-  catch (std::exception& e) {
-    tracer.onError(e.what());
-    face.getIoService().stop();
-    return 2;
-  }
-
-  Statistics statistics = statisticsCollector.computeStatistics();
-
-  std::cout << statistics << std::endl;
-
-  if (statistics.nReceived == statistics.nSent) {
-    return 0;
-  }
-  else {
-    return 1;
-  }
+  return Runner(options).run();
 }
 
 } // namespace client

tools/ping/client/ping.cpp

@@ -33,24 +33,23 @@ Ping::Ping(Face& face, const Options& options)
   , m_nOutstanding(0)
   , m_face(face)
   , m_scheduler(m_face.getIoService())
+  , m_nextPingEvent(m_scheduler)
 {
   if (m_options.shouldGenerateRandomSeq) {
     m_nextSeq = random::generateWord64();
   }
 }
 
 void
-Ping::run()
+Ping::start()
 {
-  start();
-
-  m_face.getIoService().run();
+  performPing();
 }
 
 void
-Ping::start()
+Ping::stop()
 {
-  performPing();
+  m_nextPingEvent.cancel();
 }
 
 void
@@ -73,7 +72,7 @@ Ping::performPing()
   ++m_nOutstanding;
 
   if ((m_options.nPings < 0) || (m_nSent < m_options.nPings)) {
-    m_scheduler.scheduleEvent(m_options.interval, bind(&Ping::performPing, this));
+    m_nextPingEvent = m_scheduler.scheduleEvent(m_options.interval, bind(&Ping::performPing, this));
   }
   else {
     finish();
@@ -105,9 +104,7 @@ Ping::finish()
     return;
   }
 
-  m_face.shutdown();
   afterFinish();
-  m_face.getIoService().stop();
 }
 
 Name

tools/ping/client/ping.hpp

@@ -56,51 +56,61 @@ class Ping : noncopyable
   Ping(Face& face, const Options& options);
 
   /**
-   * Signals on the successful return of a packet
+   * @brief Signals on the successful return of a packet
+   *
    * @param seq ping sequence number
    * @param rtt round trip time
    */
   signal::Signal<Ping, uint64_t, Rtt> afterResponse;
 
   /**
-   * Signals on timeout of a packet
+   * @brief Signals on timeout of a packet
+   *
    * @param seq ping sequence number
    */
   signal::Signal<Ping, uint64_t> afterTimeout;
 
   /**
-   * Signals when finished pinging
+   * @brief Signals when finished pinging
    */
   signal::Signal<Ping> afterFinish;
 
   /**
-   * Runs the ping client (blocking)
+   * @brief Start sending ping interests
+   *
+   * @note This method is non-blocking and caller need to call face.processEvents()
    */
   void
-  run();
+  start();
 
   /**
-   * Runs the ping client (non-blocking)
+   * @brief Stop sending ping interests
+   *
+   * This method cancels any future ping interests and does not affect already pending interests.
+   *
+   * @todo Cancel pending ping interest
    */
   void
-  start();
+  stop();
 
 private:
   /**
-   * Creates a ping Name from the sequence number
+   * @brief Creates a ping Name from the sequence number
+   *
    * @param seq ping sequence number
    */
   Name
   makePingName(uint64_t seq) const;
 
   /**
-   * Performs individual ping
+   * @brief Performs individual ping
    */
   void
   performPing();
 
   /**
-   * Called when ping returned successfully
+   * @brief Called when ping returned successfully
+   *
    * @param interest NDN interest
    * @param data returned data
    * @param seq ping sequence number
@@ -110,15 +120,16 @@ class Ping : noncopyable
   onData(const Interest& interest, Data& data, uint64_t seq, const time::steady_clock::TimePoint& sendTime);
 
   /**
-   * Called when ping timed out
+   * @brief Called when ping timed out
+   *
    * @param interest NDN interest
    * @param seq ping sequence number
    */
   void
   onTimeout(const Interest& interest, uint64_t seq);
 
   /**
-   * Called after ping received or timed out
+   * @brief Called after ping received or timed out
    */
   void
   finish();
@@ -130,6 +141,7 @@ class Ping : noncopyable
   int m_nOutstanding;
   Face& m_face;
   scheduler::Scheduler m_scheduler;
+  scheduler::ScopedEventId m_nextPingEvent;
 };
 
 } // namespace client