adafruit / Adafruit_CircuitPython_ESP_ATcontrol

Use the ESP AT command sent to communicate with the Interwebs
MIT License
20 stars 17 forks source link

Changes to support WPA Enterprise mode #65

Closed scogswell closed 1 year ago

scogswell commented 1 year ago

Added support for connecting via WPA Enterprise (AT+CWJEAP command) which is in the newer firmware for espressif AT commands. Tested with PEAP mode using stock espressif firmware built with enterprise option on loaded onto an adafruit Qt Py esp32-c3. I've included some examples in /fork-examples based on the original examples but in enterprise mode.

Added in support for using the AT+CWSTATE and and AT+CIPSTATE commands which replace AT+CIPSTATUS, espressif has marked AT+CIPSTATUS as deprecated. Still uses CIPSTATUS for boards that don't support CWSTATE (ie - Invector Labs Challenger RP2040 Wifi which has firmware 2.1 on an ESP8285, which I've tested my changes with). You can force using AT+CIPSTATUS with a flag in the constructor even if the firmware supports CWSTATE. New property functions and status messages are assigned for CWSTATE/CIPSTATE since the numerical codes aren't compatible with the CIPSTATUS syntax.

Some changes for soft_reset() to make it more available, and a reset checks for the "ready" message. My testbed qt py esp32-c3 doesn't have a hard reset line. Functions that called hard_reset() can now optionally call soft_reset() as well. neopixel status shown during reset with wifimanager.

adafruit_requests seems to try to reuse the old socket on some criteria this library doesn't support, so some of the examples always did a reset after one pass of the loop. Re-assigning the socket in wifimanager's get/push/post etc fixes that.

an instance in socket_connect() where SSL sockets were getting ignored (in example esp_atcontrol_countviewer.py the commented out requests that used ssl would fail).

Added functions for wifi disconnect and wifi autoconnect (whether to connect to a stored wifi AP on powerup).

bablokb commented 1 year ago

Your code is cluttered with _use_cipstatus-stuff. Wouldn't it be much simpler to hide this in the status-getter method? Also, doubling all constants does not seem to make sense to me. Maybe this can also be simplified.

scogswell commented 1 year ago

I've made changes to have the cipstatus/cwstate stuff confined to status, keeping most of the code looking like it did before I added in the enterprise stuff. In the cases where it can internally use cwstate/cipstate it returns a status code like what cipstatus does for compatibility.

bablokb commented 1 year ago

Good. We now have two pull requests changing partly the same code. Probably the second needs some work once the first is merged but I don't see any fundamental conflicts. We will see... I never had this case before so I can't tell what is the best strategy.

tekktrik commented 1 year ago

Would you mind merging/rebase to resolve the new conflicts? I can take a look and test afterwards.

scogswell commented 1 year ago

Sure thing, hopefully I did this right.

bablokb commented 1 year ago

Your pull-request does not pass the formal tests for formatting and linting. My experience is that you sometimes scratch your head if this is really necessary, but in the end this helps us all because it defines clear standards. There is a guide here: https://learn.adafruit.com/improve-your-code-with-pylint which tells you what to do. This mainly boils down to installing "black" and "pylint" (easy) and letting them test and fix your code.

tekktrik commented 1 year ago

Don't worry about the remaining pylint errors for now - we just updated our CI, so things aren't working fully yet and many of the remaining errors are bogus. I can ping you once we've worked out all the remaining issues

scogswell commented 1 year ago

Sure, I'll say that it passes muster for black and pylint locally on my machine using the .pylint that came in with the library. Thanks for your help.

tekktrik commented 1 year ago

Merged in CI changes from main