NetworkManager: Enable "before wifi action" support on every hasWifiToggle platform #10669

NiLuJe · 2023-07-10T01:26:51Z

It used to be limited to hasWifiManager platforms for some reason (spoiler: that reason was probably I couldn't be arsed to test it on Kindle, and/or it would have been clunky and messy before @yparitcher's cleanup of the network status checks).

Anyway, it's fairly trivial to do now, and, as #10605 showed, we kinda need it, as I was stuck in lala-land assuming everybody had it already, and KOSync is a broken mess without it ^^. (Plus, it really has no technical ties to any kind of NetworkMgr backend).

This involves some minor NetworkMgr refactors, and a few fixes along the way.

(Fix #10678)

To recap:

Enable before_wifi_action & after_wifi_action on hasWifiToggle platforms (which is basically all of 'em except naked SDL).
Decouple restoreWifiAsync from hasWifiManger, because we can do that on other platforms (namely, Kindle. Probably PB, too, but WiFi is already a mess there, and I can't test it).
Implement restoreWifiAsync on Kindle.
Properly flag rM as hasWifiManager & hasFastWifiStatusQuery, because it is actually both of those (it uses our wpa_supplicant backend).
Update the KOSync checks to take these changes into account, to properly disable auto_sync if necessary.
Really made the Network* event signaling consistent. For realz this time.
In an effort to make the whole beforeWifiAction framework somewhat usable there, we now assume connectivity is always available on !hasWifiToggle platforms...

This change is

NiLuJe · 2023-07-10T01:28:23Z

Lightly tested (on a 5.9.7 PW2), so if you have time to spare, feel free to give it a spin, @hius07 ;).

NiLuJe · 2023-07-10T01:29:38Z

As expected, the network activity check is hilarious on Kindle, because tmd is losing its damn mind in the background xD.

07/10/23-03:19:43 DEBUG NetworkListener: Significant network activity (delta: 19364 > threshold: 12 [ then: 1615 vs. now: 20979 ]) -> keeping Wi-Fi enabled

Ya don't say ;o).

yparitcher · 2023-07-10T02:06:51Z

frontend/ui/network/manager.lua

@@ -624,7 +634,8 @@ end
 function NetworkMgr:getPowersaveMenuTable()
    return {
        text = _("Disable Wi-Fi connection when inactive"),
-        help_text = _([[This will automatically turn Wi-Fi off after a generous period of network inactivity, without disrupting workflows that require a network connection, so you can just keep reading without worrying about battery drain.]]),
+        help_text = Device:isKindle() and _([[This is unlikely to function properly on a stock Kindle, given how chatty the framework is.]]) or


It works just fine for those of us that run frameworkless :)

Not over here (the numbers above were from a frameworkless session) ;o).

It's an old device that has almost never been online, which probably plays a part here, but even after letting it sit connected in the framework for a bit, which calmed tmd down somewhat, I still get deltas of ~360 (EDIT: ~250 today)..

I have iptables rules that only allows outgoing connections on the local lan (i dont need any helpful connections by amazon) so that is why it works for me

Fyi in case you have any other suggestions. https://hosted.weblate.org/translate/koreader/koreader/en/?checksum=4f0eae549f1bd665#comments

The English can probably be improved but "talkative, communicative..." is indeed correct. :-) It means the Kindle framework is constantly communicating with Amazon servers or something along those lines.

In Dutch I provisionally translated it as "sends/broadcasts much information," which would likely be clearer in English too.

Yeah, it's probably a bit jargony. I can't quite think of something else as succinct, though; so the alternative would be to explain it, i.e., that the native system already triggers a lot of network activity on its own.

Yes, that's probably clearer. "given how much network activity the framework generates" or something.

NiLuJe · 2023-07-10T02:48:01Z

(Draft, got some more stuff to test/fix tomorrow).

NiLuJe

Reviewed 7 of 10 files at r1, 1 of 2 files at r2, 1 of 3 files at r4, 2 of 2 files at r5, 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Frenzie, @pazos, @poire-z, and @yparitcher)

NiLuJe

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Frenzie, @pazos, @poire-z, and @yparitcher)

NiLuJe · 2023-07-11T02:09:35Z

Okay, re-tested on a PW2 & a H2O, this ought to be ready now ;).

NiLuJe

Dismissed @yparitcher from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Frenzie, @pazos, and @poire-z)

