Skip to content

Commit

Permalink
Fix for translator not initializing in time on ScanBarcodeScreen for …
Browse files Browse the repository at this point in the history
…when the scanned barcode arrives hence leading to an incorrect state:

When we start the ScanBarcodeScreen we immediately launch the Google ML Lib’s camera app. Which moves focus away from our app and also this camera app consumes a lot of the CPU. Because of that the ‘init translators’ code that happens in parallel, which involves a bunch of back-n-forth with WebView where translator JS is running sometimes doesn’t have enough time to finish hence the translator is not actually ready when you scan barcode.
It happens intermittently because some devices are fast enough to init the translators and some are too slow, also it depends on how quick you are at pointing the camera at the barcode.
So the fix is to queue up all of the scanned barcodes until the translators are initialized and then pass them to the translator.

Upping versionCode to 69.
  • Loading branch information
Dima-Android committed Jun 1, 2024
1 parent 3f50597 commit e588b78
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,23 @@ package org.zotero.android.architecture.core
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch
import org.zotero.android.architecture.coroutines.ApplicationScope

open class StateEventStream<T>(private val applicationScope: ApplicationScope, initValue: T?) {

private val sharedFlow = MutableStateFlow<T?>(initValue)
private val sharedFlow = MutableStateFlow(initValue)

fun emit(update: T?) {
sharedFlow.tryEmit(update)
}

fun emitAsync(update: T) {
applicationScope.launch {
sharedFlow.emit(update)
}
}

fun flow(): StateFlow<T?> = sharedFlow.asStateFlow()

fun currentValue(): T? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.content.Context
import androidx.lifecycle.viewModelScope
import com.google.mlkit.vision.codescanner.GmsBarcodeScanning
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
Expand All @@ -22,22 +23,28 @@ import org.zotero.android.sync.LibraryIdentifier
import org.zotero.android.sync.SchemaController
import org.zotero.android.uicomponents.Strings
import org.zotero.android.uicomponents.addbyidentifier.IdentifierLookupController
import org.zotero.android.uicomponents.addbyidentifier.TranslatorLoadedEventStream
import org.zotero.android.uicomponents.addbyidentifier.data.LookupRow
import org.zotero.android.uicomponents.addbyidentifier.data.LookupRowItem
import timber.log.Timber
import java.util.LinkedList
import javax.inject.Inject

@HiltViewModel
internal class ScanBarcodeViewModel @Inject constructor(
private val fileStore: FileStore,
private val identifierLookupController: IdentifierLookupController,
private val attachmentDownloaderEventStream: RemoteAttachmentDownloaderEventStream,
private val translatorLoadedEventStream: TranslatorLoadedEventStream,
private val schemaController: SchemaController,
private val remoteFileDownloader: RemoteAttachmentDownloader,
private val context: Context,
) : BaseViewModel2<ScanBarcodeViewState, ScanBarcodeViewEffect>(ScanBarcodeViewState()) {

private val queueOfScannedBarcodes = LinkedList<String>()

fun init() = initOnce {
setupTranslatorLoadedObserving()
setupAttachmentObserving()
val collectionKeys =
fileStore.getSelectedCollectionId().keyGet?.let { setOf(it) } ?: emptySet()
Expand All @@ -58,7 +65,11 @@ internal class ScanBarcodeViewModel @Inject constructor(
scanner.startScan()
.addOnSuccessListener { barcode ->
val scannedString = barcode.rawValue ?: ""
onLookup(scannedString)
if (translatorLoadedEventStream.currentValue() == true) {
onLookup(scannedString)
} else {
queueOfScannedBarcodes.add(scannedString)
}
}
.addOnCanceledListener {
triggerEffect(NavigateBack)
Expand Down Expand Up @@ -186,6 +197,16 @@ internal class ScanBarcodeViewModel @Inject constructor(
}.launchIn(viewModelScope)
}

private fun setupTranslatorLoadedObserving() {
translatorLoadedEventStream.flow()
.filter { it == true }
.onEach { _ ->
queueOfScannedBarcodes.forEach {
onLookup(it)
}
}.launchIn(viewModelScope)
}

private fun updateLookupState(lookupState: State) {
updateState {
copy(lookupState = lookupState)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,30 @@ internal fun ScanBarcodeScreen(
.background(color = CustomTheme.colors.zoteroItemDetailSectionBackground)
.padding(horizontal = 16.dp)
) {
scanBarcodeTable(rows = viewState.lookupRows)
if (viewState.lookupState == State.loadingIdentifiers ) {
if (viewState.lookupState == State.waitingInput) {
scanBarcodeLoadingIndicator()
item {
Spacer(modifier = Modifier.height(20.dp))
Row(
modifier = Modifier.fillMaxWidth(),
horizontalArrangement = Arrangement.Center
) {
Text(
text = stringResource(id = Strings.loading_translators),
style = CustomTheme.typography.newBody,
color = CustomTheme.colors.primaryContent,
)
}
}
} else {
scanBarcodeTable(rows = viewState.lookupRows)
if (viewState.lookupState == State.loadingIdentifiers ) {
scanBarcodeLoadingIndicator()
}
scanBarcodeError(
failedState = viewState.lookupState as? State.failed
)
}
scanBarcodeError(
failedState = viewState.lookupState as? State.failed
)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class IdentifierLookupController @Inject constructor(
private val schemaController: SchemaController,
private val dbWrapper: DbWrapper,
private val defaults: Defaults,
private val translatorLoadedEventStream: TranslatorLoadedEventStream,
private val attachmentDownloaderEventStream: RemoteAttachmentDownloaderEventStream,
) {

Expand Down Expand Up @@ -109,6 +110,7 @@ class IdentifierLookupController @Inject constructor(
translatorsLoader = translatorsLoader,
fileStore = fileStore,
noAuthenticationApi = noAuthenticationApi,
translatorLoadedEventStream = translatorLoadedEventStream,
)
lookupWebViewHandlersByLookupSettings[lookupSettings] = lookupWebViewHandler
setupObserver(lookupWebViewHandler)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class LookupWebCallChainExecutor(
private val gson: Gson,
private val translatorsLoader: TranslatorsLoader,
private val fileStore: FileStore,
private val noAuthenticationApi: NoAuthenticationApi
private val noAuthenticationApi: NoAuthenticationApi,
private val translatorLoadedEventStream: TranslatorLoadedEventStream,
) {

private lateinit var lookupWebViewHandler: LookupWebViewHandler
Expand Down Expand Up @@ -166,6 +167,7 @@ class LookupWebCallChainExecutor(
val translatorsResult =
translatorsLoader.translators(null)
sendInitTranslatorsMessage(translatorsResult)
translatorLoadedEventStream.emitAsync(true)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.zotero.android.uicomponents.addbyidentifier

import org.zotero.android.architecture.core.StateEventStream
import org.zotero.android.architecture.coroutines.ApplicationScope
import javax.inject.Inject
import javax.inject.Singleton

@Singleton
class TranslatorLoadedEventStream @Inject constructor(applicationScope: ApplicationScope) :
StateEventStream<Boolean>(applicationScope = applicationScope, initValue = false)
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@
<string name="queued">Queued…</string>
<string name="failed">Failed</string>
<string name="scan_another">Scan Another</string>
<string name="loading_translators">Loading translators… Please wait.</string>
</resources>
2 changes: 1 addition & 1 deletion buildSrc/src/main/kotlin/BuildConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ object BuildConfig {
const val compileSdkVersion = 34
const val targetSdk = 33

val versionCode = 68 // Must be updated on every build
val versionCode = 69 // Must be updated on every build
val version = Version(
major = 1,
minor = 0,
Expand Down

0 comments on commit e588b78

Please sign in to comment.