search

grimbough / FITfileR

R package for reading data from FIT files using only native R code, rather than relying on external libraries.
https://msmith.de/FITfileR
50 stars 13 forks source link

tweaks to get some files to parse #35

Closed dblodgett-cycling closed 10 months ago

dblodgett-cycling commented 10 months ago

These are the little tweaks that got me going for #33 and #34 -- can add testing if you have a preference for how to do it -- I'm more familiar with testthat than tinytest.

rawmnr commented 10 months ago

Seems to correct #32 too, thanks for the input 👍

grimbough commented 10 months ago

Thanks for the diagnostics and pull request.

This was an inadvertent bug introduced by changing how the package selected the setting for reading specific datatypes. As you noted, there was a comment about needing refactoring, and this was the case where not doing that came back to bite me as the two sections of code had diverged enough for it to be an issue. Message that contained developer data were broken, but messages that only had standard fields were fine. I've now done that refactoring so only function is used either both cases, rather than duplicating things.

I think the issue in #33 were actually a side effect of this. I'm not sure it should be possible for messages of the same type to end up with different datatypes in the same field. The whole structure of the FIT protocol should mean that messages with the same definition have the same datatypes in the same order. However if we were reading them wrong in the first place them that all goes out of the window.

dblodgett-usgs commented 10 months ago

Good deal! Thanks!