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

Advances tour stop when it's audio playback ends (AIC-555) #317

Merged
merged 4 commits into from
Nov 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions media/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ dependencies {
implementation project(':analytics')
implementation project(':base')
implementation project(':db')
implementation project(':tour_manager')
}
13 changes: 9 additions & 4 deletions media/src/main/java/edu/artic/media/audio/AudioPlayerService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import edu.artic.media.R
import edu.artic.media.audio.AudioPlayerService.PlayBackAction
import edu.artic.media.audio.AudioPlayerService.PlayBackAction.*
import edu.artic.media.audio.AudioPlayerService.PlayBackState.*
import edu.artic.tours.manager.TourProgressManager
import io.reactivex.subjects.BehaviorSubject
import io.reactivex.subjects.Subject
import javax.inject.Inject
Expand Down Expand Up @@ -168,6 +169,9 @@ class AudioPlayerService : DaggerService(), PlayerService {
@Inject
lateinit var audioPrefManager: AudioPrefManager

@Inject
lateinit var tourProgressManager: TourProgressManager

/**
* Something with one or more audio tracks. See [currentTrack] and [AudioFileModel].
*/
Expand Down Expand Up @@ -205,6 +209,7 @@ class AudioPlayerService : DaggerService(), PlayerService {
analyticsTracker.reportEvent(EventCategoryName.PlayBack, AnalyticsAction.playbackCompleted, currentTrack.value?.title.orEmpty())
audioPlayBackStatus.onNext(PlayBackState.Stopped(given))
playerNotificationManager.setPlayer(null)
tourProgressManager.playBackEnded(given)
currentTrack.onNext(EMPTY_AUDIO_FILE)
}
playbackState == Player.STATE_IDLE -> audioPlayBackStatus.onNext(PlayBackState.Stopped(given))
Expand Down Expand Up @@ -232,8 +237,8 @@ class AudioPlayerService : DaggerService(), PlayerService {
when (playBackAction) {
is PlayBackAction.Play -> {
playerNotificationManager.setPlayer(player)
// No need to seek here; that'll be done in 'setArticObject' if needed
setArticObject(playBackAction.audioFile, playBackAction.audioModel)
// No need to seek here; that'll be done in 'changeAudio' if needed
changeAudio(playBackAction.audioFile, playBackAction.audioModel)
/**
* If the audio is being played for the first time, make sure we invoke
* [AudioServiceHook.displayAudioTutorial] event.
Expand Down Expand Up @@ -362,7 +367,7 @@ class AudioPlayerService : DaggerService(), PlayerService {
}
}

fun setArticObject(_articObject: Playable, audio: AudioFileModel, resetPosition: Boolean = false) {
fun changeAudio(_articObject: Playable, audio: AudioFileModel, resetPosition: Boolean = false) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method changeAudio has 28 lines of code (exceeds 25 allowed). Consider refactoring.


val isDifferentAudio = (currentTrack as BehaviorSubject).value != audio

Expand Down Expand Up @@ -453,7 +458,7 @@ class AudioPlayerService : DaggerService(), PlayerService {
* If nothing is [currently playing][audioPlayBackStatus], this skips
* the 'pause' and 'resume' operations.
*
* @see setArticObject
* @see changeAudio
*/
override fun switchAudioTrack(alternative: AudioFileModel) {
playable?.let {
Expand Down
1 change: 1 addition & 0 deletions tour_manager/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ dependencies {
kapt libs.dagger_android_compiler
implementation project(':db')
implementation libs.rx_kotlin
implementation libs.rx_helpers
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package edu.artic.tours.manager

import dagger.Module
import dagger.Provides
import edu.artic.db.daos.ArticAudioFileDao
import javax.inject.Singleton

/**
Expand All @@ -15,6 +16,6 @@ abstract class TourManagerModule {
@JvmStatic
@Provides
@Singleton
fun tourProgressManager(): TourProgressManager = TourProgressManager()
fun tourProgressManager(audioFileDao: ArticAudioFileDao): TourProgressManager = TourProgressManager(audioFileDao)
}
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
package edu.artic.tours.manager

import android.annotation.SuppressLint
import android.support.annotation.WorkerThread
import com.fuzz.rx.Optional
import com.fuzz.rx.filterValue
import edu.artic.db.INTRO_TOUR_STOP_OBJECT_ID
import edu.artic.db.daos.ArticAudioFileDao
import edu.artic.db.models.ArticTour
import edu.artic.db.models.AudioFileModel
import io.reactivex.rxkotlin.Observables
import io.reactivex.rxkotlin.subscribeBy
import io.reactivex.subjects.BehaviorSubject
import io.reactivex.subjects.PublishSubject
import io.reactivex.subjects.Subject


import timber.log.Timber

/**
* Responsible for managing the communication between tour carousel and the map.
* @author Sameer Dhakal (Fuzz)
*/
class TourProgressManager {
class TourProgressManager(val audioFileDao: ArticAudioFileDao) {
/**
* Selected ArticObject
*/
Expand All @@ -23,4 +30,61 @@ class TourProgressManager {
val selectedTour: Subject<Optional<ArticTour>> = BehaviorSubject.createDefault(Optional(null))
val proposedTour: Subject<Optional<Pair<ArticTour, ArticTour.TourStop>>> = BehaviorSubject.createDefault(Optional(null))
val leaveTourRequest: Subject<Boolean> = PublishSubject.create()

/**
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method playBackEnded has a Cognitive Complexity of 27 (exceeds 20 allowed). Consider refactoring.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method playBackEnded has 28 lines of code (exceeds 25 allowed). Consider refactoring.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method playBackEnded has a Cognitive Complexity of 30 (exceeds 20 allowed). Consider refactoring.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method playBackEnded has 32 lines of code (exceeds 25 allowed). Consider refactoring.

* If [audioFileModel] belongs to currently selected tour i.e. [selectedTour],
* this method advances [selectedStop] to next tour stop.
*/
@WorkerThread
@SuppressLint("CheckResult")
fun playBackEnded(audioFileModel: AudioFileModel) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a @WorkerThread annotation to make it clear this shouldn't run on the main thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Updated.

Observables.combineLatest(selectedTour.filterValue(), selectedStop)
.take(1)
.subscribe { (tour, currentStopID) ->

/**
* Get audio file id
*/
val audioFileID: String? = if (currentStopID == INTRO_TOUR_STOP_OBJECT_ID) {
tour.tourAudio
} else {
tour.tourStops.find { it.objectId == currentStopID }?.audioId
}

audioFileID?.let { audioID ->
audioFileDao
.getAudioByIdAsync(audioID)
Cliabhach marked this conversation as resolved.
Show resolved Hide resolved
.toObservable()
.subscribeBy(
onNext = { audioFile ->

/**
* Check if currently ended audio playback session belongs to
* current tour stop.
*/
val isCurrentStopsAudioTranslation = audioFile
.allTranslations()
.contains(audioFileModel)

/**
* Advance [selectedStop] if current audio translation playback
* completed.
*/
if (isCurrentStopsAudioTranslation) {
val indexOfCurrentTourStop = tour.tourStops.indexOfFirst { it.objectId == currentStopID }
val nextStopIndex = Math.min(indexOfCurrentTourStop + 1, tour.tourStops.size)
tour.tourStops[nextStopIndex].objectId?.let {
selectedStop.onNext(it)
}
}
},
onError = { t ->
Timber.e(t, "Couldn't fetch audio by $audioID")
})
}
}

}


}