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

fixed: race condition when attaching the fragment #686

Merged
merged 2 commits into from
Jul 4, 2024
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
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 @@
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,9 @@ class AlarmDetailsViewModel(private val uiStore: UiStore) : ViewModel() {
uiStore.editing().value?.let { prev -> uiStore.edit(function(prev.value), prev.isNew) }
}

var newAlarmPopupSeen: Boolean = false
var newAlarmPopupSeen: Boolean
get() = uiStore.newAlarmPopupSeen
set(value) {
uiStore.newAlarmPopupSeen = value
}
}
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
5 changes: 3 additions & 2 deletions app/src/main/java/com/better/alarm/ui/state/UiStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@ class UiStore {

private val editing: MutableStateFlow<EditedAlarm?> = MutableStateFlow(null)

var newAlarmPopupSeen: Boolean = false

fun editing(): StateFlow<EditedAlarm?> {
return editing
}

fun createNewAlarm() {
editing.value = EditedAlarm(isNew = true, value = AlarmValue(isDeleteAfterDismiss = true))
newAlarmPopupSeen = false
}

fun edit(alarmValue: AlarmValue, isNew: Boolean = false) {
Expand All @@ -23,6 +26,4 @@ class UiStore {
fun hideDetails() {
editing.value = null
}

var openDrawerOnCreate: Boolean = false
}
Loading