NFD » ndn-tools

+1

I would also like convert to use Boost.ProgramOptions library instead of getopt.

This is a complex Task.
It's recommended that Assignee should first design headers and submit for review, before writing implementation code.

• Code reviews should be uploaded to Gerrit.
• Have one .hpp (and .cpp) per component, not a big .hpp.
• Use ndn::util::signal instead of Boost.Signals2. As benchmarked in #2116 note-35, Boost.Signals2 is too slow.

Got it. Should I upload what I have to Gerrit, even if it's not finished?

Got it. Should I upload what I have to Gerrit, even if it's not finished?

Whenever you need feedback for some code, upload to Gerrit, even if it's just headers.

You may mark CodeReview-2 to prevent merging.

I believe I misunderstood what the role of Face was. Should output to stdout be done through Tracer?

Should output to stdout be done through Tracer?

This depends on what is being written:

• Banner, usage, version number, etc: written by the main function, or a helper function called by the main function.
• Log line for each successful or failed probe: written by Tracer.
• Statistics at end of the session: written by printStatistics function.

If anything, we should use uint32_t for m_nextPingNo, as that's what Interest::setNonce() expects as input.

No. ping reference number is not Nonce.

Ping uses startPingNo to determine whether to randomly generate the ping number (when set to -1, which is signed), so we have two options:

1. Keep it as int64_t/int32_t
2. Add another boolean value to options called shouldGenerateRandomPingNo and then use uint64_t/uint32_t

Which should I do?

It's better to be more explicit in Options struct.

/** \brief start sequence number
 */
uint32_t startSeq;

/** \brief whether to generate random start sequence number
 *
 *  If true, a random start sequence number is generated,
 *  and \p startSeq is ignored.
 */
bool wantRandomStartSeq;

In commit:cb4818c6ca423f11ae1e11c7daa5725c66d15dba, I notice that RTTs are printed as integers, but operators need to see higher precision.

Suggestion:

• in ping.hpp, declare typedef time::duration<double, time::milliseconds::period> Rtt;
• change second parameter of Ping::afterResponse signal to Rtt
• don't use division in StatisticsCollector::recordResponse
• don't use duration_cast in Tracer::onResponse