-
Notifications
You must be signed in to change notification settings - Fork 37
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
add Queue-Times API Example #182
Conversation
Used for Disney theme park queue times
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat, thanks for publishing this!
I think I'm in favor of just printing the results once instead of looping to print the same data over and over.
That and one other tweak are noted with inline comments.
time.sleep(60) | ||
break | ||
|
||
# Loop infinitely until its time to re-poll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extra loop that is printing the results over and over again until it's time to fetch new data confused me.
At first I thought it was actually retching new data all the time and worried about rate limit problem with the server, but I did eventually realize it's not actually fetching, just re-printing the same data in a loop while it waits for the next time to fetch.
I'm in favor of removing that loop and just printing the results once. By printing them over and over it feels like it's implying to the user that there is new data to see when it's actually just printing the same data again. It also makes it a little hard to hone in on a specific ride if they are interested in one. (Though they of course could modify it to print a single one, granted.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FoamyGuy Yes, that is specifically the way they requested it to behave.
I suppose printing a "last updated" timestamp would make sense but would take up precious screen real estate. It makes less sense on a huge 3.5" TFT.
The project intention is for a portable SSD1306 with a lipo battery on a watch band. In the real script it will attempt to reconnect (via a tethered phone hotspot) every 5 minutes. Perhaps I could improve upon the code comments to specify that's the intention of this particular example?
If you gave one device to each kid and it only worked within range of the hotspot it could ensure that your kids will want to stay close enough to ensure the device works so they get ride updates too. It's a brilliant incentive for your kids to want to stay close to you in a theme park.
Maybe change the text color after an update so that every new update is unmistakably new? That would serve as an indicator of new content without taking up any screen real estate but a display is not included in this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still in favor of removing the looping behavior from the example here. That sounds like a neat project and they can definitely still use it that way if they'd like. I could also see a world where someone might want to "favorite" certain rides to get information about them more prominently or even some alerts based on the data returned.
I believe that in it's usage as an example here the continued printing of the same data over and over makes it more confusing because it's difficult to tell what is happening and when there is actually new data. None of the rest of the examples in this repo behave that way, they're all either "single shot" fetching once and ending, or repeating in a loop that fetches every X minutes or seconds and prints the current data then waits until it's time to run again. I believe it's best to have this example match that behavior and only print the data once after it's been fetched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FoamyGuy sounds good. I have the example that the person wanted in my personal repo, and linked them there, and they already have that one. Turning it into a single use "code done running" sounds good to me.
I think removing the extra infinite looping of the printouts when there isn't new data is the only thing left to change on this one. Everything else is looking good to me at this point. If you'd prefer I can work on that change and commit it here, let me know. |
@FoamyGuy I missed working on this one last night sorry! If you would like to do it go for it. I have bursted irrigation pipes to deal with for the next couple of days. Not a water main but something that will cost me money if I don't address immediately. Prior to last week I was the only person working on these examples. It's really really nice to see others take an interest in helping to clean them up. By all means go for it! Will take a load off my shoulders. |
try: | ||
qtimes_response = requests.get(url=QTIMES_SOURCE) | ||
qtimes_json = qtimes_response.json() | ||
except ConnectionError as e: | ||
print("Connection Error:", e) | ||
print("Retrying in 10 seconds") | ||
print(" | ✅ Queue-Times JSON!") | ||
|
||
DEBUG_QTIMES = False | ||
if DEBUG_QTIMES: | ||
print("Full API GET URL: ", QTIMES_SOURCE) | ||
print(qtimes_json) | ||
qtimes_response.close() | ||
print("✂️ Disconnected from Queue-Times API") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try: | |
qtimes_response = requests.get(url=QTIMES_SOURCE) | |
qtimes_json = qtimes_response.json() | |
except ConnectionError as e: | |
print("Connection Error:", e) | |
print("Retrying in 10 seconds") | |
print(" | ✅ Queue-Times JSON!") | |
DEBUG_QTIMES = False | |
if DEBUG_QTIMES: | |
print("Full API GET URL: ", QTIMES_SOURCE) | |
print(qtimes_json) | |
qtimes_response.close() | |
print("✂️ Disconnected from Queue-Times API") | |
try: | |
with requests.get(url=QTIMES_SOURCE) as qtimes_response: | |
qtimes_json = qtimes_response.json() | |
except ConnectionError as e: | |
print("Connection Error:", e) | |
print("Retrying in 10 seconds") | |
print(" | ✅ Queue-Times JSON!") | |
DEBUG_QTIMES = False | |
if DEBUG_QTIMES: | |
print("Full API GET URL: ", QTIMES_SOURCE) | |
print(qtimes_json) |
Using Justins suggested changes
@FoamyGuy ready for another review. removed infinite loop and added Justins suggested change to use with. Cleaned up serial prints to be more legible. Since Disneyland (Orlando) is currently open it's populating with actual queue times.
etc... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
I committed one final tweak to remove the call to close()
since the with context handles it now and to move the printout inside of the try block since we won't have any data to print if the connection failed and removed the sleep() calls to make it print them all at once.
Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 3.2.8 from 3.2.6: > Merge pull request adafruit/Adafruit_CircuitPython_Requests#182 from DJDevon3/DJDevon3-QueueTimesAPI > Merge pull request adafruit/Adafruit_CircuitPython_Requests#178 from DJDevon3/DJDevon3-WifiAdvanced Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
Used for Disney theme park queue times. Public API. Runs infinitely until time to update. The way this one works is slightly different from most examples. It's designed for a small display to take to the theme park and get updates on queue times.
Here's an example on a display.
Inspired by Reddit user Fishing_Quiet asked for help getting this API working on a display.