Subject: Re: [core] draft-ietf-core-senml - Last Call comments

From: Carsten Bormann <[email-masked]>
Date: Fri, 27 Oct 2017 15:55:13 +0200

On Sep 23, 2017, at 07:24, Jim Schaad <[email-masked]> wrote:
> 
> * Section 6 - I am sad, but I understand the reasons for the decision, that
> Octets are not encoded as binary values.

Hi Jim,

the intention is that the bytes (“octets” in French) of the data value are encoded as a CBOR byte string;

   o  Characters in the String Value are encoded using a definite length
      text string (type 3).  Octets in the Data Value are encoded using
      a definite length byte string (type 2).

Is that not binary enough?

Grüße, Carsten

---

From: Ari Keränen <[email-masked]>
Date: Sat, 28 Oct 2017 09:32:05 +0000

Thank you for the review Jim! 

Here are two PRs addressing all the issues you raised:
https://github.com/core-wg/senml-spec/pull/83
https://github.com/core-wg/senml-spec/pull/81

Comments inline. There doesn't seem to be any controversy with these issues so we suggest to merge the PRs on Monday and submit a new version. The only substantial change is the clarification on how to handle Version field in resolved records.

Cheers,
Ari

> On 23 Sep 2017, at 8.24, Jim Schaad <[email-masked]> wrote:
> 
> I believe that this document is ready to proceed with some nits.
> 
> I am not really setup to do a competent review on the technical aspects of
> the document, however they seem to be clear to me.
> 
> * Section 4.2 - Update Time: I suggest that the sentence starts w/ "An
> optional value in sections that represents the maximum time".  The time
> after optional confused me as I initially read this as a time not as a delta
> time.  There may also be a requirement here that an actual time value exist
> somewhere so that one can compute the absolute time that an update should
> occur by.

Makes sense. I also aligned the text of update time better with the other definitions.

> * Section 4.4 - It is not clear to me that all of the base values in section
> 4.1 provide all of the information needed in order to resolve.  While I can
> infer most of them, I am unclear what to do with Version.

Good catch. This turned out to be a bit more substantial issue than we first thought since you actually need to have the version field, if it's something else than the default, in all the resolved records.

We suggest to change this to:

 A SenML Record is referred to as "resolved" if it does not contain any
 base values, i.e., labels starting with character 'b', except for
 Version field (see below), and has no relative times. To resolve the
 records the base values of the SenML Pack (if any) are applied to the
 Record. That is, name and base name are concatenated, base time is
 added to the time of the Record, if the Record did not contain Unit
 the Base Unit is applied to the record, etc. In addition the records
 need to be in chronological order.  [...]

 The Version field MUST NOT be present in resolved records if the SenML
 version defined in this document is used and MUST be present otherwise
 in all the resolved SenML Records.

> * Section 4.4 - It may be a good idea to indicate in this section that all
> base values are to start with the letter b so that if a new base value is
> found by a resolver it will know that it has a problem.

Fixed with above.

> * Section 6 - It might be a good idea to also provide the example in CBOR
> Diagnostic notation.   That is easier to read and gives a better idea of
> what the content structure is.

Agree, we'll add one (see PR 81).

> * Section 9 - I have a problem here because in CoAP fragments are not
> currently transmitted thus the example in 9.1 is probably not a useful item
> if you are trying to reduce the transmission size.  Do you just want to use
> a query parameter instead?  If not then you need to provide some additional
> explanation.

The fragment identifier is not trying to reduce transmission size. Exactly same representation is transmitted whether fragment ID is there or not. It's used only in client side. We tried to explain this in the first paragraph of sec 9.

> * Section 12.1 - You might want to include a note for the RFC editor to
> reverse the order of note 1 and note 2 as they do not appear in that order
> in the table.

Good catch, fixed.

> * Section 12.2 - I am not clear that the column "XML Type" should refer to
> XML.  I assume that this is also the type used for JSON and CBOR.

True. Changed that to just "Type". Also changed "ID" to "EXI ID".

> * Section 14 - You might want to re-iterate the Privacy consideration that
> you noted in Section 4.3 when dealing with name assignment.  The section you
> are dealing with has a reference for dealing with IPv6, but not for the
> other recommended name times.

ACK, we updated the privacy considerations section, now pointing to also RFC 6973, and refer to privacy section from 4.3.

