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

Store the garth session information and upload the fit file via garth #132

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

jrast
Copy link
Contributor

@jrast jrast commented Sep 28, 2023

To reduce the requests during garmin login, the garth session information is stored similar to the withing session data. Also garth is used to upload the fit-file and cloudscrapper is no longer required. I don't know if we can get rid of cloudscrapper or if there is still an advantage if the upload is done with cloudscrapper.

As with my other PR: If you like these changes let me know and I can cleanup. For example the directory in which the garth session is stored should be defined at a central place.

@stynoo
Copy link
Contributor

stynoo commented Sep 28, 2023

Imho, it looks like this is the way to go. Garth does all the heavy lifting and should be able to handle MFA as well (not sure if extra code is needed to save the oauth1/2 tokens somewhere)
The directory should ideally be the same place where .withings_user.json is stored. Iirc it's /root in the containers.

As for cloudscraper: this was a drop in replacement that fixed the last auth issues. If Garth works without it then I don't see a reason to keep it.

@@ -22,31 +21,34 @@ class APIException(Exception):
class GarminConnect:
"""Main GarminConnect class"""

UPLOAD_URL = "https://connect.garmin.com/upload-service/upload/.fit"

# From https://github.com/cpfair/tapiriik
@staticmethod
Copy link
Collaborator

Choose a reason for hiding this comment

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

This question should be in the MR before. However, is this method static anymore? We reference garth.* stuff.
If yes, my bad im still kind of beginner in python/pep's.

@longstone
Copy link
Collaborator

As with my other PR: If you like these changes let me know and I can cleanup. For example the directory in which the garth session is stored should be defined at a central place.

Yes! I very appreciate every PR making this simple and maintanable.
As @stynoo already said, a file should be where the other files are. Currently /root

@jrast
Copy link
Contributor Author

jrast commented Sep 29, 2023

With matin/garth#9 merged in garth, the garmin upload can be further simplified. Currently the implementation is more of a proof-of-concept, there are some points I personally would change:

  • Create a custom garth client instead of using the global one (might be overkill)
  • I don't get why the methods of the GarminConnect class are static, this might be something which was required some time ago. But with the current implementation there are better ways.
  • If garth is providing the upload method, the GarminConnect class can be remove entierly and everything can be done in the sync_garmin method directly.

@jrast
Copy link
Contributor Author

jrast commented Sep 29, 2023

Imho, it looks like this is the way to go. Garth does all the heavy lifting and should be able to handle MFA as well (not sure if extra code is needed to save the oauth1/2 tokens somewhere)

Storing the tokens reduces the network request considerably, making the upload process more robust as less things can fail.

The directory should ideally be the same place where .withings_user.json is stored. Iirc it's /root in the containers.

I will look into this.

@matin
Copy link

matin commented Sep 29, 2023

Garth now natively supports upload as of 0.4.32.

with open("12129115726_ACTIVITY.fit", "rb") as f:
    uploaded = garth.client.upload(f)

Let me know if anyone runs into issues, and I can fix them.

@matin
Copy link

matin commented Sep 29, 2023

@jrast if you decide to do this ...

Create a custom garth client instead of using the global one (might be overkill)

take a look at how garminconnect creates a custom Garth Client vs using the global one.

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.

None yet

4 participants