Project

General

Profile

Task #1992

Replace FaceFlags with individual fields

Added by Junxiao Shi about 5 years ago. Updated about 5 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

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

Blocks NFD - Task #1993: Face query operationClosed

History

#1 Updated by Junxiao Shi about 5 years ago

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

#2 Updated by Junxiao Shi about 5 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.

#3 Updated by Junxiao Shi about 5 years ago

#4 Updated by Chengyu Fan about 5 years ago

  • Assignee set to Chengyu Fan

#5 Updated by Chengyu Fan about 5 years ago

  • Status changed from New to In Progress

#6 Updated by Chengyu Fan about 5 years ago

  • % Done changed from 0 to 70

#7 Updated by Chengyu Fan about 5 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?

#8 Updated by Junxiao Shi about 5 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.

#9 Updated by Alex Afanasyev about 5 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.

#10 Updated by Chengyu Fan about 5 years ago

will commit the change soon

#11 Updated by Junxiao Shi about 5 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.

#12 Updated by Chengyu Fan about 5 years ago

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

#13 Updated by Chengyu Fan about 5 years ago

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

#14 Updated by Junxiao Shi about 5 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.

#15 Updated by Chengyu Fan about 5 years ago

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

#16 Updated by Chengyu Fan about 5 years ago

  • Status changed from Code review to Closed

#17 Updated by Junxiao Shi about 5 years ago

  • Status changed from Closed to Feedback

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

#18 Updated by Chengyu Fan about 5 years ago

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

Should it be done recently?

#19 Updated by Junxiao Shi about 5 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.

#20 Updated by Chengyu Fan about 5 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.

#21 Updated by Junxiao Shi about 5 years ago

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

#22 Updated by Chengyu Fan about 5 years ago

  • Status changed from Feedback to Closed

Also available in: Atom PDF