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

[Ken Murchison <[e-mail]>]

Hi Ben,

Thanks for another thorough review.  Comments/questions below, and I 
will create and post -07 in the morning.


On 3/2/21 6:27 PM, Benjamin Kaduk wrote:
> 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.


Yes, you are correct that the new ABNF is no longer equivalent. Do you 
feel that we need to state that the new extendable syntax is similar but 
non-equivalent to RFC5545 in the text, or was your initial question just 
for your own clarification?


> 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".


Yes, this was a bad choice of words.  I will change it to "update the 
ACKNOWLEDGED" property.


> Section 7.2
>
> Should the DTSTAMP of the VEVENT change when the snooze events occur?


Yes, it should.  This was lazy cut-n-paste of my telemetry, which also 
accounts for the mistake which you mention below.


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


Good catch.  I will update.


>
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
>
> 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
>>
-- 
Kenneth Murchison
Senior Software Developer
Fastmail US LLC