Update to FIT SDK v20.35

[liamjm]

This required a few tweaks to generate_profile.py.

There are a large number of diffs in the final profile.py. This is caused by the presence of more "group_name" in the message list, which affect the key in the comparator for the sorted() function. This is annoying as it makes it hard to really diff. Let me know if you have other thoughts on this.

[liamjm]

@dtcooper Anyone got time to look at this one?

[pR0Ps]

@liamjm I've just had a look but it's pretty much impossible to do a thorough full review of a +/- 5000 change diff.

Some questions:

• I noticed is that all the units fields are now all unicode strings. Was this done on purpose? Does it affect anything in the parsing/data processing?
• Did all the tests pass using the final profile.py?

A few stray thoughts on making diffs easier going forward:

• Most of the elements that are output by the generate_profile.py script sort by name. However, MessageInfo, MessageList both sort their fields dictionary attributes by id (after group name). Would changing this to sort by name instead result in more manageable diffs (ie. would the numbers ever change while referring to the same data)?
• Removing the MessageList group name sorting from the generated profile.py would probably make future diffs much simpler. This would have an impact on human readability but does a file full of autogenerated struct definitions really need to be human-readable?

@dtcooper thoughts?

[dtcooper]

I trust your judgement, @pR0Ps. But I would like to see this file programatically generated, or if things need to happen by hand, then a clear documented process to generate the profile.

[liamjm]

Hey,

This change was generated by the generate_profile.py, which required a few tweaks to parse the latest SDK version. But yes, it is crazy big and unreasonable to expect manual review. A few more thoughts going forward:

• let's add running of the tests to the project to protect against regressions.
• I'll try to get v20.33 of the SDK (the source of the current version) and re-generate profile.py to ensure that no changes are being introduces by my environment.
• IMHO sorting should be done to reduce differences. We just need to work out the most stable property to sort on. To the best of my recall, many of the changes were caused by the change in group name, but I can dig into this more.

[liamjm]

Quick update:

• Adding of the test files is in https://github.com/dtcooper/python-fitparse/pull/40
• The previous versions of the SDK aren't publicly accessible (though I have asked https://www.thisisant.com/forum/viewthread/6968/). This means I'm unable to re-produce the v20.33 Profile. I have generated the v20.54 by using python3 and there are much less diffs. I'll update the docs to explain this.
• We'll need to work out the best way to produce stable sorting of the field, I've not looked at that yet.

[dtcooper]

Fear not, @liamjm! I have pretty much every fit SDK released. Drop me a line at [email protected], if you want to rework the generate profile script from the excel files contained therein, I'll send 'em your way.

D

On Fri, Jan 5, 2018 at 8:17 PM Liam Murphy [email protected] wrote:

Quick update:

• Adding of the test files is in #40 https://github.com/dtcooper/python-fitparse/pull/40
• The previous versions of the SDK aren't publicly accessible (though I have asked https://www.thisisant.com/forum/viewthread/6968/). This means I'm unable to re-produce the v20.33 Profile. I have generated the v20.54 by using python3 and there are much less diffs. I'll update the docs to explain this.
• We'll need to work out the best way to produce stable sorting of the field, I've not looked at that yet.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/dtcooper/python-fitparse/pull/31#issuecomment-355721899, or mute the thread https://github.com/notifications/unsubscribe-auth/AA1V2OIKq5HmntajhL4QtZNG0nSBE-1Aks5tHvPVgaJpZM4OFL-d .

[xmedeko]

What about to split profile.py file to more files: profile_types.py and profile_messages.py (or just the old profile.py to keep backward compatibility.) Then, it would be a bit simpler to track commit changes.