Implement handling of invalid values in arrays

[stadelmanma]

Retaining invalid values in arrays ensures we don't lose length or position information in the parsed data. Non-array invalid values are still stripped from the output.

JSON rendition of a HRV message with invalid values:

  {
    "kind": "hrv",
    "fields": {
      "time": {
        "value": [
          0.467,
          0.464,
          null,
          null,
          null
        ],
        "units": "s"
      }
    }
  }

Closes #45

[stadelmanma]

@mat-kie can you give this PR a look over? I started with your tweaks and then added some logic to retain invalid values in the array so we don't lose length or position info.

[mat-kie]

I have looked at the Fit specification again and I think there needs to be a special handling for arrays of Byte values.

I openend another PR targeting this branch where I addressed my remarks and closed my old one.

[mat-kie]

Consider removing this comment since this should be resolved with this PR

[mat-kie]

I think there is a hypothetical issue here. Since values are abstracted as enum variants, we could in theory create a Value::Array which contains values of differing types. If those values would be valid, this code would return True. However an array of values should never contain values of differing types.

As long as the parsing code is well-behaved this case should never appear but if this should be checked I would propose something like this:

 if let Some(first) = vals.first().map(|v|discriminant(v)) {
    vals.iter().all(|x|matiches!(x, Value::Invalid) || discriminant(x) == first) && vals.iter().any(|v| v.is_valid())
 } else {
    false
}

or a bit less concise but without double iteration:

if let Some(first) = vals.first().map(|v| discriminant(v)) {
    let mut same_type = true;
    let mut any_valid = false;
    for v in vals{
        same_type &= matiches!(x, Value::Invalid) ||  discriminant(v) == first;
        any_valid |= v.is_valid();
    }
    same_type && any_valid
} else {
    false
}

[stadelmanma]

You are correct, I have that noted in the actual
Value enum as a known edge case for Arrays. It’s not possible to parse a multi-type array directly due to how the FIT spec defines fields in the definition messages.

If FIT file writing ever gets implemented then we’d want to validate array contents somewhere along the way in the serialization process.

[mat-kie]

Looking at the FIT file specification again, I think there might be an Issue here. As I unuderstand it now, the special remark regarding the Byte value and arrays of it addresses the following situation:

For all Value types except the Byte type, value validity for values inside an array is determined on a per value basis. For byte values however, if one value inside the array is considered valid, all should be considered valid.

I think this might be the case, because otherwise not a single byte inside a Byte array filed would be allowed to be 0xFF.

Currently, a byte array [0xFE, 0xFF] would be returned as vec![Byte(0xFE), Invalid], I think what sould be returned in this case (and only for the Byte value type) would be vec![Byte(0xFE), Byte(0XFF)]. However I am not 100% shure about this.

My proposal:

if matches!(base_type, FitBaseType::Byte) || value.is_valid()  {
    values.push(value);
} else {
    values.push(Value::Invalid)
}

[stadelmanma]

You are spot on, great catch on that bug.

[mat-kie]

Consider let mut value = Vec::with_capacity(size as _); in line 574.

[stadelmanma]

Closing in favor of #47