Progress Sync does not automatically push/pull on Kobo Sage #10539
Comments
|
I think I already tried to fix this one in #6489. but that resulted in some unforeseen consequences and eventually it was reverted. I believe it was related to device freezing for a while, when the network was connecting. In any case, you can try modifying the plugin like I did in #6489 and see if it works for you |
|
I don't use the plugin so have subzero interest in this, but, yeah, it's not necessarily easy to do in a way that won't adversely affect UX (or even... stability on some boards), because WiFi is terrible. |
|
The resume part of the equation could admittedly be fixed rather simply now by relying on onNetworkConnected + restore wifi on resume; but the suspend part is trickier (any event the plugin get will be after we've killed wifi, IIRC). |
|
On platforms using WifiManager, you'd essentially have to create a new "WifiDisconnecting" event, fire that early in all the right places, and hook that instead of suspend. |
|
So I'm not a Lua dev and am not familiar with KOReader APIs, but it looks to me like resume already should be relying on koreader/plugins/kosync.koplugin/main.lua Line 811 in 18cd6b7 (And clearly it used to work at some point not that long ago. Looking at that plugin, nothing relevant seems to have been changed in years. Did something change about how that function works?) |
|
That at least looks sound at a glance, yeah. You'll want to check verbose debug logs to double-check, because what's attached in the OP is not detailed enough to describe the actual sequence of events. |
|
I'd expect to see an initial failure from onResume, and a successful one later from onNetworkConnected. There might also be async funtimes happening on the Sage because it's SMP, where it might be detected as connected a tad earlier than on UP systems... |
|
And, yeah, the connectivity check was updated in #10098 and #10062 TL;DR: It used to require a successful gateway ping to assume connected, now it does so as "soon" (air quotes because the check has a 2s granularity) as the kernel assigns the interface an IP address. This should be mostly irrelevant, as the plugin adds another 1s delay after that, but one never knows... In any case, can't say what's actually happening on your end without verbose debug logs ;). |
|
(Dev nit: would be nice if the plugin also chucked the sync status recap to the logs instead of an InfoMessage only ;o)). |
|
Also, yeah, the delay issue would still be a massive PITA. While I can sortof deal with disconnections properly, handling push on suspend is another kettle of fish entirely, since in most cases wifi will be off at suspend time (and not in the technical sense of the events firing too late, but in the sense that the network activity timeout will most likely have murdered wifi long ago). If I were to try to build on #6489, I'd need to do it in a way that doesn't show parts of the UI on re-connection (which I'm extremely not a fan of on principle), which would add weird silent delays of varying lengths depending on connection stability... and would still risk popping up weird errors in case of actual errors. I... really don't see a good way to handle that on devices without "always on" wifi. (Unless we go the extreme overengineering way and delay the push after suspend, possibly during the grace time, or via an rtc wakeup? That would still require silent connection setup, though...). |
|
The other solution would be to show a nag popup on suspend explicitly asking the user to allow the connection setup to handle the push, which would personally make me disable the feature so fast you couldn't even blink fast enough; but which I nonetheless "like" better than the other approaches I can think of (on principle, at least). |
|
I don't actually use the feature (at all), so this is very much a Request For Comments from people that do (on the affected devices, i.e., not Android). |
|
Yeah, I hate the popup on suspend idea. Like, if I turn the thing off (or let it sit on a couch until it times out and goes into suspend), there's no world in which I'd want to have to deal with the UI and wait for it to do something. But I guess I'm not understanding why you hate the silent connection so much? Like, my mental model would be that once I've connected to a network and not deliberately, intentionally disconnected from it, if the system silently disconnected for power-saving/timeout-y/signal strength reasons, it would also silently re-connect without my having to do anything. |
Because it's doing stuff behind your back. Note that, if all goes well (unless your before wifi action is actually prompt, which is not great even outside of this use-case (note to self: check that this isn't actually the default :D)), you'll just see a bunch of InfoMessages come and go showing you that stuff is happening sortof behind your back, with no actual user interaction required; just delays. |
I knew there was a reason I thought of that... and that's because it is ;D. (But, again, as much as I hate it, it makes sense as a conservative default). Things ought to be much smoother with a sane (i.e., "turn on") before wifi setting. |
So I understand why you're thinking of it that way, but I wonder if this is a case where expectations of people using sync might be different. Because from my perspective, when I initially connect to Wifi, my intention is to connect to Wifi forever. I connected for the express purpose of having that connection there to do any needed background syncing, both pushing and pulling. The ideal experience for me is that the Wifi never, ever, ever disconnects and just stays live. Now, I understand why for power management reasons, the Wifi needs to be turned off sometimes. But to me, the ideal implementation would be one that makes that pragmatic wifi connection management as invisible as possible: If it got turned off through a system-initiated action (timeouts, suspend, whatever), it should silently turn itself back on as much as possible so that I don't even notice it got turned off. Whereas I think if you were using it for non-sync purposes, it would feel more natural to turn Wifi on, do some network-involved stuff, and then when you're done at some point, it drops the connection and keeps it dropped until you need to reconnect, and it makes it clear and loud that it's doing so. |
Hm? Seems fine to me. Connecting to wifi is a "dangerous" action (i.e., things can freeze for a while)[1] and it's by far the biggest battery hog. [1] And as I once wrote in our UI guidelines, "If it would take the user more than half a minute to recover from a mistake, a "Cancel" button must be added to the dialog." (Source). That wasn't written with simply having to wait for half a minute in mind, but potato potato. ;-) Anyway, that is to say it may not be ideal but it doesn't seem bad either. |
|
@Frenzie: Yup, agreed, which is why I'm fine with it as a default. |
|
@mkozlows: To make a long story short, I think that would be a slippery slope in principle (and a lot of code complexity for not much) ;). The good news is that, In practice, I imagine sync people actually enable periodic sync, too; and that, hopefully, should be enough to just keep the connection alive, making the whole thing mostly moot ;). The best news is that modern WiFi chips are actually pretty good at power managing themselves while on, so the battery hog syndrome isn't as marked as it used to be. |
…ore reliable (#10605) Fix: #10539, and for context #6489, #6733, #6534 Reorganize and reword most of the settings to make it clear what actually ties into auto sync, and what doesn't. (Specifically, what happens when a pull attempts to sync forward or backward has nothing to do with auto sync, it applies in all cases; while the periodic sync *does* require auto sync). The main point of contention, though, is that auto sync will now *always* attempt to setup network connectivity (i.e., on resume/suspend/close). Periodic sync will *not* though (the intent being that, if you use periodic sync, you're relying on the activity check to actually keep wifi on at all times)). Since this may lead to a large amount of nagging about wifi toggles on devices w/ NetworkManager support, it is now *disabled* by default on those devices. (And given that it wouldn't have worked because of the lack of connectivity, that doesn't really make any practical difference ;p). Additionally, given the fact that there's no way to make this behavior viable if the "before wifi" action is left at its default of "prompt", this feature now *requires* that to be set to "turn_on" (on devices where it can, of course); attempting to toggle it on will warn about that if necessary. This change is retroactive (OTM). Includes an assortment of fixes and cleanups, including migrating to the new LuaSettings API, which is why there's no longer a smattering of superfluous flushes.
Using 2023.05.1 on a Kobo Sage, everything about Progress Sync works, except that it doesn't automatically push progress when I put it in suspend, nor pull progress when it resumes. If I do a manual push/pull in those situations, it works properly. The same version running on my Pixel 7 does work properly.
I have Progress Sync set to "Auto sync now and future" and Wi-Fi set to "Restore Wi-Fi connection on resume".
I'm attaching the crash.log (the relevant bit is right at the end); it looks to me like the problem is that it shuts down Wifi before it tries to push, and then tries to pull before it's brought Wifi fully back up.
crash.log
The text was updated successfully, but these errors were encountered: