Project

General

Profile

Actions

Task #2874

closed

Decide directory structure for NDNLPv2

Added by Junxiao Shi almost 9 years ago. Updated almost 9 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Base
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
1.00 h

Description

Decide the directory structure for NDNLPv2 implementation.


Related issues 5 (0 open5 closed)

Related to ndn-cxx - Feature #2875: NDNLPv2: TLV-TYPE codesClosedEric Newberry

Actions
Related to ndn-cxx - Feature #2876: NDNLPv2: simple value type declarationClosedEric Newberry

Actions
Related to ndn-cxx - Feature #2877: NDNLPv2: NackReason abstractionAbandonedEric Newberry

Actions
Related to ndn-cxx - Feature #2878: NDNLPv2: Nack abstractionClosedEric Newberry

Actions
Blocked by ndn-cxx - Task #2841: NDNLPv2 design: packet format API for NACK, Fragmentation, NextHopFaceId, CachePolicy, IncomingFaceIdClosedJunxiao Shi

Actions
Actions #1

Updated by Junxiao Shi almost 9 years ago

  • Blocked by Task #2841: NDNLPv2 design: packet format API for NACK, Fragmentation, NextHopFaceId, CachePolicy, IncomingFaceId added
Actions #2

Updated by Junxiao Shi almost 9 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

NDNLP-TLV exists only in NFD, but it's necessary to implement NDNLPv2 in ndn-cxx because some features, such as Nack, can be used by applications.

Unlike LocalControlHeader, management/ is not the right place to put NDNLPv2 abstractions, because many NDNLPv2 features are not limited for local use, and thus do not fit in management.

My proposal is to create a new lp/ subdirectory for NDNLPv2.

The files will be:

  • lp/tlv.hpp: TLV-TYPE codes
  • lp/sequence.hpp: typedef for Sequence value type
  • lp/nack-reason.hpp: NackReason base type, a Doxygen comment to define NackReasonConcrete concept and a concept check
  • lp/detail/nack-reason-decl.hpp: templates and specializations used for declaring NackReasons
  • lp/duplicate.hpp: typedef for Duplicate type
  • lp/give-up.hpp: typedef for GiveUp type
  • lp/nack.hpp: Nack type
  • lp/caching-policy.hpp: CachingPolicy type
  • lp/field.hpp: a Doxygen comment to define Field concept and a concept check
  • lp/detail/field-decl.hpp: templates and specializations used for declaring fields
  • lp/fields.hpp: field declaration typedefs
Actions #3

Updated by Junxiao Shi almost 9 years ago

Actions #4

Updated by Junxiao Shi almost 9 years ago

  • Related to Feature #2876: NDNLPv2: simple value type declaration added
Actions #5

Updated by Junxiao Shi almost 9 years ago

  • Related to Feature #2877: NDNLPv2: NackReason abstraction added
Actions #6

Updated by Alex Afanasyev almost 9 years ago

I have no objections for this structure (I'm assuming it would go under src/)

Actions #7

Updated by Junxiao Shi almost 9 years ago

  • Status changed from Resolved to Closed

Thanks for approval. Of course it's src/lp/*.hpp|cpp for implementation and tests/unit-tests/lp/*.t.cpp for testing.

Actions #8

Updated by Eric Newberry almost 9 years ago

Where does the Packet class fit in this directory structure?

Actions #9

Updated by Junxiao Shi almost 9 years ago

Where does the Packet class fit in this directory structure?

Opps I missed that.

  • lp/packet.hpp: Packet class
Actions #10

Updated by Junxiao Shi almost 9 years ago

  • Status changed from Closed to Resolved

Due to Nack API change (#2841 note-31), there's only one Nack-related header:

  • lp/nack.hpp: Nack type
Actions #11

Updated by Junxiao Shi almost 9 years ago

In commit:12726d8645f08edc862259b03550abaff6008066, @Alex expressed concern about concepts.hpp being included everywhere.

I neither agree nor disagree with this concern.

Although nack-reason.hpp is gone, the same situation would happens for field.hpp under current design.

If there's a valid reason, I'll change the design.

@Alex, please state the reason why it's harmful to include concepts.hpp everywhere.

Notice that concept checks are header-only and thus does not incur runtime overhead.

Actions #12

Updated by Eric Newberry almost 9 years ago

Junxiao Shi wrote:

Due to Nack API change (#2841 note-31), there's only one Nack-related header:

  • lp/nack.hpp: Nack type

Are Nack, NackHeader, and NackReason all in lp/nack.hpp?

Actions #13

Updated by Junxiao Shi almost 9 years ago

Answer to note-12:

I forgot about the other two types. This should be:

  • lp/nack-header.hpp: NackReason type and NackHeader type
  • lp/nack.hpp: Nack type
Actions #14

Updated by Junxiao Shi almost 9 years ago

Actions #15

Updated by Junxiao Shi almost 9 years ago

  • Status changed from Resolved to Closed

Due to CachePolicy design change, the file for this header field is changed to be:

  • lp/cache-policy.hpp: CachePolicyType type and CachePolicy type
Actions

Also available in: Atom PDF