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

Update Open-Sky Network Private Single Flight API Example #167

Merged
merged 8 commits into from
Apr 10, 2024

Conversation

DJDevon3
Copy link
Contributor

@DJDevon3 DJDevon3 commented Mar 18, 2024

Requires Open-Sky Network website account. Instructions in code comments. Allows for more daily calls than public API.

Added

  • pretty serial hierarchy view
  • general code cleanup and improvements
  • improved endpoints with server side failure messages
  • improved and streamlined debugging
  • shouty constants to make Pylint happy.
  • imports sorted
  • switched from circuitpython_base64 to built-in module binascii for base64 credential encoding
  • doc strings for functions
  • uses settings.toml by default
  • adafruit_circuitpython_requests now using Connection Manager by default

Serial output example:

Connecting to WiFi...
✅ Wifi!
 | Attempting to GET OpenSky-Network Single Private Flight JSON!
 | Website Credentials Required! Allows more daily calls than Public.
 | ✅ OpenSky-Network JSON!
 |  | Last Contact: 03/19/2024 09:49:10
 |  | Transponder: 4b1806
 |  | Callsign: SWR30M  
 |  | Squawk: 1000
 |  | Origin: Switzerland
 |  | Longitude: 8.7047
 |  | Latitude: 48.1189
 |  | Barometric Altitude: 3962.4
 |  | Velocity: 176.06
 |  | Vertical Rate: 0
✂️ Disconnected from OpenSky-Network API

Finished!
Board Uptime: 1.2 days
Next Update: 30 minutes
===============================

@DJDevon3
Copy link
Contributor Author

I messed up and published both the private and public API examples here.
commit 8a34d2a is the private example
commit 231e2eb is the public example
The commits didn't merge because I published them from 2 branches. sigh, I don't know how to correct this.

@dhalbert
Copy link
Contributor

The commits didn't merge because I published them from 2 branches. sigh, I don't know how to correct this.

You can revert a commit. Or fix it locally and do a push -f (force push).

@justmobilize
Copy link
Collaborator

@DJDevon3 any reason not to merge these into a single example? And base if there are creds, print out a different thing:

Connecting to WiFi...
✅ Wifi!
 | ✅ OpenSky credentials found, using private
 | Attempting to GET OpenSky-Network Single Flight JSON!

or

Connecting to WiFi...
✅ Wifi!
 | ❌ OpenSky credentials not found, using public
 | Attempting to GET OpenSky-Network Single Flight JSON!

@FoamyGuy
Copy link
Contributor

I think I am in favor of keeping them separate. I wouldn't say that it makes a huge difference either way, but I like that the seperate public API one can be a bit less complex (losing the chunk of code responsible for base 64 encoding the auth), as well as drop the requirement for adafruit_binascii

I often start my projects by copying the most relavent library example for the first feature that I decide to work on, I appreciate having the seperated minimal examples because it tends to result in less unused stuff to find and cut out of the code.

@FoamyGuy
Copy link
Contributor

@DJDevon3 I'm don't think I understand your comment about the two different commits for public / private versions of the examples.

Is that something that needs to get resolved before this PR can be considered for merging?

Maybe your intention was originally to do public and private in separate PRs? but they accidentally got both put into this one? If that is the case I think it's okay to just move forward with them both in this PR.

The changes look mostly fine to me, I'll go ahead and test them out but will hold off on approval / merging just in case there is something that needs to be done prior to that.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

I tested the public example successfully. But I'm having trouble getting the private API example to work. I double checked that my username and password are correct by logging in via the website with the same ones and using the debug flag to have it print them locally in my serial.

It seems like the Base64 version is ending up with an extra \n at the end of the base64 data, I'm not sure if that is intentional or if it's related to the problem for certain but it's odd because it isn't there in the password.

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Mar 19, 2024

About "\n" appended to base64 hash:

I switched this and many other examples that relied on base64 encoding from the community library circuitpython_base64 to adafruit_binascii as suggested by Dan. I thought the "\n" on the end was part of the hash. Oops! Obviously I didn't do enough testing. Open-Skynetwork accepts it with the \n appended with or without the slice! for me anyway, and that's quite odd.

This isn't a real login, just an example base64 hash example

VXNlcm5hbWU6MDEyMzQ1Njc4OQ==
VXNlcm5hbWU6MDEyMzQ1Njc4OQ==\n

Open-Sky Network will accept either of these for me... and that's not good. I'm glad it fails for you. It should fail for me but it doesn't.

Issue with binascii library:

The binascii simpletest doesn't include a way to spit out the hashed string without b'encapsulation\n'. The library is adding the \n... WHY?!?! It's even demonstrated in the binascii simpletest. Both b2a and a2b decoding still retains it inside the byte encapsulation and requires padding! It's extremely picky about the format which makes me think binascii was originally designed for a very specific purpose.

Problem with adafruit_binascii for web base64 encoding:

If they would have provided an example to drill it down to a pure string I'm sure they would have also used slice[2:-3] or equivalent method to output to a pure string. I don't see a method to do that. This is a problem for passing the exact correct string into the header. As far as I can tell it's presumed that the developer (you and me) must use slice to get it the rest of the way. I have a lot of branch splitting and recommits to do today anyway, will definitely fix this which should be as simple as changing TRUNCATED_BASE64_STRING = BASE64_STRING[2:-1] to TRUNCATED_BASE64_STRING = BASE64_STRING[2:-3]

