Project

General

Profile

Actions

Feature #3732

closed

Face datasets: Flags field

Added by Junxiao Shi over 8 years ago. Updated about 8 years ago.

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

100%

Estimated time:
1.50 h

Description

Add Flags field to faces/list and faces/query datasets.

This issue includes necessary changes in ndn::nfd::FaceStatus struct.


Related issues 2 (0 open2 closed)

Blocked by NFD - Task #3226: Redesign faces/enable-local-controlClosedJunxiao Shi

Actions
Blocks NFD - Feature #3733: nfdc status: display face flagsClosedEric Newberry

Actions
Actions #1

Updated by Junxiao Shi over 8 years ago

  • Blocked by Task #3226: Redesign faces/enable-local-control added
Actions #2

Updated by Junxiao Shi over 8 years ago

  • Related to Feature #3733: nfdc status: display face flags added
Actions #3

Updated by Junxiao Shi over 8 years ago

  • Start date deleted (08/11/2016)
Actions #4

Updated by Davide Pesavento about 8 years ago

  • Related to deleted (Feature #3733: nfdc status: display face flags)
Actions #5

Updated by Davide Pesavento about 8 years ago

Actions #6

Updated by Eric Newberry about 8 years ago

  • Status changed from New to In Progress
Actions #7

Updated by Eric Newberry about 8 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 50

The ndn-cxx portion of this task has been uploaded for code review.

Actions #8

Updated by Junxiao Shi about 8 years ago

we should be considerate to other implementations when doing such changes. In this case, the encoding should be added to the end (= the first thing to be prepended). This allows other implementation to successfully decode even if it is not yet supporting flags.

I disagree.
The previous and current version of FaceStatus both specifies a complete struct without allowing arbitrary TLVs at either the middle or the end.
A correct decoding procedure for the previous version of FaceStatus should have checked there isn't any junk at the end.
Thus, any management struct change would be a breaking change no matter what.

Actions #9

Updated by Alex Afanasyev about 8 years ago

I disagree with your disagreement and will refer to the Postel's robustness principle.

Actions #10

Updated by Davide Pesavento about 8 years ago

Alex Afanasyev wrote:

I disagree with your disagreement and will refer to the Postel's robustness principle.

+1

Actions #11

Updated by Eric Newberry about 8 years ago

While looking into the NFD portion of this task, I believe I found an issue with the code that was just merged. Flags should be added to FaceTraits, instead of FaceStatus, so that FaceManager::collectFaceProperties continues to work with both FaceStatus and FaceEventNotification (by accepting FaceTraits).

Actions #12

Updated by Alex Afanasyev about 8 years ago

Eric, can you submit the fix asap, so it can be put in the soon to be released version of ndn-cxx?

Actions #13

Updated by Eric Newberry about 8 years ago

Alex Afanasyev wrote:

Eric, can you submit the fix asap, so it can be put in the soon to be released version of ndn-cxx?

I'll work on it tonight. When is the release?

Actions #14

Updated by Eric Newberry about 8 years ago

  • % Done changed from 50 to 100

The NFD portion has been pushed to Gerrit. This revision assumes that Flags is in FaceStatus, but not FaceEventNotification (or FaceTraits).

Actions #15

Updated by Eric Newberry about 8 years ago

I've added Flags to the FaceEventNotification spec on the wiki.

Actions #16

Updated by Eric Newberry about 8 years ago

  • Status changed from Code review to Closed

Adding Flags to FaceEventNotification was merged a few days ago.

Actions

Also available in: Atom PDF