Skip to content

Stop scanning if wpa_supplicant state goes INACTIVE.#13

Merged
NiLuJe merged 1 commit into
koreader:masterfrom
projectgus:bugfix/stop_scan_if_inactive
Oct 15, 2024
Merged

Stop scanning if wpa_supplicant state goes INACTIVE.#13
NiLuJe merged 1 commit into
koreader:masterfrom
projectgus:bugfix/stop_scan_if_inactive

Conversation

@projectgus

@projectgus projectgus commented Oct 12, 2024

Copy link
Copy Markdown
Contributor

This happens after the first set of results and the first CTRL-EVENT-NETWORK-NOT-FOUND if there are no preferred networks configured in wpa_supplicant - i.e. it considers the scan done and stops.

Closes #12.

Have implemented to call it on every iteration where nothing else happens because calling getStatus() seems more or less instant, and seems possible that some other versions may choose to go inactive at other times.

However, AFAIK if a preferred network is configured in wpa_supplicant then it won't go inactive - will keep scanning instead.

On my Kobo Aura the tested behaviour of scanThenGetResults() is:

  • No Wi-Fi configured in Nickel - master branch takes 20 seconds to return, this PR takes 1-2 seconds (with same scan results).
  • Wi-Fi configured in Nickel & connectable - connects in 1-2 seconds (no change with this PR).
  • Wi-Fi configured in Nickel & not connectable - returns after 20 seconds with scan results (no change with this PR).

This change is Reviewable

This happens after the first set of results and the first
CTRL-EVENT-NETWORK-NOT-FOUND if there are no preferred networks configured
in wpa_supplicant - i.e. it considers the scan done and stops.

Have implemented to call it on every iteration where nothing else happens
because calling getStatus() seems more or less instant, and seems possible
that some other versions may choose to go inactive at other times.

However, AFAIK if a preferred network is configured in wpa_supplicant then
it won't go inactive - will keep scanning instead.
@NiLuJe

NiLuJe commented Oct 12, 2024

Copy link
Copy Markdown
Member

The INACTIVE state seems pretty squarely designed for that "no enabled networks in the config" case, so that sounds good to me ;).

(c.f., https://w1.fi/wpa_supplicant/devel/defs_8h.html#a4aeb27c1e4abd046df3064ea9756f0bc).

Given that, I tried moving the status check to the closest related event (i.e., the final one), but it's not really any prettier, so, yeah, probably better this way.

diff --git a/wpaclient.lua b/wpaclient.lua
index 5582200..e852d97 100644
--- a/wpaclient.lua
+++ b/wpaclient.lua
@@ -220,8 +220,15 @@ function WpaClient.__index:scanThenGetResults()
             -- It may take *multiple* scans, and events may be split across multiple reads...
             -- Which is why NetworkManager does another pass of waiting in case our heuristics fail...
             elseif ev.msg == "CTRL-EVENT-NETWORK-NOT-FOUND" then
-                found_result = false
-                expected_scans = expected_scans + 1
+                local status
+                status, err = self:getStatus()
+                if status ~= nil and status.wpa_state ~= "INACTIVE" then
+                    -- If there are no enabled networks in the configuration, wpa_supplicant won't rescan,
+                    -- and switches to the INACTIVE state.
+                    -- c.f., https://w1.fi/wpa_supplicant/devel/defs_8h.html#a4aeb27c1e4abd046df3064ea9756f0bc
+                    found_result = false
+                    expected_scans = expected_scans + 1
+                end
 
             -- Wait for every started scan to finish
             -- NOTE: Because everything is terrible, this event isn't sent on older devices... -_-"

(Plus, we miiight want to re-use that check in the future anyway).

@NiLuJe NiLuJe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @projectgus)

@NiLuJe

NiLuJe commented Oct 12, 2024

Copy link
Copy Markdown
Member

(Plus, we miiight want to re-use that check in the future anyway).

Say, abort early on COMPLETED or some such ;).

@projectgus

Copy link
Copy Markdown
Contributor Author

Given that, I tried moving the status check to the closest related event (i.e., the final one), but it's not really any prettier, so, yeah, probably better this way.

Yeah, I actually started out with it in a similar place but thought it was a little cleaner at the bottom of the loop! I also figured that I didn't know 100% what sequence of events every wpa_supplicant version might produce, so adding the check on every loop means it'll always catch it if it goes INACTIVE regardless of what event(s) come first.

(Plus, we miiight want to re-use that check in the future anyway).

😄

@NiLuJe NiLuJe merged commit 0d5c8ee into koreader:master Oct 15, 2024
@NiLuJe

NiLuJe commented Oct 15, 2024

Copy link
Copy Markdown
Member

Thanks!

NiLuJe added a commit to NiLuJe/koreader-base that referenced this pull request Oct 22, 2024
Frenzie pushed a commit to koreader/koreader-base that referenced this pull request Oct 23, 2024
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.

Scan taking a full 20 seconds on Kobo Aura

2 participants