pe-st / garmin-connect-export

Download a copy of your Garmin Connect data, including stats and GPX tracks.
MIT License
370 stars 76 forks source link

Feature/countnew #46

Closed chs8691 closed 3 years ago

chs8691 commented 4 years ago

Added argument 'count new'. This will download all new activities. For this, a pickle file '.settings' in the download directory will be used. It holds the index of the newest/latest activity that is downloaded successfully. The file will be updated every time an newer activity is downloaded (independent of the arguments). 'Count new ' is using the total activity index. This will fail, if activities will be deleted or inserted on Garmin Connect. To achieve a seamless and stable download history, the download order is now inverted - from oldest to newest. This does not quite fit with the known behavior and in special with the -sa argument. But from my point of view it is the better way. Parameter start_activity_no is still proper working but now a little bit hard to understand :0|

Examples: // User has 5 activities on Garmin Connect with index 1 (oldest) .. index 5 (newest) gcexport count 3 // Fetch activities by index 3-5, save '5' in .settings // User creates new activity with index 6 on Garmin Connect gcexport count new // Fetch activity with index 6 and update .settings with '6' gcexport count new // No effect gcexport count 5 -sa 4 // Fetch activity index 1, .settings will be untouched rm download/.setting gcexport count new // Fetch activities with index 1..6 and update .settings with '6' // User deletes activity with index 3 and creates new activity on Garmin Connect. This new activity will have index 6. gcexport count new // No effect - total number of activities hasn't changed and the last index is still '6' :0(

bxsx commented 4 years ago

I don't see it that useful but I can imagine a lot of additional effort to maintain it later. If I understand it correctly the only difference here is to have ability to keep correct counting even if user broke counting by deleting/inserting activities. I don't think this app should fix user problems. In fact, I'm sure that every user of this script is aware of the limited capabilities and definitely if manually editing Garmin Connect activities then double check expected output from the script. I might also add that the fit files are so small, that simpler way to fix this issue is by removing the directory and re-download the whole history.

cristian5th commented 4 years ago

Hello.

This proposal is very useful in order to avoid downloading the data of all activities when you actually only need the two or three newest ones.

I have created the same functionality at my branch called "personal-adjustments". My branch cannot be merged with the original project because this is not the only modification I have introduced. But it might help you to have a look at how I did it.

Instead of downloading the activities by number, I download the activities by date. This is completely avoiding all the issues you are presenting when activities are being deleted at Garmin Connect. Basically, when the argument is "count new", I change the search params to search_params = {'startDate': last_date, 'sortBy': 'startLocal', 'sortOrder': 'asc', 'limit': num_to_download} and the value of startDate is the date when the last downloaded activity was recorded. This is the data that I store in the file ".settings". Doing like this, if activities are being deleted at Garmin Connect, I will still not miss any new activity at the next download.

Regards.

bxsx commented 4 years ago

I am not against the feature, I am against the current implementation. This introduce a new file and breaks scriptability of the main tool which power was thanks to single file functionality. Introducing new file raises a lot of questions. Where to store it? Shouldn't be in the $HOME/.garmin-connect-export/ directory? What about multiple accounts? I don't think calling it ".settings" is proper as in fact its more likely a database. This raises another questions such as why not use SQLite or whatever else for further performance and functionalities? Etc. etc.

I think the most important part of this tool is to keep connections to GC API stable. There are plenty of small features that could be added and then used by few users. However, this gains complexity of the code and usability in some occasions too.

As I said previously:

I'm sure that every user of this script is aware of the limited capabilities and definitely if manually editing Garmin Connect activities then double check expected output from the script. I might also add that the fit files are so small, that simpler way to fix this issue is by removing the directory and re-download the whole history.

Also, I think this is super rare situation to edit GC history manually.

So, to keep everyone happy, you may try to implement it by adding a new arguments to the command line. I guess a switcher of how to count activities (with the default value to the current default) and maybe some extra controllers.

pe-st commented 4 years ago

If I understood correctly, your proposing to make 3 changes, maybe this should be individual pull requests?:

I can see that for new a reversed order is simpler. But for the number option reversing the counting may make the script more difficult to understand, as the number of activities in the user stats page (the total_to_download) can be misleading. For my account there are about a dozen activities that are not accounted for, let's say I have total_to_download == 1000, but the download loop only downloads 988 activities. You would then have to take into account these missing activities for the -sa option.

Thus for the reversed order I'd prefer it configurable, and maybe force to oldest-first for the new option. If the exporter is integrated into other scripts it might break them if you change the order also for the all and number options.

As for a local file to remember the last downloads.

cristian5th commented 4 years ago

Hello again.

I just wanted to continue a little more on the way I do it. Please, don't forget that it was a different Christian ( @chs8691 ) who originally proposed this and all credits should go to him.

The main reason I implemented this for myself is because executing the all option everyday is loading the GC servers without need and Garmin people might not like it and close our access one day. Using the number at the count argument is not automated for me because I don't know how many new activities I would have created since the last time the script was executed. So creating a new option was the logical way to do it. The script is automatically executed every day at 3:33 inside a Raspberry Pi at the garage.

Regards.

chs8691 commented 3 years ago

Hi! Sorry for my late answer and thank you for the really useful discussion. I agree to your objections and would like to dismiss this request. Nonetheless I still need a 'load new' function. Maybe I'll start a new request in the future, with a better integrated and more stable coding. Happy new year!

bxsx commented 3 years ago

@chs8691 thanks for your effort! I think I have something like “load new” feature in one of my local branches. I will be back to this branch next month and prepare a pull request, so you may find it interesting. I will try to mention you once the pull request will be ready.