ping: Code refactoring
Change-Id: I8b4e2c9dd3ba3adfae8d296a5635b048b35cf593
Refs: #3137
diff --git a/core/common.hpp b/core/common.hpp
index fa035c6..09d378c 100644
--- a/core/common.hpp
+++ b/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 {
diff --git a/tests/ping/integrated.t.cpp b/tests/ping/integrated.t.cpp
index f8cb30d..b8e2164 100644
--- a/tests/ping/integrated.t.cpp
+++ b/tests/ping/integrated.t.cpp
@@ -22,6 +22,7 @@
#include <ndn-cxx/util/dummy-client-face.hpp>
#include "tests/test-common.hpp"
+#include "../identity-management-time-fixture.hpp"
namespace ndn {
namespace ping {
@@ -29,7 +30,7 @@
using namespace ndn::tests;
-class PingIntegratedFixture : public UnitTestTimeFixture
+class PingIntegratedFixture : public IdentityManagementTimeFixture
{
public:
PingIntegratedFixture()
@@ -93,7 +94,7 @@
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 @@
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();
diff --git a/tests/ping/server/ping-server.t.cpp b/tests/ping/server/ping-server.t.cpp
index 874f1b2..81fb883 100644
--- a/tests/ping/server/ping-server.t.cpp
+++ b/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 @@
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)
{
}
diff --git a/tools/ping/client/ndn-ping.cpp b/tools/ping/client/ndn-ping.cpp
index a26b1b1..d94cd0c 100644
--- a/tools/ping/client/ndn-ping.cpp
+++ b/tools/ping/client/ndn-ping.cpp
@@ -31,6 +31,89 @@
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 @@
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 @@
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
diff --git a/tools/ping/client/ping.cpp b/tools/ping/client/ping.cpp
index 8f1ba98..b60be65 100644
--- a/tools/ping/client/ping.cpp
+++ b/tools/ping/client/ping.cpp
@@ -33,6 +33,7 @@
, m_nOutstanding(0)
, m_face(face)
, m_scheduler(m_face.getIoService())
+ , m_nextPingEvent(m_scheduler)
{
if (m_options.shouldGenerateRandomSeq) {
m_nextSeq = random::generateWord64();
@@ -40,20 +41,18 @@
}
void
-Ping::run()
-{
- start();
-
- m_face.getIoService().run();
-}
-
-void
Ping::start()
{
performPing();
}
void
+Ping::stop()
+{
+ m_nextPingEvent.cancel();
+}
+
+void
Ping::performPing()
{
BOOST_ASSERT((m_options.nPings < 0) || (m_nSent < m_options.nPings));
@@ -73,7 +72,7 @@
++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 @@
return;
}
- m_face.shutdown();
afterFinish();
- m_face.getIoService().stop();
}
Name
diff --git a/tools/ping/client/ping.hpp b/tools/ping/client/ping.hpp
index 3f99fd7..49ac492 100644
--- a/tools/ping/client/ping.hpp
+++ b/tools/ping/client/ping.hpp
@@ -56,51 +56,61 @@
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)
- */
- void
- run();
-
- /**
- * Runs the ping client (non-blocking)
+ * @brief Start sending ping interests
+ *
+ * @note This method is non-blocking and caller need to call face.processEvents()
*/
void
start();
+ /**
+ * @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
+ 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,7 +120,8 @@
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
*/
@@ -118,7 +129,7 @@
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 @@
int m_nOutstanding;
Face& m_face;
scheduler::Scheduler m_scheduler;
+ scheduler::ScopedEventId m_nextPingEvent;
};
} // namespace client
diff --git a/tools/ping/client/statistics-collector.hpp b/tools/ping/client/statistics-collector.hpp
index 76278db..2ced820 100644
--- a/tools/ping/client/statistics-collector.hpp
+++ b/tools/ping/client/statistics-collector.hpp
@@ -52,7 +52,7 @@
};
/**
- * @brief collects statistics from ping client
+ * @brief Statistics collector from ping client
*/
class StatisticsCollector : noncopyable
{
@@ -64,21 +64,22 @@
StatisticsCollector(Ping& ping, const Options& options);
/**
- * Returns ping statistics as structure
+ * @brief Compute ping statistics as structure
*/
Statistics
computeStatistics();
PUBLIC_WITH_TESTS_ELSE_PRIVATE:
/**
- * Called on ping response received
+ * @brief Called on ping response received
+ *
* @param rtt round trip time
*/
void
recordResponse(Rtt rtt);
/**
- * Called on ping timeout
+ * @brief Called on ping timeout
*/
void
recordTimeout();
diff --git a/tools/ping/client/tracer.hpp b/tools/ping/client/tracer.hpp
index 5a8cd35..f62b9b8 100644
--- a/tools/ping/client/tracer.hpp
+++ b/tools/ping/client/tracer.hpp
@@ -43,7 +43,8 @@
Tracer(Ping& ping, const Options& options);
/**
- * Prints ping results when response received
+ * @brief Prints ping results when response received
+ *
* @param seq ping sequence number
* @param rtt round trip time
*/
@@ -51,14 +52,15 @@
onResponse(uint64_t seq, Rtt rtt);
/**
- * Prints ping results when timed out
+ * @brief Prints ping results when timed out
+ *
* @param seq ping sequence number
*/
void
onTimeout(uint64_t seq);
/**
- * Outputs ping errors to cerr
+ * @brief Outputs ping errors to cerr
*/
void
onError(std::string msg);
diff --git a/tools/ping/server/ndn-ping-server.cpp b/tools/ping/server/ndn-ping-server.cpp
index 0a02805..2b9db41 100644
--- a/tools/ping/server/ndn-ping-server.cpp
+++ b/tools/ping/server/ndn-ping-server.cpp
@@ -30,6 +30,69 @@
namespace ping {
namespace server {
+class Runner : noncopyable
+{
+public:
+ explicit
+ Runner(const Options& options)
+ : m_options(options)
+ , m_pingServer(m_face, m_keyChain, options)
+ , m_tracer(m_pingServer, options)
+ , m_signalSetInt(m_face.getIoService(), SIGINT)
+ {
+ m_signalSetInt.async_wait(bind(&Runner::afterIntSignal, this, _1));
+
+ m_pingServer.afterFinish.connect([this] {
+ this->cancel();
+ });
+ }
+
+ int
+ run()
+ {
+ try {
+ m_pingServer.start();
+ m_face.processEvents();
+ }
+ catch (std::exception& e) {
+ std::cerr << "ERROR: " << e.what() << std::endl;
+ return 1;
+ }
+
+ std::cout << "\n--- ping server " << m_options.prefix << " ---" << std::endl;
+ std::cout << m_pingServer.getNPings() << " packets processed" << std::endl;
+
+ return 0;
+ }
+
+private:
+ void
+ cancel()
+ {
+ m_signalSetInt.cancel();
+ m_pingServer.stop();
+ }
+
+ void
+ afterIntSignal(const boost::system::error_code& errorCode)
+ {
+ if (errorCode == boost::asio::error::operation_aborted) {
+ return;
+ }
+
+ cancel();
+ }
+
+private:
+ const Options& m_options;
+ Face m_face;
+ KeyChain m_keyChain;
+ PingServer m_pingServer;
+ Tracer m_tracer;
+
+ boost::asio::signal_set m_signalSetInt;
+};
+
static time::milliseconds
getMinimumFreshnessPeriod()
{
@@ -48,36 +111,6 @@
exit(2);
}
-static void
-printStatistics(PingServer& pingServer, Options& options)
-{
- std::cout << "\n--- ping server " << options.prefix << " ---" << std::endl;
- std::cout << pingServer.getNPings() << " packets processed" << std::endl;
-}
-
-/**
- * @brief SIGINT handler: exits
- * @param pingServer pingServer instance
- * @param options options
- */
-static void
-onSigInt(PingServer& pingServer, Options& options, Face& face)
-{
- printStatistics(pingServer, options);
- face.shutdown();
- face.getIoService().stop();
- exit(1);
-}
-
-/**
- * @brief outputs errors to cerr
- */
-static void
-onError(std::string msg)
-{
- std::cerr << "ERROR: " << msg << std::endl;
-}
-
int
main(int argc, char* argv[])
{
@@ -169,30 +202,8 @@
usage(visibleOptDesc);
}
- boost::asio::io_service ioService;
- Face face(ioService);
- PingServer pingServer(face, options);
- Tracer tracer(pingServer, options);
-
- boost::asio::signal_set signalSetInt(face.getIoService(), SIGINT);
- signalSetInt.async_wait(bind(&onSigInt, ref(pingServer), ref(options), ref(face)));
-
std::cout << "PING SERVER " << options.prefix << std::endl;
-
- try {
- pingServer.run();
- }
- catch (std::exception& e) {
- onError(e.what());
- face.getIoService().stop();
- return 1;
- }
-
- printStatistics(pingServer, options);
- face.shutdown();
- face.getIoService().stop();
-
- return 0;
+ return Runner(options).run();
}
} // namespace server
@@ -203,4 +214,4 @@
main(int argc, char** argv)
{
return ndn::ping::server::main(argc, argv);
-}
\ No newline at end of file
+}
diff --git a/tools/ping/server/ping-server.cpp b/tools/ping/server/ping-server.cpp
index 799fa8d..08d17bf 100644
--- a/tools/ping/server/ping-server.cpp
+++ b/tools/ping/server/ping-server.cpp
@@ -26,11 +26,13 @@
namespace ping {
namespace server {
-PingServer::PingServer(Face& face, const Options& options)
+PingServer::PingServer(Face& face, KeyChain& keyChain, const Options& options)
: m_options(options)
+ , m_keyChain(keyChain)
, m_name(options.prefix)
, m_nPings(0)
, m_face(face)
+ , m_registeredPrefixId(nullptr)
{
shared_ptr<Buffer> b = make_shared<Buffer>();
b->assign(m_options.payloadSize, 'a');
@@ -38,22 +40,22 @@
}
void
-PingServer::run()
-{
- start();
-
- m_face.getIoService().run();
-}
-
-void
PingServer::start()
{
m_name.append("ping");
- m_face.setInterestFilter(m_name,
- bind(&PingServer::onInterest,
- this, _2),
- bind(&PingServer::onRegisterFailed,
- this, _2));
+ m_registeredPrefixId = m_face.setInterestFilter(m_name,
+ bind(&PingServer::onInterest,
+ this, _2),
+ bind(&PingServer::onRegisterFailed,
+ this, _2));
+}
+
+void
+PingServer::stop()
+{
+ if (m_registeredPrefixId != nullptr) {
+ m_face.unsetInterestFilter(m_registeredPrefixId);
+ }
}
int
@@ -77,8 +79,7 @@
++m_nPings;
if (m_options.shouldLimitSatisfied && m_options.nMaxPings > 0 && m_options.nMaxPings == m_nPings) {
- m_face.shutdown();
- m_face.getIoService().stop();
+ afterFinish();
}
}
@@ -90,4 +91,4 @@
} // namespace server
} // namespace ping
-} // namespace ndn
\ No newline at end of file
+} // namespace ndn
diff --git a/tools/ping/server/ping-server.hpp b/tools/ping/server/ping-server.hpp
index 242a54a..ff8e1c7 100644
--- a/tools/ping/server/ping-server.hpp
+++ b/tools/ping/server/ping-server.hpp
@@ -48,14 +48,20 @@
class PingServer : noncopyable
{
public:
- PingServer(Face& face, const Options& options);
+ PingServer(Face& face, KeyChain& keyChain, const Options& options);
/**
- * Signals when Interest received
+ * @brief Signals when Interest received
+ *
* @param name incoming interest name
*/
signal::Signal<PingServer, Name> afterReceive;
+ /**
+ * @brief Signals when finished pinging
+ */
+ signal::Signal<PingServer> afterFinish;
+
/** @brief starts ping server
*
* If options.shouldLimitSatisfied is false, this method does not return unless there's an error.
@@ -66,11 +72,19 @@
/**
* @brief starts the Interest filter
+ *
+ * @note This method is non-blocking and caller need to call face.processEvents()
*/
void
start();
/**
+ * @brief Unregister set interest filter
+ */
+ void
+ stop();
+
+ /**
* @brief gets the number of pings received
*/
int
@@ -78,14 +92,16 @@
private:
/**
- * Called when interest received
+ * @brief Called when interest received
+ *
* @param interest incoming interest
*/
void
onInterest(const Interest& interest);
/**
- * Called when prefix registration failed
+ * @brief Called when prefix registration failed
+ *
* @param reason reason for failure
*/
void
@@ -93,15 +109,17 @@
private:
const Options& m_options;
+ KeyChain& m_keyChain;
Name m_name;
- KeyChain m_keyChain;
int m_nPings;
Face& m_face;
Block m_payload;
+
+ const RegisteredPrefixId* m_registeredPrefixId;
};
} // namespace server
} // namespace ping
} // namespace ndn
-#endif //NDN_TOOLS_PING_SERVER_PING_SERVER_HPP
\ No newline at end of file
+#endif //NDN_TOOLS_PING_SERVER_PING_SERVER_HPP
diff --git a/tools/ping/server/tracer.cpp b/tools/ping/server/tracer.cpp
index fa4e2c8..305fc5d 100644
--- a/tools/ping/server/tracer.cpp
+++ b/tools/ping/server/tracer.cpp
@@ -43,4 +43,4 @@
} // namespace server
} // namespace ping
-} // namespace ndn
\ No newline at end of file
+} // namespace ndn
diff --git a/tools/ping/server/tracer.hpp b/tools/ping/server/tracer.hpp
index 1971f50..b8b6952 100644
--- a/tools/ping/server/tracer.hpp
+++ b/tools/ping/server/tracer.hpp
@@ -39,7 +39,7 @@
Tracer(PingServer& pingServer, const Options& options);
/**
- * Prints ping information when interest received
+ * @brief Prints ping information when interest received
* @param name interest name received
*/
void
@@ -53,4 +53,4 @@
} // namespace ping
} // namespace ndn
-#endif //NDN_TOOLS_PING_SERVER_TRACER_HPP
\ No newline at end of file
+#endif // NDN_TOOLS_PING_SERVER_TRACER_HPP