several issues (bugs?) with `Fit4Ruby::Session#check()`

[pvdb]

There appear to be several issues with Fit4Ruby::Session#check():

    # Perform some basic consistency and logical checks on the object. Errors
    # are reported via the Log object.
    def check(activity)
      unless @first_lap_index
        Log.fatal 'first_lap_index is not set'
      end
      unless @num_laps
        Log.fatal 'num_laps is not set'
      end
      @first_lap_index.upto(@first_lap_index - @num_laps) do |i|
        if (lap = activity.lap[i])
          @laps << lap
        else
          Log.fatal "Session references lap #{i} which is not contained in "
                    "the FIT file."
        end
      end
    end

• as reported on this PR not every valid FIT file specifies @first_lap_index and @num_laps for every Fit4Ruby::Session data record, but the above method assumes it does, and raises a fatal error if one or both are missing, which in turns fails to load/process an otherwise perfectly valid FIT file

• certain activity types result in FIT files with just one session per activity, and just one lap per session, in which case @first_lap_index == 0 and @num_laps == 1, which means that:

      @first_lap_index.upto(@first_lap_index - @num_laps)

... is effectively an empty enumerator:

    0.upto(-1)

... meaning that the nested checks are never actually executed 😱

I'm not quite certain what the expected behaviour is supposed to be, but AFAICT something like this would make a lot more sense:

-      @first_lap_index.upto(@first_lap_index - @num_laps) do |i|
+      @first_lap_index.upto(@num_laps - @first_lap_index - 1) do |i|

• and it's probably a good thing that the nested checks are never executed, because it looks like they have been implemented incorrectly to boot, as fixing the above issue then subsequently results in a NoMethodError:

undefined method `lap' for an instance of Fit4Ruby::Activity

It looks like the following logic was intended:

-        if (lap = activity.lap[i])
+        if (lap = activity.laps[i])

• furthermore, even if the enumerator weren't empty, then the logic is questionable, I think, as it appears to copy laps from the activity down to the session, even though the laps are already set as part of the Session initialiser:

        if (lap = activity.laps[i])
          @laps << lap
        else
          Log.fatal "Session references lap #{i} which is not contained in the FIT file."
        end

Why are the laps copied down from the activity to the session a second time?

---

Documenting my findings as an issue first, instead of submitting a PR based on assumptions I'm making about the correct behaviour ... individual PRs for each of the above issues coming your way separately, @scrapper ! 😃