Acknowledged
6 months ago

Add onTimerLapUndo() event to signify that the previous onTimerLap() event was undone

Background:

- newer watches give the user the ability to undo a manual lap by pressing the Down button while the lap banner is displayed (for 8 seconds)

Issue:

- any CIQ data field that tracks lap data will be negatively affected if and when a user undoes a lap, because apps aren't notified of this event

- this will cause CIQ data fields to calculate, display and write incorrect lap data following an undone lap

Request for CIQ changes:

- add onTimerLapUndo() event to the CIQ API, signifying that the previous lap event was undone

- add onTimerLapFinalized() event (for convenience) to the CIQ API, signifying that it's no longer possible to undo the previous lap event

- document what devs needs to do to handle lap undo properly (see next section)

What I think devs of CIQ data field apps would need to do (please correct me if I'm missing something):

- constantly maintain *two* sets of lap data at the same time: S_current_lap (the current lap data as if the current lap will not be undone), and S_previous_lap (the previous lap data, as if the current lap will be undone)

- when calculating and displaying lap data, or calling FitContributor.Field.setData() on a field of MESG_TYPE_LAP, continue to use values based on the apps *current conception* of the correct lap values. i.e. as long as the current lap is not undone, use S_current_lap. If and when the current lap is undone, copy/assign S_previous_lap to S_current_lap

(note that CIQ data fields need to be constantly calling setData() for fields of MESG_TYPE_LAP, as in the MoxyField SDK sample, since it's too late to call setData() in onTimerLap()).

- if onTimerLapFinalized() is implemented, this frees the dev from the burden of constantly maintaining two sets of lap data (they would only have to do so for up to the first 8 seconds of every lap). This would save battery life / CPU cycles, and possibly allow additional functionality due to having additional memory to spare (depends on whether that functionality can be restricted while a lap can be undone)

For example, assuming that the first activity lap is Lap 1, when onTimerLap() is triggered for the first time, the app now has to track/calculate data for both Lap 2 (the new lap) and Lap 1 (as if Lap 2 never existed). As long as Lap 2 is not undone, the data for Lap 2 should be displayed where applicable, and passed to FitContributor.Field.setData() for applicable MESG_TYPE_LAP fields. During this time, data for Lap 1 should still be tracked/calculated. If and when Lap 2 is undone, the app should start using the data for Lap 1 again (as if Lap 2 never existed).

Related discussion:

https://forums.garmin.com/developer/connect-iq/f/discussion/403749/what-event-is-triggered-when-a-lap-is-discarded/1899120#1899120

flowstate
  • 6 months ago

    Also, the lap undo feature is fairly new, but not *that* new.

    If the CIQ team wanted to delay onTimerLap(), they could've done so already.

    I submit that they didn't do so because it's absolutely the wrong thing to do. Instead of leaving well enough alone (where CIQ DF lap data works properly as long as a lap is never undone), it would break CIQ DF lap data whether or not a lap is undone or not.

  • 6 months ago

    > With the lap undo feature, the period of time between pressing the LAP button and the point where it's no longer possible to undo is a *maximum* of 8 seconds. It could be shorter.

    To be clear, the period of time between taking a manual lap and the point where it's no longer possible to undo the lap could be shorter than 8 seconds (the max length of time the lap banner will be displayed) bc the user can dismiss the lap banner prematurely (by pressing UP).

  • 6 months ago

    > delaying onTimerLap has far less impact than if every CIQ DF that uses lap data needs to change. 

    Hard disagree. If onTimerLap() is delayed, then every CIQ DF that uses lap data needs to change regardless of whether the dev cares about supporting lap undo or not. With my proposal, CIQ DFs only need to change if the dev cares about supporting lap undo - if the dev only cares about the case where laps are not undone, nothing needs to change.

    Situation 1) Delay onTimerLap

    Delaying onTimerLap breaks every CIQ data field, whether or not users actually use lap undo, and there's no logical/simple way for the CIQ API to help the dev recover. Even if a CIQ field was rewritten to track 2 sets of laps (so it can retroactively fix the data), how would it know how/when to do that? With the lap undo feature, the period of time between pressing the LAP button and the point where it's no longer possible to undo is a *maximum* of 8 seconds. It could be shorter.

    Sure, it's conceivable that a CIQ data field could recover from this situation if:

    - they kept two sets of lap data all the time (but this is already the suggested fix, without the onTimerLap delay)

    - the CIQ API added an onTimerLapFinalized() event, with additional data about how long the delay was between the actual beginning of the lap and onTimerLap()

    But now when a lap is *not* undone, the app has to go back and reconstruct the current lap data

    Situation 2) The current state of affairs, where onTimerLap is not delayed and there is no onTimerLapUndo() event. This only breaks the use case where a user actually uses lap undo.

    Situation 3) My proposed state of affairs, where onTimerLap is not delayed and there is an onTimerLapUndo() event. This is identical to 2), except developers can opt in to handle the case where users actually use lap undo.

    Basically Situation 1) (delaying onTimerLap) breaks both use cases (lap is not undone, which is by far the most common use case, and lap is undone), and it forces to the dev to do a lot of work to fix the common use case.

    Situation 2), the current state of affairs, only breaks the use case where the lap is undone. The case where the lap is not undone works just fine

    Situation 3), the proposed state of affairs, is just like 2), except the dev can opt in to handle undoing laps properly. Yes it's a lot of work, but it's opt-in.

  • 6 months ago in reply to flowstate

    So native and fit contrib data is fine, and delaying onTimerLap has far less impact than if every CIQ DF that uses lap data needs to change. 

  • 6 months ago

    > And fit cotrib data for laps wasn't written to the fit file until that time too.

    This was also covered in the linked discussion.

    FIT contrib data is not a problem, as it can be handled in exactly the same way that native data is handled. Keep in mind that setData() does not cause data to be written, it only sets the next value to be written.

    I can think of two ways that the writing of native lap data can be handled in a way that works with undoing laps:

    1) Lap data is always written immediately (for either auto or manual laps - only manual laps can be undone, though). If the user undoes a manual lap, the firmware writes a marker to the FIT file telling the reader (e.g. Connect) to ignore the previous lap

    2) Lap data writes are delayed with an 8-second buffer in the case of a manual lap). (Presumably record data would have be delayed as well). If the user doesn't undo the lap after 8 seconds, then the lap data is finally written. If the user does undo the lap, then the lap data is not written (but the other data would be written).

    Personally I think 1) would be a better choice from a data integrity POV (so if the activity crashes, you don't run the risk of losing 8 seconds of data, although arguably 8 seconds is not a huge amount to lose). But whether the real implementation is 1) or 2), CIQ data field apps absolutely do not need to care, imo.

    Currently, for MESG_TYPE_LAP fields, CIQ data fields should be continuously calling setData() with the current lap value, just as in the moxyfield sample. It's well known that it's too late to call setData at onTimerLap() (although even if this were possible, it wouldn't change the situation with regards to handling lap undo).

    CIQ data fields can simply continue to call setData() continously with their conception of the current lap value - that part doesn't need to change at all, regardless of how the actual FIT field writes are implemented in firmware.

    The only thing devs would need to change would be to keep track of two sets of lap data (current and previous) during the time in which a lap can be undone, and to switch back to the previous lap data if and when the most recent lap event is undone.