Skip to main content

Last Call Review of draft-ietf-core-senml-etch-05

Request Review of draft-ietf-core-senml-etch
Requested revision No specific revision (document currently at 07)
Type Last Call Review
Team Internet of Things Directorate (iotdir)
Deadline 2019-09-02
Requested 2019-08-21
Requested by Éric Vyncke
Authors Ari Keränen , Mojan Mohajer
I-D last updated 2019-09-02
Completed reviews Opsdir Last Call review of -05 by Carlos Pignataro (diff)
Iotdir Last Call review of -05 by Matthias Kovatsch (diff)
Secdir Last Call review of -05 by Christian Huitema (diff)
Genart Last Call review of -05 by Robert Sparks (diff)
I would appreciate a review of this document with the IoT point of view as it targets constrained device.

Please share the review with the mailing list as it is a Last Call review.

Thank you for reviewing it.

Assignment Reviewer Matthias Kovatsch
State Completed
Review review-ietf-core-senml-etch-05-iotdir-lc-kovatsch-2019-09-02
Posted at
Reviewed revision 05 (document currently at 07)
Result Ready w/nits
Completed 2019-09-02
Dear authors and list members

Here is my review for draft-ietf-core-senml-etch-05 from the IoT perspective.

## Summary

draft-ietf-core-senml-etch-05 defines new media types and their semantics for
two new SenML patch document formats (JSON and CBOR, resp.). The complexity
added to implementations that can already handle SenML is marginal and
straight-forward. Hence, I do not see any issue for constrained devices. The
explicit media types help in IoT scenarios, where machines communicate with

There are a few minor issues that can be solved by the authors alone. Hence, I
marked the result as "Ready with Nits".

It would be good to get the help from an expert on Windows Clipboard Formats
and Macintosh Uniform Type Identifiers, as no good guidelines are available to
check these IANA considerations. (This issue appears to be recurrent also for
other specs.)

## Technical comments

* P4 (3.1): I am missing assertions such as "Values in a Fetch Record MUST be
  * What should happen when a Patch Record does not have a value?

* P5 §3: The record must not be added when the value is null. (behavior not
described formally enough)

* P7: "Windows Clipboard Name" --> Microsoft and for instance HTML spec use
"Windows Clipboard Format"
  * Okay, the sting itself is the Windows Clipboard Format Name...
  * The long string with spaces ("SenML FETCH/PATCH format") is a bit weird for
  this purpose, no?
    * I also had the problem to find proper guidelines for Windows Clipboard
    Formats; are there any?
  * No Macintosh Uniform Type Identifier?

## Additional comment

* As already discussed with one of the authors, an implication for LwM2M is
probably that these patch documents must not be used with Executable Resources
(one might try to execute multiple resources at once with a PATCH method). The
application of a Patch Pack is then not idempotent anymore. Furthermore, it is
unclear what the value should be when the LwM2M Executable Resource does not
take arguments. * If executing multiple resources atomically is an important
use case, I think we need another iteration to deal with the state vs RPC issue
("use PATCH to call function(s) without arguments by giving a new state?!")

## Editorial comments

* P1 §1 (Abstract), P2 last §: "iPATCH, PATCH, and FETCH" --> "FETCH, PATCH,
and iPATCH"
  * It is easier on the brain if the order is kept consistent...

* P2 §6: "hence full name" --> "hence the unique identifiers" ?
  * RFC 8428 does not define or contain "full name", but "globally unique
  identifier for the resource"

* P3 §1: "The semantics of the ..."
  * Creates question about semantics for FETCH
  * Better to reverse sentences and start with "The rest of the document uses
  the term "(i)PATCH" to refer to both methods, as the semantics of the new
  media types are the same for the CoAP PATCH and iPATCH methods."

* P3 §1: ", that can be used with the" --> ", which ..."
* P3 §3: "Also the following ..." --> to many "also", just "The following ..."

* P4 §2 (3.1): "... when resolved, match resolved names" --> "identifiers"
  * names when resolved are resolved names, hence unclear what is compared
  * P2 calls them "full names"
  * See above, should be something like "globally unique identifier for the
* P4 §8: Add example for records with name and time
  * Would be good to quickly show what "resolved form of records" means
* P4 §9 (3.2): Add statement that SenML patch documents are always idempotent,
hence PATCH and iPATCH are equivalent?
  * Basically move the last sentence to the beginning and give explanation for

* P5 §2: "When the name" --> "When the resolved name" ?

Kind regards,