---

From: Jim Schaad <[email-masked]>
Date: Sat, 28 Oct 2017 17:35:08 -0700

Looks fine.

> -----Original Message-----
> From: Ari Keränen [mailto:[email-masked]]
> Sent: Saturday, October 28, 2017 2:32 AM
> To: Jim Schaad <[email-masked]>; core <[email-masked]>
> Cc: [email-masked]
> Subject: Re: draft-ietf-core-senml - Last Call comments
> 
> Comments inline. There doesn't seem to be any controversy with these
> issues so we suggest to merge the PRs on Monday and submit a new version.
> The only substantial change is the clarification on how to handle Version
field
> in resolved records.
> > I am not really setup to do a competent review on the technical
> > aspects of the document, however they seem to be clear to me.
> >
> > optional value in sections that represents the maximum time".  The
> > time after optional confused me as I initially read this as a time not
> > as a delta time.  There may also be a requirement here that an actual
> > time value exist somewhere so that one can compute the absolute time
> > that an update should occur by.
> 
> > 4.1 provide all of the information needed in order to resolve.  While
> > I can infer most of them, I am unclear what to do with Version.
> 
> Good catch. This turned out to be a bit more substantial issue than we
first
> thought since you actually need to have the version field, if it's
something
> else than the default, in all the resolved records.
> 
base
> values, i.e., labels starting with character 'b', except for  Version
field (see
> below), and has no relative times. To resolve the  records the base values
of
> the SenML Pack (if any) are applied to the  Record. That is, name and base
> name are concatenated, base time is  added to the time of the Record, if
the
> Record did not contain Unit  the Base Unit is applied to the record, etc.
In
> addition the records  need to be in chronological order.  [...]
> 
> version defined in this document is used and MUST be present otherwise  in
> all the resolved SenML Records.
> 
> 
> > * Section 4.4 - It may be a good idea to indicate in this section that
> > all base values are to start with the letter b so that if a new base
> > value is found by a resolver it will know that it has a problem.
> 
> Fixed with above.
> 
> > currently transmitted thus the example in 9.1 is probably not a useful
> > item if you are trying to reduce the transmission size.  Do you just
> > want to use a query parameter instead?  If not then you need to
> > provide some additional explanation.
> 
> The fragment identifier is not trying to reduce transmission size. Exactly
same
> representation is transmitted whether fragment ID is there or not. It's
used
> only in client side. We tried to explain this in the first paragraph of
sec 9.
> 
> > * Section 12.1 - You might want to include a note for the RFC editor
> > to reverse the order of note 1 and note 2 as they do not appear in
> > that order in the table.
> 
> Good catch, fixed.
> 
> > * Section 12.2 - I am not clear that the column "XML Type" should
> > refer to XML.  I assume that this is also the type used for JSON and
CBOR.
> > * Section 14 - You might want to re-iterate the Privacy consideration
> > that you noted in Section 4.3 when dealing with name assignment.  The
> > section you are dealing with has a reference for dealing with IPv6,
> > but not for the other recommended name times.
> 
> ACK, we updated the privacy considerations section, now pointing to also
> RFC 6973, and refer to privacy section from 4.3.

---

From: Jim Schaad <[email-masked]>
Date: Tue, 31 Oct 2017 20:40:11 -0700

Carsten,

Sorry for the slow reply on this, I needed to figure out what my problem was and why I was not getting a good interpretation.

My real issue boils down to the table that you are asking IANA to create as a registry.  I believe that this table is missing a number of columns and some of the data in it may be considered to be wrong.  The fact that I asked for the column 'XML Type' to be changed to 'Type' is probably not helping here.

I think that the columns for the table should probably be:

Name, Label, Type, CBOR Key, XML/JSON Type, EXI ID, Note

The Type column holds the abstract type, which as long as it maps directly to a CBOR Type means that column is not needed.

The XML/JSON type for "Data Value" should probably hold "Base64 string" which, while the expected item, should be made explicit.

Jim

> From: Carsten Bormann [mailto:[email-masked]]
> Sent: Friday, October 27, 2017 6:55 AM
> To: Jim Schaad <[email-masked]>
> Cc: [email-masked]; [email-masked]
> > * Section 6 - I am sad, but I understand the reasons for the decision,
> > that Octets are not encoded as binary values.
> 
> Hi Jim,
>