fit-parser icon indicating copy to clipboard operation
fit-parser copied to clipboard

Undefined function in binary.js inside the published npm package

Open thomaschampagne opened this issue 2 years ago • 15 comments

Hey @jimmykane,

I'm looking to understand why every cycling dynamics stats like avg_left_power_phase, avg_left_power_phase_peak are returning NaN values inside this library (file used: 7386755164.zip).

1 - I noticed that in the npm distributed package (v1.9.0) the following file node_modules\fit-file-parser\dist\binary.js has an error. Indeed inside function readData the following line:

return dataView.getUnt8(0, fDef.littleEndian);

Should be

return dataView.getUint8(0, fDef.littleEndian);

getUnt8 function doesn't exists, the getUint8 yes !

2 - Aside this, the readData (dist folder) from repository is not the same than the npm package. Why? I was unable to find tag 1.9.0 on this repository.

Thanks for your help !

Thomas

thomaschampagne avatar Sep 14 '21 17:09 thomaschampagne

Hey, that's correct it's a bug most probably.

I'll be fixing this somewhere next week if that is ok with you

On Tue, 14 Sep 2021, 20:02 Thomas Champagne, @.***> wrote:

Hey @jimmykane https://github.com/jimmykane,

I'm looking to understand why every cycling dynamics stats like avg_left_power_phase, avg_left_power_phase_peak are returning NaN values inside this library (file used: 7386755164.zip https://github.com/jimmykane/fit-parser/files/7163804/7386755164.zip).

1 - I noticed that in the npm distributed package (v1.9.0) the following file node_modules\fit-file-parser\dist\binary.js has an error. Indeed inside function readData the following line:

return dataView.getUnt8(0, fDef.littleEndian);

Should be

return dataView.getUint8(0, fDef.littleEndian);

getUnt8 function doesn't exists, the getUint8 yes !

2 - Aside this, the readData from repository is not the same than the npm package. Why? I was unable to find tag 1.9.0 on this repository.

Thanks for your help !

Thomas

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jimmykane/fit-parser/issues/28, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJVX473FKY45EB5RKSXS6TUB55ZPANCNFSM5EATCETQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jimmykane avatar Sep 16 '21 06:09 jimmykane

It's perfect :) Thanks !

By the way, I switched from npm package to current repository fit-parser state . I now have the correct code :) But i still have NaN issues on cycling dynamics stats (avg_left_power_phase=NaN, avg_left_power_phase_peak=NaN, ...).

The activity used to fetch cycling dynamics stats is https://connect.garmin.com/modern/activity/7386755164 (https://github.com/jimmykane/fit-parser/files/7163804/7386755164.zip)

Thanks !

thomaschampagne avatar Sep 16 '21 08:09 thomaschampagne

You will also find below the CSV export of this fit file using the java FitCSVTool.jar provided in latest FIT SDK from garmin

java -jar FitCSVTool.jar 7386755164.fit

Gives: 7386755164_FitCSVTool_export.csv.zip

Power phase like data is returned as is for instance: left_power_phase=28.12500043945313|202.50000316406255. This format could explain the NaN results.

Once this npm publish et field fixed I will provide new fields I identified through a PR. And also in a sports-lib PR with a lot of improvements I'm preparing for you for many weeks ;)

Thanks again!

Thomas

thomaschampagne avatar Sep 17 '21 11:09 thomaschampagne

Hey @jimmykane . Have you been able to take a look at this? Thanks to you !

thomaschampagne avatar Sep 24 '21 13:09 thomaschampagne

Not yet buddy. Broke my toe and have been a bit strange. Next week should be fixed. I have also some time free so I promise.

On Fri, 24 Sep 2021, 16:05 Thomas Champagne, @.***> wrote:

Hey @jimmykane https://github.com/jimmykane . Have you been able to take a look at this? Thanks to you !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jimmykane/fit-parser/issues/28#issuecomment-926608886, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJVX44MVTR6V45ITWSCCD3UDRZP7ANCNFSM5EATCETQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jimmykane avatar Sep 24 '21 13:09 jimmykane

Ouch ! Hope you're fine and can walk by the way? It can wait. Have a good recovery !!!

thomaschampagne avatar Sep 24 '21 19:09 thomaschampagne

I took a look and strangle this part of issue is not in the code of this repo. :-O

jimmykane avatar Sep 27 '21 09:09 jimmykane

@thomaschampagne I tested your file after some fixes here and there and works ok.

I also released sports-alliance/sports-lib v5.4.24 that should have this fixed

Screenshot 2021-09-27 at 13 45 51

jimmykane avatar Sep 27 '21 10:09 jimmykane

Hi @jimmykane.

(1) About the power I was probably unclear :D I was talking about the L/R power phase which is a angle between 0 to 360 degrees. This is what you can see there (https://connect.garmin.com/modern/activity/7386755164):

image image

This angle/phase L/R data is parsed as NaN by the fit-parser (1.9.2): image

I think the issue comes from how the data is formatted inside the FIT protocol (see https://github.com/jimmykane/fit-parser/issues/28#issuecomment-921717808)

(2) Just to mention it, I updated to "fit-file-parser": "^1.9.2", and I still have the wrong code (with the getUnt8 function undefined) in the generated dist folder.

image

I hope your toe goes better?

Thanks again,

Thomas

thomaschampagne avatar Sep 28 '21 22:09 thomaschampagne

Undefined was fixed. It was the issue in src , strange I had not spotted it. Can you try if this by any way fixes the l_power_phase etc?

If else I think those fields are not implemented for decoding. This will take a few

jimmykane avatar Sep 29 '21 07:09 jimmykane

@thomaschampagne version 1.9.3 with fix to the unknown function is out

jimmykane avatar Sep 29 '21 07:09 jimmykane

Undefined was fixed. It was the issue in src , strange I had not spotted it. Can you try if this by any way fixes the l_power_phase etc?

If else I think those fields are not implemented for decoding. This will take a few

I try this this evening! I let you know

thomaschampagne avatar Sep 29 '21 11:09 thomaschampagne

I updated to 1.9.3. Unknown function is fixed. However it doesn't fix the NaN values on power phases

image

thomaschampagne avatar Sep 29 '21 22:09 thomaschampagne

Hello, The data of left/right power phase get with a python script are like : (347.3437554272462, 201.0937531420899, 212.34375331787115, 94.218751472168). The problem it is may be its type ? (uint8)

nenes82 avatar Mar 14 '22 10:03 nenes82

Hello, my problem was that the scale value is 0,711111. I replaced comma by a dot and it's ok ! May be there is a better solution to convert value during the execution ?

REgards

nenes82 avatar May 17 '22 16:05 nenes82