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

Migration: Cloudscraper -> Garth #141

Closed
matin opened this issue Aug 6, 2023 · 16 comments
Closed

Migration: Cloudscraper -> Garth #141

matin opened this issue Aug 6, 2023 · 16 comments

Comments

@matin
Copy link
Contributor

matin commented Aug 6, 2023

There are some issues with the method of using Cloudscraper and scraping the website. Garth solves this.

It's possible to keep the same interface as what you have now. I put together a quick example without disrupting the existing interface. The integration is also fairly straightforward.

Here are some advantages of Garth over Cloudscraper:

  1. Uses OAuth1 and OAuth2 token authentication after initial login
  2. OAuth tokens survive for a year
  3. Supports MFA
  4. Auto-refresh of OAuth2 token when expired
  5. OAuth1 token is valid for one year
  6. Works on Google Colab
  7. Full test coverage

The migration from Cloudscraper to Garth would close #113, #127, #129 and #135.

Let me know if you're interested in making the migration and I can issue a PR.

@cyberjunky
Copy link
Owner

@matin nice find and work! It would indeed solve multiple issues, and save me some time (which I lack lately) currently on holiday, but have created a short Todo list around the Garmin project. So a PR would be very welcome! Thanks in advance!

@matin
Copy link
Contributor Author

matin commented Aug 6, 2023

It was fairly straightforward to migrate .get()

I included a notebook with some examples.

I'll work on the downloading, setting and uploading as well. I just wanted to provide a better idea of what the migration would look look like.

Let me know if you like this direction, and I can finish it up to issue a PR

@matin
Copy link
Contributor Author

matin commented Aug 8, 2023

@cyberjunky just want to confirm this approach looks good to you. If it does, I can migrate the rest for the PR.

@matin
Copy link
Contributor Author

matin commented Aug 18, 2023

Added some tests.

To run tests:

make install-test
make test

@AnonJohn
Copy link

Writing mostly to bump for cyberjunky. @matin - your work with Garth is very nice and elegant. Useful tutorial for an old data hand who hasn't kept up with the state of the art (e.g. pydantic integration). But thank you for taking the time to reach back here and offer improvements to the broader reaching / more complete Garmin API. Hoping it gets integrated but I for one am using it!

@matin
Copy link
Contributor Author

matin commented Aug 29, 2023

@AnonJohn Happy to hear you're using it, and thanks for the kind words.

@AnonJohn
Copy link

AnonJohn commented Sep 3, 2023

This is a combination of a new api call question and possible garth POST issue. Posting here @matin since I'm trying to use garth within garminconnect (and it's not obvious what a better place would be). ☺

I am seeking to add an api call for uploading weight. This has been reverse engineered in the past, e.g. here, here, here, and here.

I got as far as this the code below, but it fails with a "requests.exceptions.HTTPError: 402 Client Error: Payment Required for url: https://connect.garmin.com/proxy/weight-service/user-weight"

I know this was surmounted generally a few years ago by getting the header right and logging in. Any clues or help? I can't tell if it is just a paywalled API function or a garth header question.

Specifically, I note that the header "{"NK": "NT"}" had to be incorporated into other scripts (including garminconnect) to address a this payments question on the "GET" side. (see here). Might this be needed in the POST headers on garth?

def upload_weight(self, weight, date, unit = 'lbs'):
url = '/proxy/weight-service/user-weight'

    if unit == 'lbs': weight = weight / 2.2046	
    params = {
        "value": str(weight),
        "unitKey": 'kg',
        "date": date
    }
    try:
        #return self.modern_rest_client.get(url, params=params).json()
        return self.garth.post("connect",url,json.dumps(params))

    except:
        raise GarminConnectInvalidFileFormatError(f"Could not upload weight")

@matin
Copy link
Contributor Author

matin commented Sep 3, 2023

I don't have my computer on me right now, but I'll take a look tonight.

FWIW, the URLs used for Garth should be connectapi.garmin.com and not include /proxy. You can use garth.client.post("connectapi", endpoint), but I'll try it tonight to make sure it works for that particular endpoint.

@AnonJohn
Copy link

AnonJohn commented Sep 4, 2023

