Tigge / antfs-cli

Extracts FIT files from ANT-FS based sport watches such as Garmin Forerunner 60, 405CX, 310XT, 610 and 910XT.
MIT License
312 stars 76 forks source link

Fix division by zero error in progress callback when uploading #104

Closed fdChasm closed 9 years ago

fdChasm commented 10 years ago

Fix bug uploading where first call to the progress callback occurs with a 0.0 value for the new_progress argument resulting in a division by zero error.

When the progress percentage is exactly zero, it is not possible to guess how much time remains based on the elapsed time. So it makes sense to instead display "ETA: Unknown".

Tigge commented 10 years ago

Thanks for the merge request! Does this happen on the split branch as well? That's the newest development branch.

fdChasm commented 10 years ago

Did realize master wasn't the latest. I'll re-test this, but probably not until next week some time. Thanks.

Toilal commented 9 years ago

@Tigge Did you know you can set the split branch as default branch in github ? It would avoid some misunderstanding i think.

Tigge commented 9 years ago

I did not know that! Fixed that now. Thanks.

Toilal commented 9 years ago

Great :) thanks

mgr01 commented 9 years ago

Shouldn't new_progress always be > 0 in the first callback? I think that the bug is in ant.fs.manager.Application.upload() -- instead of:

callback(float(offset) / float(len(data)))

there should be:

callback(float(offset + len(data_packet)) / float(len(data)))

because you're calling callback after successfully sending data_packet to the watch. I could prepare pull request if you like.

Tigge commented 9 years ago

@mgr01 that seems correct! I would really appreciate a pull request. Should probably add a callback with value 0.0 after the upload request is successfully created as well. Also feel free to clean up any uncommented code and weird stuff in this area :).

Tigge commented 9 years ago

Since that would be in openant I'll close this issue

mgr01 commented 9 years ago

@Tigge when are you planning to merge #112 and Tigge/openant#4? I'd like to base new pull requests on these commits to avoid large conflicts.

Toilal commented 9 years ago

:+1:

Tigge commented 9 years ago

@mgr01 I'm looking though the pull request now. Looking good! Besides a few comments and nitpicks here and there I'd be nice to merge them in as soon as the few issues are cleaned up. Can probably base any new patches on top of them.