Add data types for messages

[robinkrahl]

Thank you for this crate! It really takes the pain out of parsing my Garmin logs. 👍

I think it could be even more useful if it would contain structs for the message types, with each message field being a field of the struct. As you are already parsing the profile in build.rs and auto-generating code from it, I think this should not be too complicated. For example, a representation of an Activity message could look like this:

pub struct Activity {
    pub total_timer_time: Option<f64>,
    pub num_sessions: Option<u16>,
    pub r#type: Option<fitparser::profile::field_type::Activity>,
    // ....
}

impl TryFrom<fitparser::FitDataRecord> for Activity {
    // ...
}

This would make it much easier to use the crate and also to discover the relevant message types and fields from the API docs. What are your thoughts on this?

[robinkrahl]

And with some manual effort, it would even be possible to define a mapping of the units used in the default profile to uom types and units to get something like:

pub struct Activity {
    pub total_timer_time: Option<uom::si::f64::Time>,
    pub num_sessions: Option<u16>,
    pub r#type: Option<fitparser::profile::field_type::Activity>,
    // ....
}

[stadelmanma]

Hey @robinkrahl I am glad this crate has been helpful to you! I use it fairly regularly myself in a run-tracking CLI app I built awhile back.

I’ve thought about adding structs for the messages in the past but never have since this library simply decodes FIT messages into a generic “data transfer object” format. I like the idea of the messages themselves being better documented via the API docs alone. And it would be fairly straightforward to convert back and forth between the generic DTO struct and these new types.

I try to be cognizant of the size of my dependency chain so I’ll have to do some looking into uom and weigh pros/cons. It might be more beneficial to avoid adding a new dependency and instead setup a way that unit info can be easily ingested into uom for the folks that want that added level of detail. (Although I have spent some time hacking around the units myself so maybe it would be worth it.)

How else do you think adding specific types to the crate would make thing easier? I’d be curious to hear some other use cases!

[robinkrahl]

FYI, I’ve started working on an implementation an will hopefully be able to share that soon.

How else do you think adding specific types to the crate would make thing easier?

My use case is analyzing cycling logs. I ended up working on a separate crate that contains structs for the FIT messages and some utilities for parsing. All this code could be automatically generated from the profile. If I was able to directly access the data structures, I wouldn’t have to go back at the profile sheet and look for the messages and fields I want to use, and I would not have to manually write the parsing and conversion code.

[stadelmanma]

@robinkrahl great, I look forward to seeing it! Also I bumped the bundled FIT SDK version to 21.89 which is current as of today to help minimize the diff from your new code.

[robinkrahl]

If you want to avoid having uom as a required dependency, we could also introduce wrapper types for the quantities and add conversions to uom types behind a feature flag:

pub struct Activity {
    pub total_timer_time: Option<Time>,
}

pub struct Time(pub f64);

#[cfg(feature = "uom")]
impl From<Time> for uom::si::f64::Time {
    fn from(time: Time) -> Self {
        // ...
    }
}

[robinkrahl]

#14 implemented types with Option<Value> fields. The next step should be to include the data type information. Unfortunately, it looks like most real-world files don’t follow the data types defined in the profiles, so we can’t just return an error for mismatches. We could discard everything that does not match the profile, or add a wrapper type that makes it easy to access the specified data type but also provides values that don’t follow the specification.

I’m not sure what I prefer. On the one hand, I’d like the implementation to be as generic as possible. On the other hand, the main goal of these types is improved ergonomics so it should also be as simple as possible.

[stadelmanma]

That is an unfortunate consequence of the FIT profile being modifiable, although I never actually considered it's effect on this work until now. A couple thoughts:

1. I think I lean towards concrete types for these struct members, either of the form Option<T> or an option-like wrapper that has a third state for Invalid(Value). I'm not sold on the benefits of the three-state idea and right now I like the plain Option<T> route better, largely because Option is a well known, common feature of Rust code.
2. I'm not opposed to performing type coercion, e.g. the FIT files for someone's watch record the data as u8 but the profile defines it as u16. That's an easy adjustment, going the other direction is more of an issue.
3. If we want to have concrete types, what do we want to do with the data that "doesn't fit"? Store it in a hash map to allow lookup? Add a function to disambiguate a true "None" as in truly "not-present" vs an invalid data type.
4. I wonder if there's a good method to allow a user provided struct to "override" something that doesn't line up well with the FIT profile their chosen device implemented?

[robinkrahl]

After giving it some more thought and starting the implementation, I think the best approach would be to keep the fields as Option<_> and only set them if the value is valid. Invalid values are stored in invalid_fields: Vec<FitDataField>. This would mean that access is easy if the field data is valid, and users can still handle special cases manually.