hould be connectapi.garmin.com and not include /proxy. You can use garth.client.post("connectapi", endpoint), but I'll try it tonight to make sure it works for that particular endpoint.

Many thanks Matin! FWIW I started with the connectapi (and have done it with and without proxy) to no avail. Errors below. Others have (once) had it working as I've been trying above; and direct website inspection suggests it too.

It may be a malformed request as there is a 400 error for the way you suggest. Any help is most appreciated!

No proxy:
Error in request: 404 Client Error: Not Found for url: https://connect.garmin.com/weight-service/user-weight

400 Client Error: Bad Request for url: https://connectapi.garmin.com/weight-service/user-weight

PS - When we are done here I will delete these posts if I can and open a new ticket if there is a useful addition. Don't want to distract from the Garth conversation.

@matin
Copy link
Contributor Author

matin commented Sep 4, 2023

@AnonJohn Here's the pure Garth implementation for add user weight.

tz_delta is only necessary in this example because Colab is running on UTC, and for some reason, Garmin wants to receive both UTC and local time in the request.

HTH

I can add it to this library if @cyberjunky would like.

@AnonJohn
Copy link

AnonJohn commented Sep 4, 2023

@AnonJohn Here's the pure Garth implementation for add user weight.

tz_delta is only necessary in this example because Colab is running on UTC, and for some reason, Garmin wants to receive both UTC and local time in the request.

HTH

I can add it to this library if @cyberjunky would like.

Brilliant! It works for me. I bow in your general direction; this would have taken me weeks!!! Incidentally I note that garmin also takes "lbs" as a valid unit (for those of us in the benighted part of the world).

@cyberjunky - encourage you to say yes to adding this to the library! Use cases from other threads and implementations:

  • Uploading historical weight data to garmin
  • Syncing with other devices / platforms / services. This is particularly and issue for people that have other smart scales, but also applies to people who want to (e.g.) have data on google fit (or any of the platforms that integrate) and also want it uploaded into Garmin (without needing to enter it twice).

Cheers + thanks again @matin!

@AnonJohn
Copy link

AnonJohn commented Sep 11, 2023

Hi @matin - Me again, but this time with what I think is a bug in your preliminary implementation of garminconnect via garth. When attempting to download an activity (e.g:

garmin.download_activity(activity_id, dl_fmt=garmin.ActivityDownloadFormat.ORIGINAL)

One gets this error:
site-packages/garth/http.py", line 152, in connectapi
rv = resp.json()
site-packages/requests/models.py", line 975, in json
raise RequestsJSONDecodeError(e.msg, e.doc, e.pos)

It appears that garth's "connectapi" function is attempting to parse the response (which is a binary file) as json: rv = resp.json()

In the original implementation the .json() call was put outside of the request handling (in the various other functions). I think you moved it inside connectapi. Could adopt the original approach or put a test in connectapi?

Putting a try in connectapi, as below, does work. But it is probably more elegant to mirror the original code and put the json parsing on the functions that call connectapi.

try:
rv = resp.json()
except:
rv=resp

@matin
Copy link
Contributor Author

matin commented Sep 11, 2023

@AnonJohn thanks for finding and reporting a bug! I'll work on fixing it today.

@matin
Copy link
Contributor Author

matin commented Sep 11, 2023

@AnonJohn fixed with included tests in both Garth and garminconnect. I added garth.download(path) to Garth and adapted accordingly in garminconnect.

Could you give it a try?

If you installed garminconnect with pip (as opposed to cloning the fork), you can run the following to get the latest:
pip install -U git+https://github.com/matin/python-garminconnect.git@garth#egg=garminconnect

@AnonJohn
Copy link

@matin installed the latest and confirmed it solves the issue. A nice solution. Cheers + thanks! I'll continue to report any issues I find in the hopes that this becomes the main line. I really do like how the garth integration improved garminconnect.

@cyberjunky
Copy link
Owner

I have merged all code, and all outstanding PR's some small issues left, but the many tests i did using example.py for all calls multiple times, shows it rock solid now (need to test the MFA part) but this is a major upgrade to my code, thanks @matin and all that helped and tested it before me ;-)
Waiting a few hours/days to see if I can fix the last issues for new release version.

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

No branches or pull requests

3 participants