Re: [core] AD review of draft-ietf-core-senml-12

[Ari Keränen <[e-mail]>]

Hi Alexey,

Thank you for the review! And sorry for the late reply. We have now addressed all your comments in the PR 98:
https://github.com/core-wg/senml-spec/pull/98

Also see answers to your comments inline.


Cheers,
Ari

> On 11 Jan 2018, at 23.12, Alexey Melnikov <[e-mail]> wrote:
> 
> Hi,
> I nearly finished my review (I still need to double check a few things
> on fragment identifiers and media type registration compatibility with
> base media types like application/xml). I think the document is
> generally in a good shape, but there are a few things that we need to be
> fixed or discussed first.
> 
> I will also ask for a couple of extra reviews of this document.
> 
> Major:
> 
> 1) In order to be able to register application/senml+exi and
> application/sensml+exi, +exi suffix needs to be registered (+json, +xml
> and +cbor are already registered. See
> <https://www.iana.org/assignments/media-type-structured-suffix/media-type-structured-suffix.xhtml#structured-syntax-suffix>).
> There is separate registration procedure for this. Note that there was
> already a discussion about registering +exi and a Designated Expert
> opinion was that EXI is not a generic encoding format (JSON, CBOR and
> XML are) that can be decoded without knowing schemaID. The WG can argue
> against this point, but I am not convinced that the WG will convince the
> Designated Expert otherwise.
> 
> If you don't want to register the +exi suffix, the easiest fix to the
> document is to change "+" in application/senml+exi and
> application/sensml+exi to "-", as "-" doesn't have special semantics
> associated with suffixes.

Fixed by changing to "-exi" as you suggested and was discussed in this thread already.

> 
> 2) The following omissions are pretty easy to fix, but I think they are
> important enough for readers of documents:
> 
> First references to UTF-8, XML, Relax NG, XML Schema need Normative
> References. (Some of these are missing Normative References, some of
> these are just missing them on first use.)

Added references for each.

> I also suspect that some IESG members will have an opinion that the CDDL
> reference is Normative. So you might need to choose between making CDDL
> reference Normative (which will result in this document not being
> published until CDDL draft is also approved for publication) and
> removing CDDL definition from the document.

Clarified that this is informative (similar text was OK with earlier RFC):

 As a convenient reference, the JSON and CBOR representations can be
 described with the common CDDL [I-D.ietf-cbor-cddl] specification in
 Figure 1 (informative).

> Minor:
> 
> 
> Section 2:
> 
> Some devices have accurate time while others do not so SenML supports
> absolute and relative times.  Time is represented in floating point
> as seconds and values greater than zero represent an absolute time
> relative to the Unix epoch while values of 0 or less represent a
> 
> Does Unix epoch need a reference?

Added reference to the POSIX spec and added new clarifying text:

 Time is represented in floating point
 as seconds.  Values greater than zero represent an absolute time
 relative to the Unix epoch (1970-01-01T00:00Z in UTC time) and the
 time is counted same way as the Portable Operating System Interface
 (POSIX) "seconds since the epoch" [TIME_T]. Values of 0 or less
 represent a relative time in the past from the current time.

> relative time in the past from the current time.
> 
> 3.  Terminology
> 
> The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
> "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
> "OPTIONAL" in this document are to be interpreted as described in
> [RFC2119].
> 
> I suggest you switch to the updated phrasing that allows for lowercase
> "must", etc:
> 
> The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
> "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
> "OPTIONAL" in this document are to be interpreted as described in
> BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all
> capitals, as shown here.

Fixed as suggested.

> In 4.4
> 
> Although it is RECOMMENDED that concatenated names are represented as
> URIs [RFC3986] or URNs [RFC8141], the restricted character set
> specified above puts strict limits on the URI schemes and URN
> namespaces that can be used.
> 
> I understand that this phrasing is a result of discussions in the WG. Is
> it possible/useful to point out some typical URI schemes that can't be
> used here, such as HTTP/HTTPS/COAP?

Added two examples:
 However, the restricted character set
 does not allow the use of many URI schemes, such as the 'tag' scheme
 [RFC4151] and the 'ni' scheme [RFC6920], in names as such.

> In Section 5:
> 
> The mantissa SHOULD
> be less than 19 characters long and the exponent SHOULD be less than
> 5 characters long.
> 
> The above two SHOULDs: is the above sufficient for interoperability?
> What would happen if the SHOULD is not followed?

The SHOULDs are there to facilitate efficient parsing of the data. We added this:

  In the interest of avoiding unnecessary verbosity and speeding
  up processing, the mantissa SHOULD be less than 19 characters long
  and the exponent SHOULD be less than 5 characters long.

> In 5.1.2:
> 
>  SenML defines a
> separate media type to indicate Sensor Streaming Measurement Lists
> (SensML) for this usage (see Section 12.3.2).
> 
> It would be good to mention something about this in the IANA
> registrations for media types, as on a cursory look one can think that
> there are lots of duplicated registrations.

Added to 12.3 (Media Type Registrations): 

 This document registers media
 types for each serialization format of SenML (JSON, CBOR, and EXI)
 and also media types for the same formats of the streaming use
 (SensML).

> I just want to double check regarding binary data in XML. I assume there
> is a need to use XML escaping for bytes not otherwise allowed in XML? It
> might be worth pointing this out to avoid naive (and buggy) implementations.

Base64 encoding is used with binary data: https://tools.ietf.org/html/draft-ietf-core-senml-12#section-7

> The following comment applies to all media type registrations:
> 
> 12.3.1.  senml+json Media Type Registration
> 
> Interoperability considerations: Applications should ignore any JSON
> key value pairs that they do not understand.  This allows backwards
> compatibility extensions to this specification.  The "bver" field can
> be used to ensure the receiver supports a minimal level of
> functionality needed by the creator of the JSON object.
> 
> Is it worth mentioning here special treatment of keys that end with "_":
> 
> Implementations MUST ignore fields they don't recognize unless that
> field has a label name that ends with the '_' character in which
> case an error MUST be generated.
> 
> (The above text is from page 7. Might need to reword it in the media
> type registration template)?


Changed this to:
Interoperability considerations: Applications MUST ignore any JSON
key value pairs that they do not understand unless the key ends with
the '_' character in which case an error MUST be generated. This
allows backwards compatible extensions to this specification.

And similar changes to all other registrations.

> In Section 13:
> 
> You should also mention that defined formats don't include executable
> content. (Media Type reviewers typically ask this question)

Added to Section 13:
 The SenML
 formats defined by this specification do not contain any executable
 content.  However, future extensions could potentially embed
 application specific executable content in the data.

> Nits:
> 
> Abstract
> 
> This specification defines media types for representing simple sensor
> measurements and device parameters in the Sensor Measurement Lists
> (SenML).  Representations are defined in JavaScript Object Notation
> (JSON), Concise Binary Object Representation (CBOR), eXtensible
> Markup Language (XML), and Efficient XML Interchange (EXI), which
> share the common SenML data model.  A simple sensor, such as a
> temperature sensor, could use this media type in protocols such as
> 
> this media type --> one of these media types

Fixed.

> HTTP or CoAP to transport the measurements of the sensor or to be
> configured.
> 
> 
> First references to HTTP, COAP, S/MIME and TLS need Informative References.

Fixed.

> 12.1.  Units Registry
> 
> IANA will create a registry of SenML unit symbols.  The primary
> purpose of this registry is to make sure that symbols uniquely map to
> give type of measurement.
> 
> give --> given

Fixed.

> Best Regards,
> Alexey
> 
> 
> _______________________________________________
> core mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/core