esafirm / RxDownloader

An Rx wrapper for Download Manager in Android
MIT License
108 stars 32 forks source link

Progress listener #7

Open rahuldange09 opened 7 years ago

rahuldange09 commented 7 years ago

Is there any progress listener So i can show progress in my app.

ildar2 commented 7 years ago

Yeah, got the same question the first time I saw the code. Filepath in onNext call? Whaaa? Filepath should be set on request creation, onNext events should notify about progress, onCompleted should signal successful download.

esafirm commented 6 years ago

Hi sorry for the late response. This is due fetching progress data in DownloadManager is not straight-forward and maybe have a cost on performance.

I will try to support this on the next version

Kisty commented 6 years ago

Doesn't the BroadcastReceiver get progress updates anyway? Would need a lot of code I don't think. Not sure how to get progress as well as the resulting download path using one call. I guess you could return a Pair with both an observable to the resulting download and another `Observable (or Flowable?) for progress on the download. Would be very interested in this.

Btw, Happy Christmas!

esafirm commented 6 years ago

@Kisty i guess that is one way to do it. What i was thinking at the time was wrapping the result in a simple object or a Pair. And its definitely with different function/method or maybe a Transformer. WDYT?

Happy Christmas! (Too late ☚ī¸ )

mrmceduns commented 6 years ago

@esafirm Please when is the next version with the progress listener going to be released?. Awesome library btw! :)

Kisty commented 6 years ago

I think I've almost got a working version of using download progress as a PoC in our app. The broadcast receiver only gets Download completed events (boo) but running a thread and emitting results by polling. Haven't quite got it, but I think it's almost there. I ended up having a Flowable<Pair<Int, String>> to give progress with String being null when not complete, then String with path once completed. Wondering if it's better to return 2 Flowables (1 Flowable, 1 Single) one for progress, the other for the actual result. What are your thoughts, @esafirm & guys?

esafirm commented 6 years ago

@Kisty that's so cool 👍. I think we should have 2 variant for it.

  1. Single<String> to only receive the result
  2. Flowable<Pair<Int, String>> to receive the progress and the result.

Have you considering create an "event" data object as the substitute for Pair like RxBinding library?

Kisty commented 6 years ago

Yeah, I think 2 variants would be a good idea!

I'm not sure about the event data object you're referring to. Am looking at RxBinding but can't quite see it. Do you mean having something like Flowable<Pair<Int, String>> where Pair<Int,String> could be something like ProgressState which does the same thing as the Pair?

esafirm commented 6 years ago

Yep, something like that. I think It gave us more information than Pair right?

Kisty commented 6 years ago

@esafirm Cool. I'm not sure when I can work on this and give a PR but it's on my radar.

Kisty commented 6 years ago

@esafirm Also, the sample app could do with updating. We will need to find a file to download for the sample app. Any ideas? Perhaps the git zip of the repo else with nyan cat image, it will download without showing little progress as it'll be too quick.

esafirm commented 6 years ago

@Kisty noted. Well there is a website that provide test file for download with various size -> https://www.thinkbroadband.com/download

Kisty commented 6 years ago

@esafirm That's the file I ended up using. Almost got a PoC working though am having trouble in scheduling the download task and getting progress while it's downloading. I thought I got RxJava2 but now I'm not so sure. Any idea how to start a task on another thread (for polling the progress) and get an observable which has just the progress? I think I'm on the right track, though I'm not sure. Am using a PublishProcessor (instead of a PublishSubject because I think we may need backpressure, which is what Flowables have and Observables (in RxJava2) don't have backpressure) as the subject which is returned when calling downloadWithProgress() Flowable<Pair<Integer, String>> (Pair will be it's own DownloadProgress class instead).

Do you think we need backpressure, or is it not worth it? If not, we can just use PublishSubject instead (see https://github.com/ReactiveX/RxJava/wiki/What's-different-in-2.0#observable-and-flowable).

(see https://github.com/Kisty/RxDownloader/tree/feature/add-progress)

Kisty commented 6 years ago

@esafirm Managed to get a PoC working now. I reasoned that it didn't need to be Flowable as we define the polling interval (500ms) so backpressure won't be needed. Can you take a look at these commits? I don't think it's ready to be pulled in as I want to squish the wip commit with the main one.

Also, I've replaced the sample with a screen that just shows progress in the centre and removed the nyan cat download. Should I instead keep the nyan cat sample and have another screen demo'ing the progress, or have it on the same screen with 2 halves (one for nyan cat, another to show progress)

Kisty commented 6 years ago

@esafirm have you managed to take a peek at the PR, yet? #18

esafirm commented 6 years ago

@Kisty sorry i haven't had a chance, so busy with works.

Should I instead keep the nyan cat sample and have another screen demo'ing the progress, or have it on the same screen with 2 halves (one for nyan cat, another to show progress)

I think it's better to have different screen for the demos 😄

have you managed to take a peek at the PR, yet? #18

I will check on it ASAP, probably this weekend. Sorry i couldn't check it sooner đŸ˜Ļ

nqnhat1988 commented 5 years ago

@Kisty @esafirm Could you continue on this issue? It would be a present for us