Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fit: Added support for ANT+ FIT format (used by Garmin devices) #863

Merged
merged 5 commits into from
Feb 10, 2024

Conversation

mlofjard
Copy link
Contributor

@mlofjard mlofjard commented Feb 6, 2024

No description provided.

@wader
Copy link
Owner

wader commented Feb 6, 2024

Nice! will have a look tomorrow, but some quick notes:

Some tests and test files would be nice. Any idea if there are any good ones around under liberal license? alternatively is there some good way to produce own test files?

Possible to include the scripts used to generate the tables, even if messy? can also have a README.md under the fit directory etc with instructions how to regenerate etc

@wader wader mentioned this pull request Feb 6, 2024
@mlofjard
Copy link
Contributor Author

mlofjard commented Feb 7, 2024

Yes, I will check the license on the SDK example files, and if they are not liberal enough I will throw in some of my own files. I'll also do some linting/cleanup and put my nodeJS script in the dev-folder, along with some markdown documentation in the fit-folder as suggested.

@wader
Copy link
Owner

wader commented Feb 7, 2024

Yes, I will check the license on the SDK example files, and if they are not liberal enough I will throw in some of my own files.

👍

I'll also do some linting/cleanup and put my nodeJS script in the dev-folder, along with some markdown documentation in the fit-folder as suggested.

I usually try keep all format specific things in format/<name> and if test related like tests, test files or script to generate things keep them in testdata.

btw to add authorship, references, known limitations, help etc you can add a fit.md and include it, ex:
https://github.com/wader/fq/blob/master/format/bson/bson.md
https://github.com/wader/fq/blob/master/format/bson/bson.go#L14-L27
that will then be used for cli help fq -h fit and to generated usage documentation (make doc)

@wader
Copy link
Owner

wader commented Feb 7, 2024

Output looks really good, impressive work 👍 hope i get more time to review tomorrow

format/fit/mappers/values.go Outdated Show resolved Hide resolved
@wader
Copy link
Owner

wader commented Feb 8, 2024

Should have been reviewing the code but ended up play around with it! try this:

go run . -r '{kml: {"@xmlns": "https://www.opengis.net/kml/2.2", Document: {Placemark: {LineString: {coordinates: [.dataRecords[].dataMessage | select(.position_lat and .position_long) | "\(.position_long),\(.position_lat)"] | join("\n")}}}}} | to_xml'  format/fit/testdata/activity.fit > test.kml

@mlofjard
Copy link
Contributor Author

mlofjard commented Feb 8, 2024

Should have been reviewing the code but ended up play around with it! try this:

go run . -r '{kml: {"@xmlns": "https://www.opengis.net/kml/2.2", Document: {Placemark: {LineString: {coordinates: [.dataRecords[].dataMessage | select(.position_lat and .position_long) | "\(.position_long),\(.position_lat)"] | join("\n")}}}}} | to_xml'  format/fit/testdata/activity.fit > test.kml

Yup, that is the route for STHLM Bike 2023 =)

Copy link
Owner

@wader wader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks overall good i think, will continue tomorrow

format/fit/fit.go Outdated Show resolved Hide resolved
format/fit/fit.go Show resolved Hide resolved
format/fit/fit.go Outdated Show resolved Hide resolved
format/fit/fit.go Outdated Show resolved Hide resolved
format/fit/fit.go Outdated Show resolved Hide resolved
format/fit/fit.go Outdated Show resolved Hide resolved
format/fit/mappers/values.go Outdated Show resolved Hide resolved
format/fit/fit.go Outdated Show resolved Hide resolved
Copy link
Owner

@wader wader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments. Do you have something left you want to change or add before merge?


