Re: [calsify] Benjamin Kaduk's Discuss on draft-ietf-calext-valarm-extensions-05: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

Hi Ken,

Thanks for the quick turnaround!
The parent/sibling and proximity rewrites came out well, which is good to
see.  I do have a few more remarks on the -06, most notably on whether the
"alternative but equivalent form [of the ABNF syntax]" claim continues to
hold with the addition of "alarm-subcomp".  (I understand that we need to
allow subcomponents in order to have the VLOCATION components present, but
we seem to be "sneaking it in" at the moment.)

I'll post my comments in the datatracker for archival purposes, but rather
than have it send a separate mail I'll just post them here so they're
accessible:

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Thanks for responding to my previous question to clarify that the intent
is to provide an equivalent form for the alarm ABNF to what is in RFC
5545.  (I personally am not prepared to assert that it definitely is/was
equivalent, since I only looked closely enough to ascertain that it is
at least very similar.  I would prefer if we had some independent
verification of that, but I don't even know what form that verification
would take, so I can hardly insist upon it.)

That said, in making the changes to adapt to VLOCATION, we had to add
the "alarm-subcomp" production that includes x-comp / iana-comp, and
it's pretty hard for me to claim that with alarm-subcomp in place, the
ABNF remains "equivalent" to the RFC 5545 form.  So maybe we have to
revisit that claim, or leave the initial alarm-subcomp as an empty
production and make some additional explicit extension where we allow
subcomponents, or something.

Section 7

   3.  When the "snooze" alarm is triggered, the client MUST do the
       following:

       A.  Reset the "ACKNOWLEDGED" property on the original related
           alarm.

(nit) I think that "clear" or "remove" would probably be more clear than
"reset".

Section 7.2

Should the DTSTAMP of the VEVENT change when the snooze events occur?

Also, IIUC, the ACKNOWLEDGED time for the primary alarm
(8297C37D-BA2D-4476-91AE-C1EAA364F8E1) should be something after
20210302T152000Z after the second snooze event.  (It currently shows as the
same 20210302T151514Z from after the first snooze.)

It's also a little surprising that the final acknowledgment occurs after the
meeting starts, some 6 minutes after the alarm triggered, but that's not
actually a protocol error, so it's technically okay.

Section 8

      "PROXIMITY" property - indicates that a location based trigger is
      to be used and which direction of motion is used for the trigger

It looks like we got rid of the "direction of motion" reference later on
(where I had actually commented on it), but this one should go as well.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Thanks,

Ben

