Skip to content

Introduce Non-blocking wifi scan for ESP32 #7526

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marcidy
Copy link
Contributor

@marcidy marcidy commented Jul 11, 2021

Scan for APs in the background and retrieve ap records with a new function call.

scan run without any arguments, or as scan(True) is backwards compatible and returns a list of AP records.

scan(False) runs a non-blocking scan, taking advantage of the blocking argument to esp_wifi_scan_start.

When the scan is complete, the records are available by calling wlan.get_scan_results().

When running a non-blocking scan, the results are stored in the wifi driver's dynamic memory. A call to esp_wifi_scan_get_ap_records is requires to free that memory.

While testing, i noticed that other calls to the driver (e.g. connect, deactivate, etc). interrupt the scan and emit a SYSTEM_EVENT_SCAN_DONE event. These appear to clear out the memory also, meaning no records are retrieved. I didn't add any extra behavior to free the driver's memory as the IDF seems to handle it as long as the driver's state is changed. e.g., the records don't hang around in memory if a connect is run after a scan, but before a call to esp_wifi_scan_get_ap_records.

wlan.get_scan_results has 2 behaviors:

  • If no SCAN_DONE event is emitted, it returns None. Either a scan is in progress or no scan was attempted.
  • If a SCAN_DONE event occurred, it will return a (possibly empty) list of records one time

I added a check against the record size as there are cases when a SCAN_DONE event is emitted but there are zero records returned, specifically when a scan was interrupted.

I didn't add anything to esp_state for scanning / scan_done since it wasn't strictly necessary, as the driver manages it's memory correctly imo. If the user want's to check if there are scan results, just call wlan.get_scan_results().

to test:

>>> import network
>>> wlan = network.WLAN(network.STA_IF)
>>> wlan.scan()
# list of APs as before
>>> wlan.scan(False)
>>> wlan.get_scan_results()
>>>
# scan finished
>>> wlan.get_scan_results()
# list of APs

To trigger edge cases which clear records:

>>> wlan.scan(False)
>>> wlan.connect('ssid', 'pass')
>>> wlan.get_scan_results()

* Adds SYSTEM_EVENT_SCAN_DONE handling to event_handler
* Extracts AP record retrieval to new function esp_get_scan_results
* Adds boolean argument to esp_scan, defaulting to blocking scan
* Non-blocking scan requires calling new function get_scan_results
* Adds check for zero records retrieved

Signed-off-by: Matt Arcidy <marcidy@gmail.com>
Copy link
Member

@jimmo jimmo left a comment

Choose a reason for hiding this comment

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

Thanks @marcidy !

I think this approach makes sense but I wonder if it would be better to try and do something callback-oriented instead:

def on_scan_complete(sta_if, results):
  ...

sta.scan()  # blocking scan
sta.scan(on_scan_complete)  # non-blocking

this would be easier to integrate with asyncio (i.e. just use a asyncio.Event to turn this into await async_sta.scan() ).

Roughly to implement this you'd need:

  • Add a mp_obj_t pointing to the callback. This would need to be a root pointer.
  • Set the callback in esp_scan()
  • Use mp_sched_schedule in event_handler to schedule a method that would then call esp_wifi_scan_get_ap_num etc and then call the registered callback. (Note the two-level thing here where you schedule an intermediate function rather than the user callback directly, you need to get into scheduler context before you can construct the results list).

@marcidy
Copy link
Contributor Author

marcidy commented Jul 12, 2021

Thanks for the review, @jimmo !

Makes sense, ill give it a shot in the next few days.

I'm not familiar with the term 'root pointer', and google isn't helpful. Is that a pointer declared outside any other scope in the file (e.g. at the top)?

Might need a bit of guidance later, but I think I'm clear on what's needed to implement this approach.

@jimmo
Copy link
Member

jimmo commented Jul 12, 2021

I'm not familiar with the term 'root pointer', and google isn't helpful. Is that a pointer declared outside any other scope in the file (e.g. at the top)?

Because MicroPython uses a mark & sweep garbage collector, it can only find things that are reachable from the "roots". When you assign the callback to a global C variable, the GC won't know about it, and therefore might collect the callback function.

See mpstate.h (and gc_collect_start in gc.c). In particular you will need to add it to the port-specific root pointers (see MICROPY_PORT_ROOT_POINTERS in esp32/mpconfigport.h).

@marcidy
Copy link
Contributor Author

marcidy commented Jul 12, 2021

got it, that's perfectly clear, thanks!

@marcidy marcidy changed the title Introduce Non-blocking wifi scan for ESP32 WIP Introduce Non-blocking wifi scan for ESP32 Jul 15, 2021
@marcidy marcidy marked this pull request as draft July 15, 2021 21:44
    Adds a new root pointer for a user-supplied callback function for a
    non-blocking AP scan.

Signed-off-by: Matt Arcidy <marcidy@gmail.com>
@marcidy marcidy force-pushed the non-blocking-wifi-scan branch 2 times, most recently from 1755596 to 8e0fd5f Compare July 16, 2021 03:40
@marcidy
Copy link
Contributor Author

marcidy commented Jul 16, 2021

@jimmo My latest commits have a callback working, but I have some questions before finishing:

  1. The callback you suggested included the sta_if in the signature.
def on_scan_complete(sta_if, results)

Currently I'm not doing that, as there isn't a direct way to pass sta_if (or self_in), since the initial scheduling is done from the event handler. Right now the callback takes just the results argument. I could add a struct to hold callback args set when scan is called, but wanted to double check as that adds some code that may not be necessary depending on your answer.

edit: Clearly the interface would be useful if I wanted to connect to a specific AP in the callback, but not necessarily the only way to do it. I think I can use the wlan_sta_obj instead of passing through self, but I'm little stuck with mp_sched_schedule only taking one arguement. I could pack the interface and results into a tuple and pass that? would still change the desired callback signature

  1. This looks a bit goofy to me:
178                 mp_sched_schedule(MP_OBJ_FROM_PTR(&esp_wifi_scan_cb_obj), mp_const_none);

This was since mp_sched_schedule needs instances, and I decided to use a function rather than a class (e.g. like machine_pin_irq). And it's possible I'm missing something subtle about constructing a python function and calling it.

edit: I found instances similar to this in other ports, so it's not as strange as i first thought.

  1. Do I need to add anything else to allow async compatibility?

Thanks!

example:

>>> import network
>>> sta = network.WLAN(network.STA_IF)
>>> sta.active(1)
True
>>> sta.scan()
# scan results
>>> 
>>> def cb(results):
...     print("Callback!")
...     for ap in results:
...         print(ap)
...         
...         
... 
>>> sta.scan(cb);print("not blocking!")
not blocking!
>>> Callback!
# scan results

    * Add optional callback to esp_scan to triggers a non-blocking scan
    * Schedule AP results retrieval when a SCAN_DONE event is emitted
    * Schedule user callback and pass it the results

Signed-off-by: Matt Arcidy <marcidy@gmail.com>
@marcidy marcidy force-pushed the non-blocking-wifi-scan branch from 8e0fd5f to 89caebb Compare July 21, 2021 23:22
@marcidy
Copy link
Contributor Author

marcidy commented Jul 21, 2021

To use 2 args with a user-supplied callback, I made use of mp_call_function_2_protected which is used similar to mp_call_function_1_protected in modsocket callback handling. Works OK (see below).

I'm unclear what to do with an async.Event to see if it will work with asyncio, i haven't used Events in a way that it's clear to me. With the syntax you showed, I would expect scan to return something awaitable.

I'm also a bit unclear on initializing the callback in esp_initialize. I took that pattern from pin_irq callbacks. I suppose I could initialize it to None? I'm not sure what's best.

        memset(&MP_STATE_PORT(esp_wifi_scan_cb_handler), 0, sizeof(MP_STATE_PORT(esp_wifi_scan_cb_handler)));

example:

>>> def cb(sta_if, results):
...     ssid = `testssid`
...     pword = 'testpword'
...     for ap in results:
...         if ap[0].decode('ascii') == ssid:
...             sta_if.connect(ssid, pword)
...             
...             
... 
>>> 
>>> sta.isconnected()
False
>>> sta.scan(cb);print("non-blocking")
non-blocking
>>> sta.isconnected()
True

@marcidy marcidy changed the title WIP Introduce Non-blocking wifi scan for ESP32 Introduce Non-blocking wifi scan for ESP32 Jul 21, 2021
@marcidy marcidy marked this pull request as ready for review July 21, 2021 23:40
@UnexpectedMaker
Copy link
Contributor

This is cool @marcidy ! @jimmo is this on your short list? I sure hope so!

@stephanelsmith
Copy link
Contributor

Just checking in on the question, and wow awesome updates! Can't wait to give this a try, especially with asyncio compatbility.

@curlyz
Copy link

curlyz commented Sep 18, 2022

I realized that this PR is not merged, but it doesn't matter that much since we can still combine asyncio and call this wifi.scan() in _thread

tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Feb 1, 2023
Add front buttons as D0/1/2, matching the silk
@stephanelsmith
Copy link
Contributor

I think about this PR periodically. Would love to see callback based non-blocking wifi scan for esp32. Any chance of brining this one back into the fold?

@projectgus
Copy link
Contributor

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants