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

#94 Downloads failing immediately correctly update UI #104

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

Conversation

Chralu
Copy link
Contributor

@Chralu Chralu commented Sep 15, 2023

Fixes #94

Add a Lock to DownloadService. This ensures that download status updates are not processed while a download start/stop is requested (they are processed right after).

@Chralu Chralu force-pushed the fix/94_Download_failing_immediately_doesnt_update_UI branch 3 times, most recently from 63e7f66 to c819a1e Compare September 17, 2023 07:24
@amugofjava
Copy link
Owner

Hi @Chralu,

Thanks for this pr. I think this gets us part of the way there, but I need to revisit this section as there are still problems - for example when in airplane mode. If you start a download, it appears to cancel, but when you return it is queued but never gets any further.

Screen_recording_20230929_181525.webm

@Chralu Chralu force-pushed the fix/94_Download_failing_immediately_doesnt_update_UI branch 2 times, most recently from f03cbf9 to f11a99c Compare December 22, 2024 14:31
@Chralu
Copy link
Contributor Author

Chralu commented Dec 22, 2024

Hi @amugofjava ,

I've reworked this PR a bit to handle the case you described.

The issue was caused by an inconsistency between :

  • episode state stored in Sembast
  • episode state streamed through DownloadService, Bloc to UI

The fix mainly consists in using the standard download progress workflow when download initialization fails.

…tes are not processed while a download start/stop is requested (they are processed right after).
@Chralu Chralu force-pushed the fix/94_Download_failing_immediately_doesnt_update_UI branch from 7b81651 to d6277c5 Compare December 26, 2024 13:04
@Chralu Chralu force-pushed the fix/94_Download_failing_immediately_doesnt_update_UI branch from d6277c5 to 75036f2 Compare December 26, 2024 13:21
@amugofjava
Copy link
Owner

amugofjava commented Jan 15, 2025

Hi @Chralu,

Thanks for the updated PR.

Moving the download parts fully into the DownloadService makes sense; however, doing so has introduced a regression. Previously, hitting download would change the status to DownloadState.queued and push the update to the stream. What happens now is hitting download calls downloadEpisode but the status is not updated until we receive an update from the manager. The end result is it takes a while for the UI to update.

If you take a look at the capture below that shows current behaviour:

download_before.mp4

And compare to the behaviour in the PR:

download_after.mp4

There is a noticeable delay between hitting download and the UI being updated.

I think it should be an easy fix. The episode stream needs updating before the download process kicks in.

@Chralu
Copy link
Contributor Author

Chralu commented Jan 15, 2025

Hi @amugofjava ,

thanks for the review !
I don't have much time to look at it for now, but I will check how to fix that behavior in the next weeks.

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.

Download failing immediately doesn't update UI
2 participants