func fitDecodeDefinitionMessage(d *decode.D, drc *dataRecordContext, lmfd localFieldDefMap, dmfd devFieldDefMap, isDevMap localMsgIsDevDef) {
d.FieldU8("reserved")
endian := d.FieldU8("architecture")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sym map this?

fDefLookup, isSet := mappers.FieldDefMap[messageNo][fieldDefNo]
if isSet {
var foundName = fDefLookup.Name
lmfd[drc.localMessageType][i] = mappers.FieldDef{Name: foundName, Type: typ, Size: size, Format: fDefLookup.Type, Unit: fDefLookup.Unit, Scale: fDefLookup.Scale, Offset: fDefLookup.Offset}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: i usually prefer one field init per line in if not very smal structs, maybe there are some more like this?

fieldUint(d.FieldU16, 2, fDef, uintFormatter, &fdc)
case "uint32", "uint32z":
fieldUint(d.FieldU32, 4, fDef, uintFormatter, &fdc)
case "uint64", "uint64z":
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder what the deal with the *z variants, not allowed to be zero? i guess we could map description if invalid for the different variants... do if you feel like it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already handled by the uintFormatter in mappers/values.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've figured out the *z variants. They are used as bitflags, meaning 0 is invalid because then it lacks any value. It also means another weekend project for me =)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already handled by the uintFormatter in mappers/values.go.

Aha yes forgot that

I think I've figured out the *z variants. They are used as bitflags, meaning 0 is invalid because then it lacks any value. It also means another weekend project for me =)

:) that makes sense, and the flags are defined somewhere?

But let me know if want to do it as separate future PR or so

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There aren't that many of them, but they are defined in the Profile.xlsx.
None of their example files nor mine have any of those fields though, so I can't test it either way. I'm good for merging now if you are. Do you want me to squash rebase it first?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha i see, add if you happen to find files using them.

In this case i would probably squash into one or very few commits, but the commit history looks reasonable so i'm ok to merge as is also.


```
$ fq '[.dataRecords[] | select(.dataRecordHeader.messageType == 0).dataMessage]' file.fit
```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if we can come up with some more fancy usage examples 🤔 maybe add later

| | | data_message{}: 0x1f-0x28 (9)
0x10| 00| .| manufacturer: 256 0x1f-0x21 (2)
0x20|01 |. |
0x20| 03 dc | .. | product: 56323 0x21-0x23 (2)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this and product be mapped? i see a manufacturer or product map in types_generated.go

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Profile.xlsx (and thus types_generated.go) has no match for manufacturer 256. Product is a dynamic field (sometimes called a subfield) that has different lookup_tables depending on the value of another field, in this case manufacturer. Maybe I'll give this perticular nut another go at cracking this weekend. =)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha i see, ok give it a go, no hurry to merge :)

d.Fatalf("Unknown endian %d", endian)
}
messageNo := d.FieldU16("global_message_number", mappers.TypeDefMap["mesg_num"])
if messageNo == 206 { // developer field_description
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could generate constants for these but i have a feeling very few of them would be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make a manual constant for this "magic" number, and then have a gathered place for the if more are needed in the future.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

@mlofjard
Copy link
Contributor Author

mlofjard commented Feb 9, 2024

Some minor comments. Do you have something left you want to change or add before merge?

I have some stuff left that I know of, e.g (dynamic/subfields) and accumulated timestamp values from compressed messages. But the subfields are a bigger task, and I have no files with compressed message headers to test on. So I think they'll both have to wait for future versions.

@wader
Copy link
Owner

wader commented Feb 10, 2024

Merge once CI is green?

@wader wader merged commit 5684bc4 into wader:master Feb 10, 2024
5 checks passed
@mlofjard mlofjard deleted the fit branch February 10, 2024 00:31
@wader
Copy link
Owner

wader commented Feb 10, 2024

🥳 Thanks a lot and hope for more PRs in the future!

btw i know a colleague of mine that is super excited about fit support, ping @KristianKarl

@mlofjard
Copy link
Contributor Author

🥳 Thanks a lot and hope for more PRs in the future!

btw i know a colleague of mine that is super excited about fit support, ping @KristianKarl

I have already started on my next format: a nes rom decoder, with a to_asm function for decompiling to 6502 assembly code. =)

@wader
Copy link
Owner

wader commented Feb 10, 2024

Oh nice, it's addictive 😄 to_asm as in return a string with 6502 assembly? interesting 🤔

I have played around with the idea of having opcode decoders but got a bit stuck, didn't feel very neat, maybe could be useful? ex draft PR #215

0x40| 00 | . | local_message_type: 0 0x4d.4-0x4e (0.4)
| | | data_message{}: 0x4e-0x50 (2)
0x40| 00 64| .d| hrm_ant_id: 25600 0x4e-0x50 (2)
0x50|39 50| |9P| | crc: 20537 (valid) 0x50-0x52 (2)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nitpick: In some future PR maybe this can be presented in hex? add a scalar.UintHex mapper. Maybe there are more fields more suitable in hex?

I've thought about generalising this for crc-ish fields somehow not sure how, would be nice to provide both calculated and expected value.

@wader
Copy link
Owner

wader commented Feb 10, 2024

BTW i'm think of will do a new release later today or tomorrow, has been a while. Just do a PR if you want to include some smaller fixes.

And if you happen to have some nice examples to show up the fit decoder drop them here and will include in the release notes! was thinking about including covert to KML maybe?

@KristianKarl
Copy link

Most awesome PR! Great job both! 👏

I already used fq and it's new fit decoder to create a summary of each of my 1300+ fit files in 1 json file. This file is super easy to query using jq to get my longest, fastest etc ride, or to get the total distance for a specific bike. Very handy, I'll do a write-up shortly how to do that,

@wader
Copy link
Owner

wader commented Feb 11, 2024

🥳 Nice! i've thought about splitting up https://github.com/wader/fq/blob/master/doc/formats.md somehow to have a page per format, maybe even use the github wiki? then write ups like that could be included?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants