Skip to content

Commit

Permalink
fixed: race condition when attaching the fragment
Browse files Browse the repository at this point in the history
This fixes #654
  • Loading branch information
yuriykulikov committed May 1, 2024
1 parent 8293b52 commit 2c7e170
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 56 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/gradle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ jobs:
- name: Set up JDK 11
uses: actions/setup-java@v4
with:
java-version: '11'
distribution: 'adopt'
java-version: '17'
distribution: 'temurin'
- name: Grant execute permission for gradlew
run: chmod +x gradlew
- name: Spotless
Expand Down
4 changes: 2 additions & 2 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ val acraEmail =
android {
compileSdk = 33
defaultConfig {
versionCode = 31504
versionName = "3.15.04"
versionCode = 31601

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
versionName = "3.16.01"
applicationId = "com.better.alarm"
minSdk = 21
targetSdk = 33
Expand Down
6 changes: 3 additions & 3 deletions app/src/main/java/com/better/alarm/data/Alarmtone.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ fun Alarmtone.ringtoneManagerUri(): String? {

@Serializable
sealed class Alarmtone {
@SerialName("Silent") @Serializable object Silent : Alarmtone()
@SerialName("Silent") @Serializable data object Silent : Alarmtone()

/** User has selected the same alarm as in the application settings. */
@SerialName("Default") @Serializable object Default : Alarmtone()
@SerialName("Default") @Serializable data object Default : Alarmtone()

/** The ringtone is a SystemDefault ringtone. */
@SerialName("SystemDefault") @Serializable object SystemDefault : Alarmtone()
@SerialName("SystemDefault") @Serializable data object SystemDefault : Alarmtone()

@SerialName("Sound") @Serializable data class Sound(val uriString: String) : Alarmtone()

Expand Down
10 changes: 8 additions & 2 deletions app/src/main/java/com/better/alarm/platform/Permissions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@ import com.better.alarm.data.ringtoneManagerUri
import com.better.alarm.logger.Logger
import com.better.alarm.services.PlayerWrapper
import com.better.alarm.ui.ringtonepicker.userFriendlyTitle
import kotlin.coroutines.coroutineContext
import kotlinx.coroutines.ensureActive

/** Checks if all ringtones can be played, and requests permissions if it is not the case */
fun checkPermissions(activity: Activity, tones: List<Alarmtone>) {
suspend fun checkPermissions(activity: Activity, tones: List<Alarmtone>) {
checkSetAlarmPermissions(activity)
checkMediaPermissions(activity, tones)
}

private fun checkMediaPermissions(activity: Activity, tones: List<Alarmtone>) {
private suspend fun checkMediaPermissions(activity: Activity, tones: List<Alarmtone>) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) return
val mediaPermission =
when {
Expand Down Expand Up @@ -55,6 +57,10 @@ private fun checkMediaPermissions(activity: Activity, tones: List<Alarmtone>) {
.map { ringtone -> ringtone.userFriendlyTitle(activity) }

if (unplayable.isNotEmpty()) {
logger.warning {
"Some ringtones are not playable: $unplayable, showing dialog to request permissions"
}
coroutineContext.ensureActive()
try {
AlertDialog.Builder(activity)
.setTitle(activity.getString(R.string.alert))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,13 @@ import io.reactivex.disposables.Disposable
import io.reactivex.disposables.Disposables
import java.text.SimpleDateFormat
import java.util.Calendar
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.withContext
import kotlinx.coroutines.launch
import org.koin.android.ext.android.inject
import org.koin.androidx.viewmodel.ext.android.viewModel

Expand Down Expand Up @@ -305,19 +304,18 @@ class AlarmDetailsFragment : Fragment() {
value to defaultRingtone
}
.map { (alarmtone, default) ->
withContext(Dispatchers.IO) {
when {
// Default (title)
// We need this case otherwise we will have "App default (Default (title))"
alarmtone is Alarmtone.Default && default is Alarmtone.SystemDefault ->
default.userFriendlyTitle(requireActivity())
// App default (title)
alarmtone is Alarmtone.Default ->
getString(
R.string.app_default_ringtone, default.userFriendlyTitle(requireActivity()))
// title
else -> alarmtone.userFriendlyTitle(requireActivity())
}
val hostActivity = requireActivity()
when {
// Default (title)
// We need this case otherwise we will have "App default (Default (title))"
alarmtone is Alarmtone.Default && default is Alarmtone.SystemDefault ->
default.userFriendlyTitle(hostActivity)
// App default (title)
alarmtone is Alarmtone.Default ->
hostActivity.getString(
R.string.app_default_ringtone, default.userFriendlyTitle(hostActivity))
// title
else -> alarmtone.userFriendlyTitle(hostActivity)
}
}
.onEach { ringtoneSummary.text = it }
Expand Down Expand Up @@ -371,7 +369,7 @@ class AlarmDetailsFragment : Fragment() {
override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
if (data != null && requestCode == ringtonePickerRequestCode) {
val alarmtone = data.getPickedRingtone()
checkPermissions(requireActivity(), listOf(alarmtone))
lifecycleScope.launch { checkPermissions(requireActivity(), listOf(alarmtone)) }
logger.debug { "Picked alarm tone: $alarmtone" }
modify("Ringtone picker") { prev -> prev.copy(alarmtone = alarmtone, isEnabled = true) }
}
Expand Down
11 changes: 6 additions & 5 deletions app/src/main/java/com/better/alarm/ui/main/AlarmsListActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ import com.google.android.material.snackbar.Snackbar
import io.reactivex.disposables.Disposables
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.rx2.awaitFirst
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.protobuf.ProtoBuf
import org.koin.android.ext.android.inject
Expand Down Expand Up @@ -113,11 +115,10 @@ class AlarmsListActivity() : AppCompatActivity() {

setContentView(R.layout.list_activity)

store
.alarms()
.take(1)
.subscribe { alarms -> checkPermissions(this, alarms.map { it.alarmtone }) }
.apply {}
val context = this
lifecycleScope.launch {
checkPermissions(context, store.alarms().awaitFirst().map { it.alarmtone })
}

backPresses.onBackPressed(lifecycle) { finish() }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ import android.content.Intent
import android.media.Ringtone
import android.media.RingtoneManager
import android.net.Uri
import android.os.Looper
import android.widget.Toast
import androidx.core.net.toUri
import androidx.fragment.app.Fragment
import com.better.alarm.R
import com.better.alarm.data.Alarmtone
import com.better.alarm.data.ringtoneManagerUri
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext

/**
* Shows the ringtone picker.
Expand Down Expand Up @@ -88,26 +92,27 @@ fun Intent.getPickedRingtone(): Alarmtone {
return alarmtone
}

fun Alarmtone.userFriendlyTitle(context: Context): CharSequence {
return runCatching {
when (this) {
is Alarmtone.Silent -> context.getText(R.string.silent_alarm_summary)
else ->
RingtoneManager.getRingtone(context, this.ringtoneManagerUri()?.toUri())
.title(context)
}
}
.getOrDefault("")
suspend fun Alarmtone.userFriendlyTitle(context: Context): CharSequence {
checkNotNull(Looper.myLooper()) { "userFriendlyTitle should be called from the main thread" }
return when (this) {
is Alarmtone.Silent -> context.getText(R.string.silent_alarm_summary)
else ->
ringtoneManagerUri() //
?.toUri()
?.let { uri -> context.getRingtoneTitleOrNull(uri) }
?: context.getText(R.string.silent_alarm_summary)
}
}

private fun Ringtone.title(context: Context): CharSequence {
// this can fail, see
// https://github.com/yuriykulikov/AlarmClock/issues/403
return try {
getTitle(context) ?: context.getText(R.string.silent_alarm_summary)
} catch (e: Exception) {
context.getText(R.string.silent_alarm_summary)
} catch (e: NullPointerException) {
null
} ?: ""
/**
* Call to [Ringtone.getTitle] can fail, see
* [AlarmClock#403](https://github.com/yuriykulikov/AlarmClock/issues/403)
*/
private suspend fun Context.getRingtoneTitleOrNull(uri: Uri): CharSequence? {
val context = this
return withContext(Dispatchers.IO) {
runCatching { RingtoneManager.getRingtone(context, uri).getTitle(context) }
.onFailure { if (it is CancellationException) throw it }
.getOrNull()
}
}
22 changes: 16 additions & 6 deletions app/src/main/java/com/better/alarm/ui/settings/SettingsFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import android.os.Handler
import android.os.Looper
import android.os.Vibrator
import android.provider.Settings
import androidx.lifecycle.lifecycleScope
import androidx.preference.CheckBoxPreference
import androidx.preference.ListPreference
import androidx.preference.Preference
Expand All @@ -24,6 +25,11 @@ import com.better.alarm.ui.ringtonepicker.getPickedRingtone
import com.better.alarm.ui.ringtonepicker.showRingtonePicker
import com.better.alarm.ui.ringtonepicker.userFriendlyTitle
import io.reactivex.disposables.CompositeDisposable
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import kotlinx.coroutines.rx2.asFlow
import org.koin.android.ext.android.inject

/** Created by Yuriy on 24.07.2017. */
Expand Down Expand Up @@ -77,10 +83,12 @@ class SettingsFragment : PreferenceFragmentCompat() {
check(alarmtone !is Alarmtone.Default) { "Default alarmtone is not allowed here" }
logger.debug { "Picked default alarm tone: $alarmtone" }
prefs.defaultRingtone.value = alarmtone.asString()
checkPermissions(
requireActivity(),
listOf(alarmtone),
)
lifecycleScope.launch {
checkPermissions(
requireActivity(),
listOf(alarmtone),
)
}
}

override fun onPreferenceTreeClick(preference: Preference): Boolean {
Expand Down Expand Up @@ -111,15 +119,17 @@ class SettingsFragment : PreferenceFragmentCompat() {
val current = Alarmtone.fromString(prefs.defaultRingtone.value)
showRingtonePicker(current, 43)
}

prefs.defaultRingtone
.observe()
.asFlow()
.map { Alarmtone.fromString(it) }
.subscribe { alarmtone ->
.onEach { alarmtone ->
val summary = alarmtone.userFriendlyTitle(requireContext())
logger.debug { "Setting summary to $summary" }
ringtoneTitle = summary
}
.also { disposables.add(it) }
.launchIn(lifecycleScope)
}

bindListPreference(Prefs.KEY_ALARM_SNOOZE, prefs.snoozeDuration) { duration ->
Expand Down

0 comments on commit 2c7e170

Please sign in to comment.