We can implement on some platforms without full manager support That also implies actually running it onResume via NetworkListener, instead of the Generic Power event handler...

That was hard.

checks in //

Although the fact that an implementation detail (i.e., that Kindle doesn't use a script) leaksthrough there is... not great.

Makes the report somewhat more accurate

NetworkConnected event...

We have a verbose script on ntx, but nothing on Kindle, for instance

Boy be chatty.

counterpart

I'm... not sure there was ever anything else, but I only have a vague recollection of writing this, and I've already fixed dumb things about it in this PR, so, who knows xD.

That was less painful than I expected (p4merge ftw!)

NiLuJe

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Frenzie, @pazos, and @poire-z)

The toggles are interactive there, so, that's not great... We'll see how NetworkMgr's connectivity check fares there, it might need to be nerfed on Android, too...

NiLuJe

Reviewed 1 of 1 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Frenzie, @pazos, and @poire-z)

Followup to koreader#10669, fix koreader#10694

Followup to #10669, fix #10694

* Kindle: Implement a NetworkMgr backend loosely based on WpaClient in order to allow feature-parity with hasWifiManager platforms. This involves dealing with the native wifid over lipc (the native IPC system, based on DBus), through custom Lua bindings (https://github.com/notmarek/openlipclua), since the stock ones lack support for the needed hasharray data type. * NetworkMgr: Clear up leftover hallucinations from #10669, making `enableWifi` much simpler (and much more similar to `turnOnWifiAndWaitForConnection`). * NetworkMgr: Made it clearer that `turnOnWifi` implementations *must* deal with `complete_callback`, as part of the aforementioned changes mean that it's *always* wrapped in a connectivity check, and we need that for proper event signaling. * Android, Emu: Run `complete_callback` properly in `turnOnWifi`. * Kindle: Support `powerd:isCharged()` on the PW2 (yes, this is random, it just happened to be my test device :D). * NetworkMgr:disableWifi: Properly tear down any potential ongoing connection attempt (e.g., connectivity check). * NetworkMgr:promptWifi: Make the "wifi enabled but not connected" popup clearer if there's an ongoing connection attempt, and gray out the "Connect" button in this case (as it would only lead to another "connection already in progress" popup anyway). * NetworkMgr:reconnectOrShowNetworkMenu: Make *total* scanning failures fatal (they will lead to an immediate wifi teardown). * NetworkMgr:reconnectOrShowNetworkMenu: Clear up the long-press behavior (which *always* shows the network list popup) so that it doesn't weirdly break all the things (technical term!). * NetworkMgr:reconnectOrShowNetworkMenu: When we manage to connect to a preferred network on our own *before* showing the network list, make sure it is flagged as "connected" in said list. * NetworkMgr:reconnectOrShowNetworkMenu: Make connection failures fatal in non-interactive workflows (they'll lead to a wifi teardown). * NetworkSetting (the aforementioned network list widget): Clear NetworkMgr's "connection pending" flag on dismiss when nothing else will (i.e., when there's no connectivity check ticking).

@benoit-pierre

so as not to break the beforeWifiction shenanigans... Which is most of them, only the Emulator subclass sets it, for... reasons. (That Emulator quirk is also why we can't simply scrap the whole thing to use NetworkMgr's default imps). Fix koreader#12203 (thanks to @benoit-pierre for spotting that one). This was clearly an oversight on my part when working on koreader#10669

@benoit-pierre

…s, (#12217) so as not to break the beforeWifiction shenanigans... Which is most of them, only the Emulator subclass sets it, for... reasons. (That Emulator quirk is also why we can't simply scrap the whole thing to use NetworkMgr's default imps). Fix #12203 (thanks to @benoit-pierre for spotting that one). This was clearly an oversight on my part when working on #10669

* Kindle: Implement a NetworkMgr backend loosely based on WpaClient in order to allow feature-parity with hasWifiManager platforms. This involves dealing with the native wifid over lipc (the native IPC system, based on DBus), through custom Lua bindings (https://github.com/notmarek/openlipclua), since the stock ones lack support for the needed hasharray data type. * NetworkMgr: Clear up leftover hallucinations from koreader#10669, making `enableWifi` much simpler (and much more similar to `turnOnWifiAndWaitForConnection`). * NetworkMgr: Made it clearer that `turnOnWifi` implementations *must* deal with `complete_callback`, as part of the aforementioned changes mean that it's *always* wrapped in a connectivity check, and we need that for proper event signaling. * Android, Emu: Run `complete_callback` properly in `turnOnWifi`. * Kindle: Support `powerd:isCharged()` on the PW2 (yes, this is random, it just happened to be my test device :D). * NetworkMgr:disableWifi: Properly tear down any potential ongoing connection attempt (e.g., connectivity check). * NetworkMgr:promptWifi: Make the "wifi enabled but not connected" popup clearer if there's an ongoing connection attempt, and gray out the "Connect" button in this case (as it would only lead to another "connection already in progress" popup anyway). * NetworkMgr:reconnectOrShowNetworkMenu: Make *total* scanning failures fatal (they will lead to an immediate wifi teardown). * NetworkMgr:reconnectOrShowNetworkMenu: Clear up the long-press behavior (which *always* shows the network list popup) so that it doesn't weirdly break all the things (technical term!). * NetworkMgr:reconnectOrShowNetworkMenu: When we manage to connect to a preferred network on our own *before* showing the network list, make sure it is flagged as "connected" in said list. * NetworkMgr:reconnectOrShowNetworkMenu: Make connection failures fatal in non-interactive workflows (they'll lead to a wifi teardown). * NetworkSetting (the aforementioned network list widget): Clear NetworkMgr's "connection pending" flag on dismiss when nothing else will (i.e., when there's no connectivity check ticking).

@benoit-pierre

…s, (koreader#12217) so as not to break the beforeWifiction shenanigans... Which is most of them, only the Emulator subclass sets it, for... reasons. (That Emulator quirk is also why we can't simply scrap the whole thing to use NetworkMgr's default imps). Fix koreader#12203 (thanks to @benoit-pierre for spotting that one). This was clearly an oversight on my part when working on koreader#10669

NiLuJe requested review from Frenzie, pazos and poire-z as code owners July 10, 2023 01:26

yparitcher reviewed Jul 10, 2023

View reviewed changes

NiLuJe marked this pull request as draft July 10, 2023 02:47

NiLuJe commented Jul 11, 2023

View reviewed changes

NiLuJe marked this pull request as ready for review July 11, 2023 02:09

NiLuJe commented Jul 11, 2023

View reviewed changes

NiLuJe added 17 commits July 11, 2023 16:07

Okay, that looks like a feasible gameplan....

9498638

The Emu is already flagges hasWifiToggle

198ed2b

Start updating the KOSync capchecks

a0cced7

Decouple restoreWifiAsync from hasWifiManager

9518e2f

We can implement on some platforms without full manager support That also implies actually running it onResume via NetworkListener, instead of the Generic Power event handler...

Implement restoreWifiAsync on Kindle

2293354

That was hard.

Add a crappy flag to potentially avoid running multiple connectvity

2d88d05

checks in //

Forgot that one

b509acd

Slightly more accurate check there

222f6c3

Although the fact that an implementation detail (i.e., that Kindle doesn't use a script) leaksthrough there is... not great.

Why, yes, rM is actually a first-class wireless citizen ;p

74a8d5a

Don't delay the first connectvity check for no reason

d9d5d2d

Makes the report somewhat more accurate

Simplify NetworkMgr init

a591b22

Eh, nextTick *should* be just fine for the initial, startup

f378cf9

NetworkConnected event...

Actively check the connectivity after a menu toggle on !ntx platforms

a9ca0c2

Add a tiny bit of debugging context to the async resume

f915d1d

We have a verbose script on ntx, but nothing on Kindle, for instance

Ditto on NetworkMgr's side

362ddbf

Log the time spent in goOnlineToRun, like the real connectivity check

b2b54d6

Tweak the connectvity check's help message on Kindle

506876c

Boy be chatty.

NiLuJe added 8 commits July 11, 2023 16:14

Stash that behind a hasWifiToggle capcheck, like its onPowerEvent

730c7ab

counterpart

Review pass

04402ad

Dedupe those handlers

e2f08ed

Clarify that comment

e228ed5

I'm... not sure there was ever anything else, but I only have a vague recollection of writing this, and I've already fixed dumb things about it in this PR, so, who knows xD.

Stray river

1659bc0

Comment tweak

b25c8f9

Harmless typo

9a68311

Post-rebase fixup

5ba0f9e

That was less painful than I expected (p4merge ftw!)

NiLuJe force-pushed the universal_before_wifi_action branch from d9e6687 to 5ba0f9e Compare July 11, 2023 14:17

NiLuJe commented Jul 11, 2023

View reviewed changes

NiLuJe mentioned this pull request Jul 11, 2023

"Error connecting to the network" when opening the file browser - KOSync trying to sync with Wi-Fi disabled? #10678

Closed

Skip the wifi teardown on suspend on Android

f1df419

The toggles are interactive there, so, that's not great... We'll see how NetworkMgr's connectivity check fares there, it might need to be nerfed on Android, too...

NiLuJe commented Jul 12, 2023

View reviewed changes

NiLuJe merged commit d57325a into koreader:master Jul 12, 2023

Frenzie added this to the 2023.07 milestone Jul 12, 2023

This was referenced Jul 14, 2023

/sbin/route deprecated - infinite attemps to connect to wifi on newer linux distributions #10087

Closed

Koreader OPDS and Wireless Calibre function keep asking for wifi prompt in WSL2 Linux #10694

Closed

NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Jul 15, 2023

NetworkManager: Bypass isOnline on !hasWifiToggle platforms

abd7df2

Followup to koreader#10669, fix koreader#10694

NiLuJe mentioned this pull request Jul 15, 2023

NetworkManager: Bypass isOnline on !hasWifiToggle platforms #10697

Merged

NiLuJe added a commit that referenced this pull request Jul 15, 2023

NetworkManager: Bypass isOnline on !hasWifiToggle platforms (#10697)

dacc3c2

Followup to #10669, fix #10694

This was referenced Jul 16, 2023

Release 2023.06.1 crashes when trying to go back to filebrowser from open document #10704

Closed

"Do you want to turn on wifi" after sleep #10730

Closed

Android: EOL versions and devices #10614

Closed

NiLuJe mentioned this pull request Jun 22, 2024

Kindle: Add wifi selector #12056

Merged

NiLuJe mentioned this pull request Jul 24, 2024

SDL: Return true in NetworkMgr:isConnected on !hasWifiToggle platforms #12217

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NetworkManager: Enable "before wifi action" support on every hasWifiToggle platform #10669

NetworkManager: Enable "before wifi action" support on every hasWifiToggle platform #10669

NiLuJe commented Jul 10, 2023 •

edited

Loading

NiLuJe commented Jul 10, 2023 •

edited

Loading

NiLuJe commented Jul 10, 2023

yparitcher Jul 10, 2023

NiLuJe Jul 10, 2023 •

edited

Loading

yparitcher Jul 10, 2023

Frenzie Jul 14, 2023

NiLuJe Jul 14, 2023 •

edited

Loading

Frenzie Jul 14, 2023

NiLuJe commented Jul 10, 2023

NiLuJe left a comment

NiLuJe left a comment

NiLuJe commented Jul 11, 2023

NiLuJe left a comment

NiLuJe left a comment

NiLuJe left a comment

NetworkManager: Enable "before wifi action" support on every hasWifiToggle platform #10669

NetworkManager: Enable "before wifi action" support on every hasWifiToggle platform #10669

Conversation

NiLuJe commented Jul 10, 2023 • edited Loading

NiLuJe commented Jul 10, 2023 • edited Loading

NiLuJe commented Jul 10, 2023

yparitcher Jul 10, 2023

Choose a reason for hiding this comment

NiLuJe Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

yparitcher Jul 10, 2023

Choose a reason for hiding this comment

Frenzie Jul 14, 2023

Choose a reason for hiding this comment

NiLuJe Jul 14, 2023 • edited Loading

Choose a reason for hiding this comment

Frenzie Jul 14, 2023

Choose a reason for hiding this comment

NiLuJe commented Jul 10, 2023

NiLuJe left a comment

Choose a reason for hiding this comment

NiLuJe left a comment

Choose a reason for hiding this comment

NiLuJe commented Jul 11, 2023

NiLuJe left a comment

Choose a reason for hiding this comment

NiLuJe left a comment

Choose a reason for hiding this comment

NiLuJe left a comment

Choose a reason for hiding this comment

NiLuJe commented Jul 10, 2023 •

edited

Loading

NiLuJe commented Jul 10, 2023 •

edited

Loading

NiLuJe Jul 10, 2023 •

edited

Loading

NiLuJe Jul 14, 2023 •

edited

Loading