Subject: [core] YANG to CBOR mapping version 1
From: peter van der Stok <[email-masked]>
To: Core <[email-masked]>
Date: Tue, 05 Jul 2016 11:50:39 +0200
List-Id: "Constrained RESTful Environments \(CoRE\) Working Group list" <core.ietf.org>

Hi authors,

Attached my review of the document.
I have not looked at the enum, union encoding as this was done by Andy.

______________________________________________________________________________
YANG to CBOR
Section 1 line 2; remove “only”
Page 5; the table is referenced nowhere
End of section 2.1 “Deltas are represented as positive number” is 
contradicted in section 4.2 “SIDs as keys” second bullet: “Clients and 
servers MUST support negative deltas.
Suggestion: Deltas are represented as numbers preceded by a + or – sign
End page 5: “value need” -> “value needs”
Section 3: “the specification supports three types of keys.”
I would suggest only 2: character strings and numbers.
The use of deltas is either application (CoOL) specific or YANG to CBOR 
mapping specific.
If it is YANG to CBOR specific, I would encourage the use of another 
CBOR type that specifies a delta. As specified here its use is 
ambiguous; and forces each application to specify explicitly what is 
meant.
In the case that the delta is CoOL specific, I recommend to remove it 
completely from this draft, and explain the use of Deltas in the 
appropriate CoOL draft.
In the latter case: What remains are two key types: number and character 
string.
Last alinea of section 3: “entities generating” should that not be” 
agents (processes) generating”…
“The CoAP content-Format Option …. In the first place.”  I don’t 
understand this phrase.
Section 4.2 title:  ‘container” Schema Node. Is “CBOR map key” not a 
better title?
As suggested earlier: Reduce this to string (major type 3) and number 
(major type 0) keys.
I think that a large number of the CBOR encoding examples in section 4.2 
can be left out. They can be generated any time by an interested 
implementer; and now confuse the issues.
Section 4.4; the example is really complex; some explanation what this 
all illustrates would be welcome. Why is the use of choice necessary to 
understand the list encoding? The CBOR encoding can be removed. I 
suggest to first present the names example followed by the number 
example. The hash example is really not needed (see above).
Section 5: I need more explanation to relate the instance examples to 
the type definitions.
For example section 5.1; does the 1280 come from {mtu: 1280} ? Here we 
do need the CBOR encoding to see the relation between the type 
specification and the CBOR encoding.
Section 5.2 example illustrates major type 1? Not immediately clear 
unless you decode the CBOR.
Section 5.3; Start phrase got lost 3 lines lower.
In the example, where do I find in the CBOR encoding that the fraction 
is 2 digits?
Sections 5.4 - 5.8; see comments on section 5.1
Section 5.9 Here more text is needed to connect the complex YANG 
specification to “eth1” value.
Section 5.10; what is a name-space qualified form? In YANG-JSON it is 
defined but may need some repetition here.
Why is the subject SIDs as identityref* split in two parts? And again I 
find the CBOR encoding difficult to relate to the definition example.
Section 5.11
CBOR diagnostic is: {isrouter: [null]} ?
Section 5.13.
I don’t understand how this section relates to YANG to CBOR mapping. If 
the section refers to specifying an instance identifier in the request, 
I think it can be removed. The use of [key=value] as instance identifier 
is well explained in restconf draft. The use of numeric identifier is 
currently described in CoMI draft, and differently in CoOL draft. I 
propose to remove this section as it does not directly relate to CBOR 
encoding.

However, I am missing how to transport a given list instance with its 
key values in the CBOR encoding. Section 4.4 describes the encoding of a 
complete list not a subset of its instances.

_____________________________________________________________________________________________
Hope this helps,
Peter

-- 
Peter van der Stok
vanderstok consultancy
mailto: [email-masked]