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

Use ConnectionManager #151

Merged
merged 16 commits into from
Feb 27, 2024
Merged

Conversation

justmobilize
Copy link
Collaborator

@justmobilize justmobilize commented Jan 23, 2024

Update adafruit_requests to use Adafruit_CircuitPython_ConnectionManager for socket handling. As well as clean up examples

@justmobilize justmobilize changed the title Connection manager Use ConnectionManager Feb 21, 2024
@justmobilize
Copy link
Collaborator Author

Updated the RTD example section to have all 4 boards listed:
image

@justmobilize
Copy link
Collaborator Author

@tannewt asking here, just so we have record of the conversation.

The code that is currently here which is re-worked from the old set_socket to set a global instance - Is this something we want to keep?

Since set_socket is gone, everyone will still need to move to passing in a socket_pool and ssl_context and thus aren't making anything easier by leaving this here.

@justmobilize justmobilize marked this pull request as ready for review February 22, 2024 17:23
@justmobilize
Copy link
Collaborator Author

@tannewt I'm also moving this to ready for review. I would like to add some more tests before it merges one we know what's what

@tannewt
Copy link
Member

tannewt commented Feb 22, 2024

@tannewt asking here, just so we have record of the conversation.

The code that is currently here which is re-worked from the old set_socket to set a global instance - Is this something we want to keep?

Since set_socket is gone, everyone will still need to move to passing in a socket_pool and ssl_context and thus aren't making anything easier by leaving this here.

Ya, let's just remove it in favor of Session().

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

One variable name I don't want to see copy and pasted more places. Looks good otherwise.

raise
# Get WiFi details, ensure these are setup in settings.toml
ssid = os.getenv("CIRCUITPY_WIFI_SSID")
appw = os.getenv("CIRCUITPY_WIFI_PASSWORD")
Copy link
Member

Choose a reason for hiding this comment

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

What is appw for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just standardized in what there was the most of. Assumed it was an internal preference. Can change them all to password or anything else that makes sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ahhh- AP PassWord

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer password. This is the only other change from me.

@justmobilize
Copy link
Collaborator Author

@tannewt asking here, just so we have record of the conversation.
The code that is currently here which is re-worked from the old set_socket to set a global instance - Is this something we want to keep?
Since set_socket is gone, everyone will still need to move to passing in a socket_pool and ssl_context and thus aren't making anything easier by leaving this here.

Ya, let's just remove it in favor of Session().

@tannewt after removing and adding tests, any other changes?

@justmobilize
Copy link
Collaborator Author

@tannewt this is ready for review again. Coverage is at 85%, I tackled what was easy and added some fixtures to remove boilerplate. Will add more tests in a future pass.

image

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

@tannewt tannewt merged commit cd34a7e into adafruit:main Feb 27, 2024
1 check passed
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 27, 2024
Updating https://github.com/adafruit/Adafruit_CircuitPython_ADXL37x to 1.1.9 from 1.1.8:
  > Merge pull request adafruit/Adafruit_CircuitPython_ADXL37x#5 from FoamyGuy/fix_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 7.0.0 from 6.1.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#188 from justmobilize/remove-set-socket

Updating https://github.com/adafruit/Adafruit_CircuitPython_GUVX_I2C to 1.0.7 from 1.0.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_GUVX_I2C#5 from FoamyGuy/fix_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_OV5640 to 1.1.16 from 1.1.15:
  > Merge pull request adafruit/Adafruit_CircuitPython_OV5640#30 from FoamyGuy/fix_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8574 to 1.0.9 from 1.0.8:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCF8574#8 from FoamyGuy/fix_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8575 to 1.0.5 from 1.0.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCF8575#7 from FoamyGuy/fix_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_SPD1656 to 0.9.0 from 0.8.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_SPD1656#7 from prcutler/root-group-fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_ST7735 to 1.2.13 from 1.2.12:
  > Merge pull request adafruit/Adafruit_CircuitPython_ST7735#19 from prcutler/root-group-fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_UC8151D to 1.3.3 from 1.3.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_UC8151D#13 from FoamyGuy/fix_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_VEML7700 to 2.0.0 from 1.1.22:
  > Merge pull request adafruit/Adafruit_CircuitPython_VEML7700#26 from standsi/SetInitialLightGain

Updating https://github.com/adafruit/Adafruit_CircuitPython_VL53L1X to 1.1.14 from 1.1.13:
  > Merge pull request adafruit/Adafruit_CircuitPython_VL53L1X#16 from analogic/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_VL53L4CD to 1.2.0 from 1.1.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_VL53L4CD#9 from markkamp/measurement_quality

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_Beacon to 1.0.5 from 1.0.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_Beacon#3 from FoamyGuy/fix_circup_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_PortalBase to 2.0.0 from 1.16.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PortalBase#100 from justmobilize/remove-set-socket

Updating https://github.com/adafruit/Adafruit_CircuitPython_Radial_Controller to 1.0.12 from 1.0.11:
  > Merge pull request adafruit/Adafruit_CircuitPython_Radial_Controller#3 from FoamyGuy/fix_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 3.0.0 from 2.0.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#151 from justmobilize/connection-manager

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
@justmobilize justmobilize deleted the connection-manager branch February 29, 2024 17:16
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

2 participants