I might make a PR to adafruit_binascii so that all you have to do is pass a username:password string into it and it spits out a base64 encoded string. With http/s server getting a lot of work in Circuit Python it's just a matter of time before this becomes a necessary feature. If you know of a better way that already does this I'm all ears. This is the first time I'm using binascii, previously I'd always used circuitpython_base64.

@DJDevon3 DJDevon3 changed the title Update Open-Sky Network Single Flight API Example Update Open-Sky Network Private Single Flight API Example Mar 19, 2024
@FoamyGuy
Copy link
Contributor

Interesting about binascii adding the newline. It appears that behavior is inherieted from CPython: https://docs.python.org/3/library/binascii.html#binascii.b2a_base64 although in CPYthon the eventually added a newline argument to b2a_base64 in order to disable the newline being added python/cpython#69544

I think we should add same newline argument and behavior into adafruit_binascii so it can match the CPython implementation.

I don't necessarily think that adafruit_binascii would be the appropriate place for a function that takes username:password and spits out the string ready to be added to a header unless the CPYthon version has that functionality. For the circuitpython libraries that match CPython named ones they should be a subset of the CPython one with matching behavior whenever possible. I do think the functionality itself would be valuable but should get added somewhere that matches a CPython API if possible.

In CPython requests it looks like there is HTTPBasicAuth which handles that https://requests.readthedocs.io/en/latest/user/authentication/ we could add that class here in the adafruit_requests library and then have matching APIs between circuitpython and CPython more closely which will make code more portable.

@FoamyGuy
Copy link
Contributor

Actually it looks like there is also a core module binascii and it does already have the newline argument.

>>> import binascii
>>> dir(binascii)
['__class__', '__name__', '__dict__', 'a2b_base64', 'b2a_base64', 'crc32', 'hexlify', 'unhexlify']
>>> binascii.b2a_base64(b"hello")
b'aGVsbG8=\n'
>>> binascii.b2a_base64(b"hello", newline=False)
b'aGVsbG8='

I ran the above on a M5 Cardputer (ESP32 S3 based)

So it looks like you could use this and won't even need to have adafruit_binascii as a required library if I am understanding correctly (assuming this module is enabled in the core on all or most devices, I only tried 1 so far)

@DJDevon3
Copy link
Contributor Author

I did see that last night when looking into the library but was unsure how to use it or if it was a usable parameter. Thank you for the example. I'll take a look into updating them after the dust settles from all the PR's.

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Mar 19, 2024

Figured out why I'm still able to login even with \n appended. It only checks the first 18 characters of the hash, ignores anything after lol. So \n should have also worked for you unless your hash is less than 20 characters.

VXNlcm5hbWU6MDEyMzQ1Njc4OQ==  # this will log you in
VXNlcm5hbWU6MDEyMzQ  #  so will this
VXNlcm5hbWU6MDEyMz  #  this fails to authorize

However if your base64 hash ends up being shorter than 18 characters it will check them all... I hope! I haven't tested it since I can't change my username to be shorter.

@DJDevon3
Copy link
Contributor Author

Also used this commit to delete the twitter api example and use ruff with pre-commit as an experiment.

@DJDevon3
Copy link
Contributor Author

This one is ready to go. Please merge this one first, then PR 172

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Apr 1, 2024

Figured out why I'm still able to login even with \n appended. It only checks the first 18 characters of the hash, ignores anything after

Yikes! 😨

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

lets use encode() and decode() instead of the slicing to convert from bytestring to string.

I've tested the changes suggested bellow successfully and they appear to be working successfully.

I think this is the proper way to do those conversions and it allows us to get rid of SLICED_BASE64_STRING which makes that portion of the code a little bit less complex.

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 2, 2024

I'm using pylint ignore module import due to this.

C:\Users\Devon\Documents\GitHub\Adafruit_CircuitPython_Requests\examples\wifi\expanded>black requests_wifi_api_openskynetwork_private.py
All done! ✨ 🍰 ✨
1 file left unchanged.

C:\Users\Devon\Documents\GitHub\Adafruit_CircuitPython_Requests\examples\wifi\expanded>pylint requests_wifi_api_openskynetwork_private.py
************* Module requests_wifi_api_openskynetwork_private
requests_wifi_api_openskynetwork_private.py:10:0: E0401: Unable to import 'adafruit_connection_manager' (import-error)
requests_wifi_api_openskynetwork_private.py:11:0: E0401: Unable to import 'wifi' (import-error)

-------------------------------------------------------------------
Your code has been rated at 9.12/10 (previous run: 10.00/10, -0.88)

This is on my local pylint. It has helped me clean up a lot of errors before submitting but some I cannot get passed.

Will try to submit even with the error and see what happens.

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 2, 2024

Apparently that passes here but not on my local pylint. Makes me wonder how many other linting errors I've been fixing unnecessarily.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you!

@FoamyGuy FoamyGuy merged commit d595686 into adafruit:main Apr 10, 2024
1 check passed
@DJDevon3 DJDevon3 deleted the DJDevon3-openskynetwork_private branch April 10, 2024 23:18
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Apr 11, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 3.2.4 from 3.2.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#184 from DJDevon3/GithubAPI
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#177 from DJDevon3/DJDevon3-WifiSimpleTest
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#176 from DJDevon3/DJDevon3-MultipleCookies
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#167 from DJDevon3/DJDevon3-openskynetwork_private
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#168 from DJDevon3/DJDevon3-openskynetwork_private_area

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
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.

4 participants