Hi Christian, thank you for these extensive comments. The collected changes are in https://github.com/ietf-wg-asdf/SDF/pull/184 …which is planned to be part of the upcoming -22 submission. Details below. On 2025-03-01, at 13:42, Christian Amsüss <christian@xxxxxxxxxxx> wrote: > > Reviewer: Christian Amsüss > Review result: Almost Ready > > > My review was focused on IoTdir topics (as it was requested through that > channel). Those look largely fine, there's a bit of by-catch. > > As a whole, the document is well structured and readable in a cover-to-cover > way (where any questions that would be excessive to treat at their introduction > are clarified later). > > There are some areas that, despite being editorially sounding good, mismatch > what I understand to be IETF best practices, namely around their use of URIs > and internationalization. > > IoT specific comments > --------------------- > > * WoT interactions: Many concepts described here overlap with the W3C Web of > Things specification (events, actions, properties). That is to be expected > working in the same space, and AIU, interactions with WoT as one model to > convert to and from have been explored and discussed. > > Nonetheless, a reader coming from an IoT background will likely be familiar > with WoT, and their perception of whether this is a document they can work > with could easily be improved by referencing WoT (for example in 2.2.1 where > three other thing desscription methods are listed as aligned, possibly > together with them in the introduction, clarifying their relation; listing > the ecosystems for which the sdf-mapping draft provides mappings could serve > the same purpose). > > This might also work against receiving copies of https://xkcd.com/927/. I have received some pushback for extending the historical/“how did we get where we are” part in other places, so I only put in a reference to WoT into 2.2.1. fa692d6 > * The IETF ecosystem already describes SenML (RFC8428), which to some extent > exposes properties (which are self-described in terms of their units). Any > referencable mapping effort to tie this back in would help make the IETF > specification shape more well-rounded. SenML is a standard generic data model for data-in-flight, SDF is a meta-model for capturing interaction models. SDF makes use of SenML’s unit registries [4.7], but otherwise these two standards are solving sufficiently different problems. (There is a T2TRG activity looking at SenML and the interaction modeling language YANG; although that is quite nascent at the moment, the subject is alive.) [4.7]: https://www.ietf.org/archive/id/draft-ietf-asdf-sdf-21.html#table-4 > * 7.3: This is not perfectly clear as to what is supposed to be escaped; 'is > required for any characters in unit names as required by ABNF rule "pchar"' > can be read as either > > 1. "The URI syntax makes requirements on this, and thus some have to be > escaped (and pchar is part of those rules)" -- that'd mean that there is a > <urn:ietf:params:unit:/>. > > 2. "The names need to be in pchar" -- that'd mean that there is a > <urn:ietf:params:unit:%2f>. > > I'd think (and hope) you mean the 2., Indeed. > also because it works way better with > core-href (maybe even perfectly), and because that slash does not really > delimit path components, and because it does not pull in a rule that is not > *precisely* what is meant, but the absence of "/" in the "specifically" > section would indicate you might mean 1. > > If it is 2, text could instead say: > >> Index value: Percent-encoding (...) is required of any characters in unit >> names except unreserved, sub-delims, ":" or "@" (i.e., the result >> must match the pchar rule). > > (Excluding the pct-encoded from the wording ensures that the unit measuring > physique in similarity to fire fighters, "%FF", doesn't pass through to stay > %ff, but becomes %25FF). 5201ea9 > General standardization practice > -------------------------------- > > * I see why internationalization of given names is not done (they are more like > URIs and not just designed for human consumption), but many text strings in > the information block, and later in the common qualities, are designed for > human consumption (title, description); Harald mentioned he'd come back to > this in the ART review. This has previously been addressed in #180 (specifically 2136ff3). > * Extension feature names will need to be non-conflicting, but no registry or > format is set up for them. Some ways of achieving this is to require them to > be URI shaped, having an IANA registry, or both (as is done for link > relations). Good point. Added a registry (with URI escape) in ae97655 > * "it is probably wise to" is a neat way to avoid RFC6919 language, but however > accurately that document captures our feelings, apparently, more is expected > of standards track documents. > > So, SHOULD Given Names with such encoding be avoided, or SHOULD (or even > MUST) implementations support this encoding (BUT WE KNOW THEY WON'T)? The text is deliberately open with respect to the choice a model makes of the given names (note that given names are given by the designer of an SDF model, who may be describing, e.g., affordances that make sense in specific cultures). There may be many reasons to use given names that do require percent encodings. There is a common expectation that the URI and JSON Pointer implementations being used for SDF do support URIs and JSON Pointers in full, so we didn’t see a reason to put a specific requirement there beyond the inclusion of an example that should make the requirement obvious. > Names and URIs > ~~~~~~~~~~~~~~ > > * 4. Names and namespaces > > Nothing about using JSON paths to contribute to the default prefix is > formally wrong (just somewhat unclear, eg. 3.2 talking of "local"), but while > from a high-level perspective it looks like it would work just by virtue of > having the JSON path fragment interpretation of the media type and using URI > semantics, it just appears to do that and really does something else: Indeed, the RFC 3986 URI Reference resolution is not used. Instead, SDF uses string concatenation to build URIs to virtual representations. > (I'm ignoring the full CURIEs here -- those are well understood to just be > applied as text substitutions; this is about the references to the local > document and the default namespace.) > > The sdfRef values look like they are URI references, They are not — they are just the right hand side to a concatenation (to an unnamed or globally named virtual representation). > and they even work that > way for referencing items defined in the current document (because there the > media type uses JSON paths), but not for referencing items contributed to the > default namespace by other documents. The latter *would* work if defining a > default namespace established a base URI embedded in the content per Section > 5.1.1 of RFC3986 -- but this document does not do that. Declaring the default > namespace an embedded URI would seemingly break the ability to reference > local elements (just like in HTML, <base href="somewhere" /> breaks <a > href="#foo">), but that is saved by virtue of all local elements being > contributed to the default namespace (and thus once more available after > setting a base URI). > > Note that none of this works because the *current* document's media type > imports JSON Pointer fragment identifiers: The requirement is really on the > default document to have those semantics, and that they match the local > document after merging. (Conveniently, JSON merge patch retains that). > > > The document as it is runs through a mix of using URIs a bit, sometimes using > URI mechanisms, then again talking of strings that "look like" a URI or have > a fragment identifier "attached". This is confusing, and deviates from the > established practice of using URIs as data format components. (It doesn't > help that sections like 4.1 repeat requirements of URIs, eg. that an absolute > URI is a URI with a hier-part -- every URI is a URI and has a hier-part, and > in https that can not even be empty). SDF here really needs to act like a linker in software generation: All models that contribute to a namespace together create a single virtual representation to which the fragment identifier syntax is then applied. The URIs cannot be directly referenceable because new models can start contributing to the namespace as well (e.g., by an SDO that adds a model to its namespace). A tool that resolves these references needs to support maintaining a collection of contributing models for each namespace; the way that this is managed is outside the scope of the SDF definition. > * Relatedly (but that is not the problem of *this* document), the way local > references are used will also make it hard for deref-id to be used, for the > JSON-path-style identifiers will need to work with the representation there. > That will require that the dereferenced document is either a union > representation of all SDF documents that contribute to a namespace, or to > have some index format based on explicit links. That document could then > create explicit links from some fragment references to the documents that > really define them: > > <./extension-foo-v2025.sdf.json>;rel=see-also;anchor="./#/sdfData/extItem1", > <./extension-foo-v2025.sdf.json>;rel=see-also;anchor="./#/sdfData/extItem2", > <./index-v2025.sdf.json>;rel=see-also;anchor="./#/sdfData/switch" > > * 3.2 has concrete fall-out of this: delimiting by the fragment is more than > just a convention here, it is a practical requirement at least for the > default namespace, because names are only contributed by the document through > fragment encoded JSON paths that necessarily replace the fragment. There is no URI resolution intended, so I’m not sure this is really true. > * sdfRequired introduces a short-hand notation (referenceable-name) for what is > otherwise found in sdfRef. If(!) references are already not really URI > references but just look like them, why not do that also for sdfRef? (in > figure 4, currentTemperature could "sdfRef":"temperatureData" by the same > rules). Syntactically, the value of an sdfRef already is an sdf-pointer. Semantically, there is a problem: `true` does not make sense for sdfRef, and a `referenceable-name` might be ambiguous (»All affordance declarations that are directly in the same grouping (i.e., not nested further in another grouping) and that carry this name« works for sdfRequired, because *all* matches are made required, but does not work very well for sdfRef). > * 4.7 note 1: URIs in lieu of SenML units sound sensible enough to warrant a > note to the SenML DEs to not register any new units containing a colon; an > update to the registry might be considered. Would that prohibit the registration of names with colons or just point out that they should be avoided? In the latter case, we might need the escape hatch anyway, so I’d simply provide that. > Alternatively, if we do *not* want keep that registry free of colons, making > an exception of "URNs from this sub-namespace MUST NOT be used in a unit > quality" would make sense for exactly those units as an escape hatch. Added the escape hatch, added IANA consideration to add note. be608d8 > But I think we should make a choice before this document goes out. > > > Editorial comments > ------------------ > > * Terminology: Three Conceptual Terms are defined using the term Grouping, even > though this is a Specfication Language Term. If they are really conceptual, > the term could be replaced with "set of affordances" here (or "affordance (or > set thereof)"). I didn’t know better than to add a forward reference. 120b0f0 > * "when triggered, have more effect than just reading, updating, or observing > Thing state": Yet that is precisely what the actions in the example do. Maybe > "have an effect that can exceed just reading, updating or observing Thing > state". Nice. (Used “go beyond” instead of “exceed”, which sounds a bit like a total order.) 022d928 > > * "qualities of idempotence and side-effect safety": I suppose those are > planned but not specified here; any informative refrence to where they are > planned? Oh. This is using quality in the English sense. Changing to “characteristic” (9110 “method properties” don’t really fit either). (There might be “SDF qualities” to indicate those as well, they are conceivable but not actually planned at this time.) b61c9d1 > * 6.1: The example ns:/sdfObject violates the convention set up in 3.2 Good catch. bdc587b > * 7.4.1 Instructions to the experts are not rendered as a list. Already fixed earlier in 89ac71c > * 7.4.1 "pattern" could use a reference into C.2 so that its consumers know > which regexp language is being referred to. They know that from the first sentence of “Conventions” in 1.2. > * 7.4.1 maxLength, minLength could use a reference to C.2, lest people keep > asking about what is meant by "character". Frankly, I'd use "measured in > Unicode scalar values" everywhere; "character" just misleads people to think > they know what it means, and if they really do, they understand whata Unicode > scalar value is. Good point. b82f500 > Curious questions > ----------------- > > * Would a switch actually be modelled to have a boolean property "value", vs. an > "on"/"off" enum? (The example in D.2 is clearer in that it says "powered", > which is more clearly a yes/no question). This is a constructed example, avoiding the need to explain enums right in the overview. (Of course you would use an enum, to integrate 50 % on, transitions, etc.) Now, do you really want to know the answer to this? I did warn you… https://github.com/one-data-model/playground/tree/master/sdfObject/sdfobject-on_off_switch.sdf.json Thank you again for this great review! Grüße, Carsten -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
