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

Add httpserver simpletest with connection manager #91

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

DJDevon3
Copy link
Contributor

Only handles setup for socketpool. Coming from requests examples I'm more used to seeing connection manager handle the pool now. It might be good for consistency to role out connection manager for all examples but I'll leave that up to you. It will be nice to at least have 1 example that uses Connection Manager in there.

Only handles setup for socketpool. Coming from requests examples I'm more used to seeing connection manager handle the pool now. It might be good for consistency to role out connection manager for all examples but I'll leave that up to you.
@michalpokusa
Copy link
Contributor

michalpokusa commented Mar 27, 2024

Is connection manager now the intended way of handling socket pools? Like settings.toml that replaced secrets.py.

If yes, I could make a PR that changes all examples and add explanation to docs.

@dhalbert
Copy link
Contributor

Yes! @justmobilize, would you agree?

@justmobilize
Copy link

@dhalbert and @michalpokusa - yes ConnectionManager is now the way to handle socket pools.


# Initalize Wifi, Socket Pool, Request Session
pool = adafruit_connection_manager.get_radio_socketpool(wifi.radio)
ssl_context = adafruit_connection_manager.get_radio_ssl_context(wifi.radio)

Choose a reason for hiding this comment

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

Since ssl_context isn't needed it doesn't need to be defined here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justmobilize Thank you, updated.

@michalpokusa
Copy link
Contributor

Ok then, later this week I will update all the examples and docs.

Changed ssid and password env's to match the simpletest_manual example.
from adafruit_httpserver import Server, Request, Response

# Get WiFi details, ensure these are setup in settings.toml
ssid = os.getenv("WIFI_SSID")

Choose a reason for hiding this comment

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

@michalpokusa, which env names do you want for examples in this repo? Should it not use the CIRCUITPY_* ones so it doesn't auto connect?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only the httpserver_simpletest_manual.py example uses env names at runtime, other examples utilize settings.toml with CIRCUITPY_* envs and auto connect feature.

Either way, if something is not consistent with other examples I will correct it in my PR, e.g. missing emphasised lines or description of example in docs.

Choose a reason for hiding this comment

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

I would probably use CIRCUITPY_*, where possible, since people playing with different things would most likely already have those on their devices...

Copy link
Contributor

Choose a reason for hiding this comment

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

They are only used in one example that clearly states that this is a manual approach. Every other example uses settings.toml, it is just to show there are multiple possibilities.

image

If I am correct, setting the CIRCUITPY_* envs also turns on the web workflow, which might not be desired.

Choose a reason for hiding this comment

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

Ahhh. Yeah and that was my thought, if the general ones are used, it auto connects, and thus it can be used for web workflow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I did the right thing by setting it back to how the manual example has it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure how I will order the examples. IMO right now it can stay like that and if it becomes necessary I will change that in my PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me 👍

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.

This looks good to me, I tested it successfully on a feather S3 TFT.

@FoamyGuy FoamyGuy merged commit 4cc9503 into adafruit:main Apr 1, 2024
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Apr 2, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SH1107 to 1.7.0 from 1.6.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SH1107#21 from FoamyGuy/displayio_9x_compatibility

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 2.0.1 from 2.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#41 from FoamyGuy/9x_compatibility

Updating https://github.com/adafruit/Adafruit_CircuitPython_EK79686 to 1.1.0 from 1.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_EK79686#1 from FoamyGuy/root_group_update

Updating https://github.com/adafruit/Adafruit_CircuitPython_HX8357 to 1.4.3 from 1.4.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_HX8357#22 from tyeth/adjust_setosc

Updating https://github.com/adafruit/Adafruit_CircuitPython_IL91874 to 1.4.0 from 1.3.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_IL91874#19 from FoamyGuy/9x_compatibility

Updating https://github.com/adafruit/Adafruit_CircuitPython_MLX90393 to 2.1.4 from 2.1.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_MLX90393#38 from caternuson/mux_fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_TCA8418 to 1.0.12 from 1.0.11:
  > Merge pull request adafruit/Adafruit_CircuitPython_TCA8418#14 from FoamyGuy/root_group_update

Updating https://github.com/adafruit/Adafruit_CircuitPython_BluefruitConnect to 1.2.14 from 1.2.13:
  > Merge pull request adafruit/Adafruit_CircuitPython_BluefruitConnect#39 from vin1953/example-url-fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 4.5.7 from 4.5.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#91 from DJDevon3/DJDevon3-CMSimpleTest

Updating https://github.com/adafruit/Adafruit_CircuitPython_MagTag to 2.2.10 from 2.2.9:
  > Merge pull request adafruit/Adafruit_CircuitPython_MagTag#92 from makermelissa/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_PyBadger to 4.0.0 from 3.8.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PyBadger#69 from FoamyGuy/root_group_prop

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 3.2.3 from 3.2.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#179 from FoamyGuy/remove_pylint_disable
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#175 from DJDevon3/DJDevon3-RocketLaunchLive
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#174 from DJDevon3/DJDevon3-Youtube

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.

None yet

5 participants