Project

General

Profile

Actions

Task #1992

closed

Replace FaceFlags with individual fields

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

Status:
Closed
Priority:
Normal
Assignee:
Category:
Management
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
6.00 h

Description

Replace FaceFlags field with individual fields: FaceScope, FacePersistency, LinkType.

Spec: FaceMgmt rev41.

This Task involves changes in the following places:

  • ndn::nfd::FaceStatus type
  • ndn::nfd::FaceEventNotification type
  • Face::getFaceStatus and Face Dataset
  • Face Status Change Notification

Recommended:

  1. Declare a base class for common fields in FaceStatus and FaceEventNotification types, or declare a concept for getters and setters of those common fields.
  2. Modify Face::getFaceStatus to use this base class or concept.
  3. Populate face status change notification with modified Face::getFaceStatus.

Related issues 2 (0 open2 closed)

Related to NFD - Task #1991: Display face attribute fields instead of FaceFlagsClosedChengyu Fan

Actions
Blocks NFD - Task #1993: Face query operationClosedChengyu Fan

Actions
Actions #1

Updated by Junxiao Shi over 9 years ago

  • Related to Task #1991: Display face attribute fields instead of FaceFlags added
Actions #2

Updated by Junxiao Shi over 9 years ago

This Task has interlocked dependency with #1991. Suggested procedure is:

  1. In #1992, make changes to ndn::nfd::FaceStatus type, and mark old API as obsolete.
  2. #1991 and rest of #1992 can be worked in parallel.
  3. In #1992, delete old API after #1991 is completed.
Actions #3

Updated by Junxiao Shi over 9 years ago

Actions #4

Updated by Chengyu Fan over 9 years ago

  • Assignee set to Chengyu Fan
Actions #5

Updated by Chengyu Fan over 9 years ago

  • Status changed from New to In Progress
Actions #6

Updated by Chengyu Fan over 9 years ago

  • % Done changed from 0 to 70
Actions #7

Updated by Chengyu Fan over 9 years ago

Another part of this task is change Face::getFaceStatus and Face Dataset, should this part submit with #1991 or just submit as #1992 in NFD repository?

Actions #8

Updated by Junxiao Shi over 9 years ago

After the Change for ndn::nfd::FaceStatus type and ndn::nfd::FaceEventNotification type is merged, you can make two separate Changes for #1991 and rest of #1992 (except deleting old API). Those can be worked on in parallel.

After both of those Changes are merged, make a fourth Change to delete old API.

Actions #9

Updated by Alex Afanasyev over 9 years ago

Changes have been merged into ndn-cxx and corresponding changes need to be made to NFD. Currently, the master branch of NFD doesn't compile with the master branch of ndn-cxx.

Actions #10

Updated by Chengyu Fan over 9 years ago

will commit the change soon

Actions #11

Updated by Junxiao Shi over 9 years ago

The recently merged ndn-cxx commit is designed to be backwards-compatible, but a file is renamed and we forget to set a alias at old file name.

We should make a urgent commit to ndn-cxx that makes a symbolic link from nfd-face-flags.hpp to nfd-face-traits.hpp.

Actions #12

Updated by Chengyu Fan over 9 years ago

Can I just quickly commit the one to fix this issue, and latter commit others?

Actions #13

Updated by Chengyu Fan over 9 years ago

The quick fix is let face/face.hpp include the header nfd-face-traits.hpp, instead of nfd-face-flags.hpp.

Actions #14

Updated by Junxiao Shi over 9 years ago

No, you should update ndn-cxx to make it backward-compatible, not update NFD.

To test for backwards-compatibility, compile unchanged NFD and NLSR, and they should succeed.

Actions #15

Updated by Chengyu Fan over 9 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 70 to 100
Actions #16

Updated by Chengyu Fan over 9 years ago

  • Status changed from Code review to Closed
Actions #17

Updated by Junxiao Shi over 9 years ago

  • Status changed from Closed to Feedback

This Task also includes deleting deprecated APIs, after #1991 is closed.

Actions #18

Updated by Chengyu Fan over 9 years ago

For deleting deprecated APIs, do you mean delete the deprecated APIs in ndn-cxx?

Should it be done recently?

Actions #19

Updated by Junxiao Shi over 9 years ago

Yes, obsolete APIs can be deleted now.

The purpose of making the first commit backwards-compatible is to avoid breaking NFD outright.
After #1991 completion, NFD shouldn't have any code using obsolete APIs.

Actions #20

Updated by Chengyu Fan over 9 years ago

I think the obsolete APIs contain the face flag setter and getter, isLocal, isOnDemand, and FaceFlags enum.

But there are still some NFD codes use some of them:

  1. tests/daemon/mgmt/face-manager.cppm line 1602, 1664 use setFlags()
  2. tools/nfd-autoreg.cpp, line 162, 167, and 298 use isLocal or isOnDemand

I'll modify them first, and then move the obsolete APIs in ndn-cxx.
If I misunderstand anything, please let me know.

Actions #21

Updated by Junxiao Shi over 9 years ago

If there's any NFD code that uses obsolete APIs, they should be changed as part of this Task.

Actions #22

Updated by Chengyu Fan over 9 years ago

  • Status changed from Feedback to Closed
Actions

Also available in: Atom PDF