Skip to content
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

Revert "Kosync network callback" #6535

Merged
merged 1 commit into from Aug 20, 2020
Merged

Conversation

Galunid
Copy link
Member

@Galunid Galunid commented Aug 20, 2020

Reverts #6489 since it introduced problems for people having [*]Auto sync now and in the furture & Action when wifi off: prompt.


This change is Reviewable

@Frenzie Frenzie added this to the 2020.08.1 milestone Aug 20, 2020
@Frenzie Frenzie merged commit 85b498d into master Aug 20, 2020
@NiLuJe
Copy link
Member

NiLuJe commented Aug 20, 2020

Now that I'm back, remind me what was the issue that made you try this approach?

@Frenzie Frenzie deleted the revert-6489-kosync-network-callback branch August 20, 2020 11:11
@Galunid
Copy link
Member Author

Galunid commented Aug 20, 2020

@NiLuJe Kosync plugin doesn't adapt to your QoL wifi improvements, i.e. doesn't enable wifi when pushing/pulling progress and disabling it after. It just displays message along the lines of "Something went wrong, check your network"

@NiLuJe
Copy link
Member

NiLuJe commented Aug 20, 2020

@Galunid: On which device?

@NiLuJe
Copy link
Member

NiLuJe commented Aug 20, 2020

Ah, yeah, KOSync:getProgress and KOSync:updateProgress may be called without Wi-Fi (i.e., pagination).

There's not really much we can do, as they may be called at a lot of weird moments, and, the rerunWhenOnline stuff will (as it should) always show the relevant UI bits.

That said, on Kobo, with restore Wi-Fi on resume, it should behave properly in most cases, as it's frequent enough (I think?), that it should keep Wi-Fi alive.

@NiLuJe
Copy link
Member

NiLuJe commented Aug 20, 2020

Frequent enough given an appropriate DAUTO_SAVE_PAGING_COUNT. Since that's gone now, a better fix would probably to make that threshold an actual plugin setting, as per the TODO ;).

@Galunid
Copy link
Member Author

Galunid commented Aug 20, 2020

Yes, actually that's the reason I disabled Auto progress sync, since those weird times hung the device for ~30s. A better way to handle this would be to check if manual == true was passed when calling

function KOSync:getProgress(manual)
if not self.kosync_username or not self.kosync_userkey then
if manual then
promptLogin()
end
return
end
local KOSyncClient = require("KOSyncClient")
local client = KOSyncClient:new{
custom_url = self.kosync_custom_server,
service_spec = self.path .. "/api.json"
}

So if user called it, enable wifi, otherwise skip.

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.

None yet

3 participants