Sensor Measurement Lists (SenML)
draft-ietf-core-senml-16

Note: This ballot was opened for revision 14 and is now closed.

Alissa Cooper Yes

(Spencer Dawkins) Yes

Comment (2018-04-15 for -14)
No email
send info
Thank you for doing this work. It's impressive.

Alexey Melnikov Yes

Comment (2018-04-19 for -14)
No email
send info
Authors also need to reply to the IANA review from Klaus Hartke. He has rejected some CoAP registrations as they currently stand (IANA ticket #1055261), and he hasn't heard back from the authors yet.

Ignas Bagdonas No Objection

Deborah Brungard No Objection

(Ben Campbell) (was Discuss) No Objection

Comment (2018-05-18)
No email
send info
Thank you for addressing my DISCUSS by adding the additional text to section 13, and for addressing my other comments.

The change to section 13 is sufficient for me to clear the discuss, but I suggest adding a sentence to the following effect to the end of the new paragraph:

“Malicious use of SenML to change system state could have severe consequences, potentially including violation of physical security, property damage, and even loss of life."

Suresh Krishnan No Objection

Warren Kumari No Objection

Mirja Kühlewind No Objection

(Terry Manderson) No Objection

(Eric Rescorla) No Objection

Comment (2018-04-16 for -14)
No email
send info
Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D4594







COMMENTS
>   
>   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

You're kind of burying the lede here. This document defines SenML, it
doesn't just define media type.


>      measurement every second, batch up 60 of them, and then send the
>      batch to a server.  It would include the relative time each
>      measurement was made compared to the time the batch was sent in each
>      SenML Record.  The server might have accurate NTP time and use the
>      time it received the data, and the relative offset, to replace the
>      times in the SenML with absolute times before saving the SenML Pack

You use "Pack" here before you define it in  S 3.


>      kinds of fields: base and regular.  The base fields can be included
>      in any SenML Record and they apply to the entries in the Record.
>      Each base field also applies to all Records after it up to, but not
>      including, the next Record that has that same base field.  All base
>      fields are optional.  Regular fields can be included in any SenML
>      Record and apply only to that Record.

It looks like it's permissible to intermix Base and Regular fields in
the same record. Assuming that's so, it would be helpful to say so.


>         ("vs" for "String Value") and binary data ("vd" for "Data Value").
>         Exactly one value field MUST appear unless there is Sum field in
>         which case it is allowed to have no Value field.
>   
>      Sum:  Integrated sum of the values over time.  Optional.  This field
>         is in the unit specified in the Unit value multiplied by seconds.

This text is hard to read, but the dimensional analysis seems
potentially wrong, for at least some measurements. I assume you're
thinking of something like watts and watt-hours. but if the value is
for instance bits, then "bit-seconds" is not a meaningful unit, and
yet you might want to sum these.


>   
>      The SenML format can be extended with further custom fields.  Both
>      new base and regular fields are allowed.  See Section 12.2 for
>      details.  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.

How does this map to CBOR? I see you explain this later, but here
would be helpful


>      concatenated name MUST consist only of characters out of the set "A"
>      to "Z", "a" to "z", "0" to "9", "-", ":", ".", "/", and "_";
>      furthermore, it MUST start with a character out of the set "A" to
>      "Z", "a" to "z", or "0" to "9".  This restricted character set was
>      chosen so that concatenated names can be used directly within various
>      URI schemes (including segments of an HTTP path with no special

Assuming I am understanding this correctly, then "/" is a problem
inside a path component.


>      specified above puts strict limits on the URI schemes and URN
>      namespaces that can be used.  As a result, implementers need to take
>      care in choosing the naming scheme for concatenated names, because
>      such names both need to be unique and need to conform to the
>      restricted character set.  One approach is to include a bit string
>      that has guaranteed uniqueness (such as a 1-wire address).  Some of

citation for 1-wire please.


>                      | Encoding | Size | Compressed Size |
>                      +----------+------+-----------------+
>                      | JSON     |  573 |             206 |
>                      | XML      |  649 |             235 |
>                      | CBOR     |  254 |             196 |
>                      | EXI      |  161 |             184 |

Compression not working out so well for EXI


>   
>      To select a single SenML Record, the "rec" scheme followed by a
>      single number is used.  For the purpose of numbering records, the
>      first record is at position 1.  A range of records can be selected by
>      giving the first and the last record number separated by a '-'
>      character.  Instead of the second number, the '*' character can be

This is an odd notation as * usually means "all". Why not just omit
the terminal #?


>   
>      New entries can be added to the registration by Expert Review as
>      defined in [RFC8126].  Experts should exercise their own good
>      judgment but need to consider that shorter labels should have more
>      strict review.  New entries should not be made that counteract the
>      advice at the end of Section 4.4.

Note that you say earlier that you don't expect to define new CBOR
integers. That should probably be repeated here.


>      Sensor data can range from information with almost no security
>      considerations, such as the current temperature in a given city, to
>      highly sensitive medical or location data.  This specification
>      provides no security protection for the data but is meant to be used
>      inside another container or transport protocol such as S/MIME
>      [RFC5751] or HTTP with TLS [RFC5246] that can provide integrity,

This is the wrong citation for HTTP over TLS. That's RFC 2818.


>      We would like to thank Alexander Pelov, Alexey Melnikov, Andrew
>      McClure, Andrew McGregor, Bjoern Hoehrmann, Christian Amsuess,
>      Christian Groves, Daniel Peintner, Jan-Piet Mens, Jim Schaad, Joe
>      Hildebrand, John Klensin, Karl Palsson, Lennart Duhrsen, Lisa
>      Dusseault, Lyndsay Campbell, Martin Thomson, Michael Koster, Peter
>      Saint-Andre, Roni Even, and Stephen Farrell, for their review

Nit: no comma after Farrell

Alvaro Retana No Objection

Adam Roach (was Discuss) No Objection

Comment (2018-05-07 for -15)
No email
send info
Thanks for addressing my DISCUSS and comments.

Martin Vigoureux No Objection

Benjamin Kaduk (was Discuss) Abstain

Comment (2018-05-18)
No email
send info
Thank you for addressing my comments!

I am changing my position to Abstain, because I am still uncomfortable
with the possibility for non-hierarchical, interleaving use of
base values, but recognize that the WG has consensus to do so, and
I will not stand in the way.

Original ballot text preserved below.

DISCUSS

I agree with Ben's DISCUSS.

Additionally, I have serious reservations about introducing the
concept of "base fields" that apply to subsequent array elemnets
unless overridden.  It seems to violate an abstraction barrier for
at least some of the serialization formats, and prevents snippets
from being composable and commutable absent the
resolution/normalization process.  It does not seem like the markup
language and the document contain suffient safeguards against misuse
to prevent security holes (both sensor data and commands could be
misinterpreted).  It seems that some substantially expanded text
should be added on the hazards of the non-resolved format and giving
guidance on when resolution/normalization must be performed in order
to avoid correctness and security issues.

There also seem to be sizeable risks associated with the semantics
for time values.  In particular, both with the use of an
implicit-"now-ish", and with positive and negative values being
interpreted with respect to a different absolute time base.  (The
involvement of base time is a further complication -- I do not
remember any discussion of the interaction of a (positive) base time
and a negative regular time, for example.  I also do not remember
any discussion of how the "now-ish" semantics make it actively
harmful to do things like store-and-forward or archive SenML data
(again, absent normalization), or what sort of granularity the
"now-ish" semantics are expected to adhere to.  (Is "yesterday"
still considered "roughly now"?)  I understand the desire for this
sort of semantics, but the current specification seems to leave many
potential problems exposed.

COMMENT

Section 4.4

Just "Considerations" is an unusual subject title.

Having no Unit and no Base Unit is allowed, but you don't say what
the semantics are in that case (presumably just a dimensionless
counter for integers, with units not really being applicable to
non-numeric types).  Interestingly, Section 5.1.7 deems it fit to
use "/" for the unit for a boolean value, even though "/" is
supposed to be a (continuous/floating-point) ratio.


Section 4.5

Just to double-check: you really do want to privilege this
specification's version for eternity, for the purpose of being
omitted from resolved records?


Section 12.1 is there not some other units registry we can use?  I
fear begetting https://xkcd.com/927/  .  Also, how is/should the
table be sorted?


Also in Section 12.1, number 9, is the need for case sensitivity in
Units (or otherwise?) normatively covered anywhere?  If not, should
it?



Section 12

Are Base fields supposed to get negative CBOR labels
(and not other fields)?  Is this mentioned explicitly somewhere?
(Yes, I know that the intent is for no more CBOR lablels to be
allocated, but that is only a SHOULD-level requirement.)

In
   Extensions that are mandatory to understand to correctly process the
   Pack MUST have a label name that ends with the '_' character.
should that say something about "mandatory to understand but not
defined in this document"?  (Also in 12.3.1 et seq?)


Section 13

Why are we talking about "executable content" at all?  It seems
quite unrelated to the rest of the document.