On Tue, Mar 02, 2021 at 03:16:54PM -0500, Ken Murchison wrote:
> Hi Benjamin,
> 
> Thanks for the detailed review.  Responses below, but you'll want to 
> look at the new text in -06.
> 
> 
> On 2/24/21 8:25 PM, Benjamin Kaduk via Datatracker wrote:
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-calext-valarm-extensions-05: Discuss
> >
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut this
> > introductory paragraph, however.)
> >
> >
> > Please refer tohttps://www.ietf.org/iesg/statement/discuss-criteria.html
> > for more information about IESG DISCUSS and COMMENT positions.
> >
> >
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-calext-valarm-extensions/
> >
> >
> >
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> >
> > Please update to reflect the changes made in draft-ietf-calext-eventpub-extensions-17
> > (e.g., there is no longer STRUCTURED-LOCATION and VLOCATION plays a similar
> > role).
> 
> 
> Fixed, with associated extension to the ABNF and corrected example.
> 
> 
> 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> > [edited to add a comment section, now that I finished my way through
> > the document]
> >
> > A lot of these improvements look familiar from
> > draft-ietf-calext-jscalendar :)
> >
> > Please note the question about interactions between "geo:" URIs and
> > (presumed mobile) vehicles, in Section 8; that was almost a Discuss.
> >
> > Section 3
> >
> > Is the new syntax for VALARM claimed to be exactly equivalent or just
> > "basically the same" as the RFC 5545 definition?
> 
> 
> The new syntax is intended to be equivalent.  I've augmented the text to 
> state that.
> 
> 
> 
> > Section 6.1
> >
> >     Description:  This property is used to specify when an alarm was last
> >        sent or acknowledged.  This allows clients to determine when a
> >
> > (editorial) the "last sent" part only becomes clear in the second
> > paragraph; perhaps "when an alarm was last acknowledge (or sent, if
> > acknowledgment is not possible)" would help clarify (and would also do
> > better to set up the rest of this paragraph that mostly assumes the
> > "acknowledged" case).
> 
> 
> Clarified using your suggested rewording.
> 
> 
> 
> > Section 7
> >
> >     To "snooze" an alarm, clients create a new "VALARM" component within
> >     the parent component of the "VALARM" that was triggered and is being
> >     "snoozed" (i.e., as a "sibling" component of the "VALARM" being
> >     snoozed).  [...]
> >
> > The way this is written seems to imply something that does not seem
> > correct.  That is, the "i.e., as a 'sibling'" seems to imply that VALARM
> > will always exist in a parent/child relationship, with only "child"
> > instances ever actually triggering.  Since we only just in this document
> > allow VALARM to have the RELATED-TO property, I don't see how that could
> > be the case.  Perhaps it is enough to just say "e.g., as a "sibling"
> > component of the "VALARM" being snoozed", since "e.g." does not imply
> > that the statement applies for all cases, but I would expect some more
> > substantial discussion of the procedures involved when there is just a
> > single alarm being snoozed, and how the first child is created, what
> > needs to be done (if anything else) to create the overarching "parent",
> > etc.  The only text I see right now that covers this case is the
> > addition of "UID" if not already present (and UID has to be present if
> > there's a parent/child relationship), but that's not really the same
> > thing.
> >
> >     Alternatively, if the "snooze" alarm is itself "snoozed", the client
> >     MUST remove the original "snooze" alarm and create a new one, with
> >     the appropriate trigger time and relationship set.
> >
> > (This part seems a bit at odds with the "as a 'sibling' text above that
> > I complained about -- if the alarm being triggered is getting removed,
> > it seems hard for it to be a sibling of anything.)
> >
> > Section 7.1
> >
> >     This specification adds the "SNOOZE" relationship type for use with
> >     the "RELTYPE" property defined in Section 3.2.15 of [RFC5545].  This
> >     is used when relating a "snoozed" "VALARM" component to the original
> >     alarm that the "snooze" was generated for.
> >
> > Is this going to be the "parent" or the "sibling" (or potentially
> > either)?  Note my previous comment about "sibling"s getting removed...
> 
> 
> I think the parent/child/sibling language wasn't quite clear.  I 
> reorganized the snooze process as a series of steps and tried to clarify 
> the parent/child relationship with an example.
> 
> 
> > Section 8
> >
> >        "STRUCTURED-LOCATION" [I-D.ietf-calext-eventpub-extensions] - used
> >        to indicate the actual location to trigger off, specified using a
> >        geo: URI [RFC5870] which allows for two or three coordinate values
> >        with an optional uncertainty
> >
> > (nit?) maybe "trigger off of"?
> >
> >        "STRUCTURED-LOCATION" [I-D.ietf-calext-eventpub-extensions] - used
> >        to indicate the actual location to trigger off, specified using a
> >        geo: URI [RFC5870] which allows for two or three coordinate values
> >        with an optional uncertainty
> >
> > How do I use a geo: URI to indicate the vehicle to which I (dis)connect
> > via Bluetooth with?
> >
> > Section 8.1
> >
> > (side note) is there work underway to define proximity triggers for
> > JSCalendar?
> >
> >     Description:  This property is used to indicate that an alarm has a
> >        location-based trigger.  Its value identifies the direction of
> >        motion used to trigger the alarm.  One or more location values
> >        MUST be set using "STRUCTURED-LOCATION" properties.
> >
> > (editorial) I don't see how we get "direction of motion" from the rest
> > of the description.  What I see described is just, well, a
> > proximity-based trigger, without sensitivity to in what direction the
> > proximity is attained.  There are triggers based on whether the distance
> > metric crosses a threshold in one direction or the other, but that's
> > still not directional in a typical 3-dimentional vector sense.
> 
> 
> Sections 8 and 8.1 have been rewritten in -06 to hopefully be more clear.
> 
> 
> > Section 8.2
> >
> > Is there a reason for the "TRIGGER" line to appear twice in the example?
> 
> 
> No reason other than a mistake.  Corrected.
> 
> 
> > Section 9
> >
> > I think there is also potential for abuse in causing embarassing or
> > otherwise undesired alerts (especially audio ones) when a victim is in a
> > particular location.  (But the mitigation is basically the same as for
> > the already-indicated threats.)
> 
> 
> Augmented the text remove the word "spam" and to include embarrassment 
> as a possible outcome.
> 
> 
> > Section 13
> >
> > It's not entirely clear to me that RFC 4791 needs to be classified as
> > normative.
> 
> 
> Moved to Informational.
> 
> -- 
> Kenneth Murchison
> Senior Software Developer
> Fastmail US LLC
>