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

#1407 improvements to audio settings #1429

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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 app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ dependencies {
androidTestImplementation "androidx.test.espresso:espresso-core:${espressoVersion}"
androidTestImplementation "androidx.test.espresso:espresso-intents:${espressoVersion}"
implementation 'com.crashlytics.sdk.android:crashlytics:2.10.1'
implementation 'com.google.code.gson:gson:2.8.6'
Copy link
Contributor

Choose a reason for hiding this comment

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

we already use moshi so let's not add gson


errorprone 'com.google.errorprone:error_prone_core:2.3.3'
//noinspection GradleDynamicVersion
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.quran.labs.androidquran.dao.audio

import android.os.Parcelable
import com.quran.data.model.SuraAyah
import kotlinx.android.parcel.Parcelize

@Parcelize
data class AudioPlaybackSettings(val start: SuraAyah,
val end: SuraAyah,
val verseRepeatCount: Int = 0,
val rangeRepeatCount: Int = 0,
val enforceRange: Boolean = false) : Parcelable
Comment on lines +1 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

what if, instead of doing this, we remove this class (and the corresponding changes to read/write this) - and instead do the work in the AudioService?

Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,5 @@ public class Constants {
public static final String PREF_CHECKED_PARTIAL_IMAGES = "didCheckPartialImages";
public static final String PREF_CURRENT_AUDIO_REVISION = "currentAudioRevision";
public static final String PREF_SURA_TRANSLATED_NAME = "suraTranslatedName";
public static final String PREF_AUDIO_PLAYBACK_SETTINGS = "audioPlaybackSettings";
}
Original file line number Diff line number Diff line change
Expand Up @@ -414,15 +414,8 @@ private void handleIntent(Intent intent) {
audioQueue = new AudioQueue(quranInfo, audioRequest,
new AudioPlaybackInfo(start, 1, 1, basmallah));
Crashlytics.log("audio request has changed...");

if (player != null) {
player.stop();
}
state = State.Stopped;
Crashlytics.log("stop if playing...");
Comment on lines -418 to -422
Copy link
Contributor

Choose a reason for hiding this comment

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

stopping is sometimes the right thing to do - but this is where we should check. you have the old audioRequest (before we override it in line 409). we can check for example if the current verse is within the range of playback (i.e. between oldAudioRequest.getStart() and oldAudioRequest.getEnd()), then don't stop. otherwise stop, and in either case, we'll replace the audioRequest.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the reply @ahmedre I will work on the changes as soon as possible
Ma'Assalam

}

processTogglePlaybackRequest();
processTogglePlaybackRequest();
} else if (ACTION_PLAY.equals(action)) {
processPlayRequest();
} else if (ACTION_PAUSE.equals(action)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
import com.quran.data.core.QuranInfo;
import com.quran.labs.androidquran.QuranApplication;
import com.quran.labs.androidquran.R;
import com.quran.labs.androidquran.dao.audio.AudioPlaybackSettings;
import com.quran.labs.androidquran.dao.audio.AudioRequest;
import com.quran.data.model.SuraAyah;
import com.quran.labs.androidquran.ui.PagerActivity;
import com.quran.labs.androidquran.ui.helpers.HighlightType;
import com.quran.labs.androidquran.util.QuranSettings;
import com.quran.labs.androidquran.util.QuranUtils;
import com.quran.labs.androidquran.widgets.QuranSpinner;

Expand Down Expand Up @@ -45,7 +47,10 @@ public class AyahPlaybackFragment extends AyahActionFragment {
private ArrayAdapter<CharSequence> startAyahAdapter;
private ArrayAdapter<CharSequence> endingAyahAdapter;

@Inject QuranInfo quranInfo;
@Inject
QuranInfo quranInfo;
@Inject
QuranSettings quranSettings;

@Override
public View onCreateView(LayoutInflater inflater,
Expand Down Expand Up @@ -78,15 +83,15 @@ public View onCreateView(LayoutInflater inflater,
repeatRangeSpinner.setAdapter(rangeAdapter);
repeatRangeSpinner.setOnItemSelectedListener(
new AdapterView.OnItemSelectedListener() {
@Override
public void onItemSelected(AdapterView<?> parent, View view, int position, long id) {
updateEnforceBounds(position);
}
@Override
public void onItemSelected(AdapterView<?> parent, View view, int position, long id) {
updateEnforceBounds(position);
}

@Override
public void onNothingSelected(AdapterView<?> parent) {
}
});
@Override
public void onNothingSelected(AdapterView<?> parent) {
}
});
final ArrayAdapter<CharSequence> verseAdapter =
new ArrayAdapter<>(context, ITEM_LAYOUT, repeatOptions);
verseAdapter.setDropDownViewResource(
Expand Down Expand Up @@ -169,15 +174,20 @@ private void apply() {
if (updatedRange) {
pagerActivity.toggleActionBarVisibility(true);
}
AudioPlaybackSettings lastAudioSettings =
new AudioPlaybackSettings(currentStart, currentEnding, verseRepeat, rangeRepeat,
enforceRange);
quranSettings.setLastAudioSettings(lastAudioSettings);
}

}

private void initializeSuraSpinner(final Context context,
QuranSpinner spinner,
final ArrayAdapter<CharSequence> ayahAdapter) {
String[] suras = context.getResources().
getStringArray(R.array.sura_names);
for (int i=0; i<suras.length; i++){
for (int i = 0; i < suras.length; i++) {
suras[i] = QuranUtils.getLocalizedNumber(context, (i + 1)) +
". " + suras[i];
}
Expand All @@ -192,12 +202,12 @@ public void onItemSelected(AdapterView<?> parent, View view, int position, long
int sura = position + 1;
int ayahCount = quranInfo.getNumberOfAyahs(sura);
CharSequence[] ayahs = new String[ayahCount];
for (int i = 0; i < ayahCount; i++){
for (int i = 0; i < ayahCount; i++) {
ayahs[i] = QuranUtils.getLocalizedNumber(context, (i + 1));
}
ayahAdapter.clear();

for (int i=0; i<ayahCount; i++){
for (int i = 0; i < ayahCount; i++) {
ayahAdapter.add(ayahs[i]);
}
}
Expand Down Expand Up @@ -264,16 +274,15 @@ private int positionToRepeat(int position) {
protected void refreshView() {
final Context context = getActivity();
if (context instanceof PagerActivity && start != null && end != null) {
final AudioRequest lastRequest =
((PagerActivity) context).getLastAudioRequest();
final AudioPlaybackSettings lastAudioSettings = quranSettings.getLastAudioSettings();
final SuraAyah start;
final SuraAyah ending;
if (lastRequest != null) {
start = lastRequest.getStart();
ending = lastRequest.getEnd();
verseRepeatCount = lastRequest.getRepeatInfo();
rangeRepeatCount = lastRequest.getRangeRepeatInfo();
shouldEnforce = lastRequest.getEnforceBounds();
if (lastAudioSettings != null) {
start = lastAudioSettings.getStart();
ending = lastAudioSettings.getEnd();
verseRepeatCount = lastAudioSettings.getVerseRepeatCount();
rangeRepeatCount = lastAudioSettings.getRangeRepeatCount();
shouldEnforce = lastAudioSettings.getEnforceRange();
decidedStart = start;
decidedEnd = ending;
applyButton.setText(R.string.play_apply);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
import android.os.Environment;
import android.preference.PreferenceManager;

import com.google.gson.Gson;
import com.quran.labs.androidquran.BuildConfig;
import com.quran.labs.androidquran.R;
import com.quran.labs.androidquran.dao.audio.AudioPlaybackSettings;
import com.quran.labs.androidquran.dao.audio.AudioRequest;
import com.quran.labs.androidquran.data.Constants;
import com.quran.labs.androidquran.service.QuranDownloadService;

Expand All @@ -21,6 +24,7 @@
import androidx.annotation.VisibleForTesting;



public class QuranSettings {
private static final String PREFS_FILE = "com.quran.labs.androidquran.per_installation";

Expand All @@ -29,6 +33,7 @@ public class QuranSettings {
private Context appContext;
private SharedPreferences prefs;
private SharedPreferences perInstallationPrefs;
Gson gson = new Gson();

public static synchronized QuranSettings getInstance(@NonNull Context context) {
if (instance == null) {
Expand Down Expand Up @@ -156,7 +161,9 @@ public void setShowRecents(boolean minimizeRecents) {
prefs.edit().putBoolean(Constants.PREF_SHOW_RECENTS, minimizeRecents).apply();
}

public boolean getShowDate() { return prefs.getBoolean(Constants.PREF_SHOW_DATE, false); }
public boolean getShowDate() {
return prefs.getBoolean(Constants.PREF_SHOW_DATE, false);
}

public void setShowDate(boolean isDateShown) {
prefs.edit().putBoolean(Constants.PREF_SHOW_DATE, isDateShown).apply();
Expand All @@ -165,6 +172,7 @@ public void setShowDate(boolean isDateShown) {
public boolean isShowSuraTranslatedName() {
return prefs.getBoolean(Constants.PREF_SURA_TRANSLATED_NAME, appContext.getResources().getBoolean(R.bool.show_sura_names_translation));
}

// probably should eventually move this to Application.onCreate..
public void upgradePreferences() {
int version = getVersion();
Expand Down Expand Up @@ -195,7 +203,7 @@ public void upgradePreferences() {
.remove(QuranDownloadService.PREF_LAST_DOWNLOAD_ERROR)
.remove(QuranDownloadService.PREF_LAST_DOWNLOAD_ITEM)
.remove(Constants.PREF_ACTIVE_TRANSLATION)
// these aren't migrated since they can be derived pretty easily
// these aren't migrated since they can be derived pretty easily
.remove("didPresentPermissionsRationale") // was renamed, removing old one
.remove(Constants.PREF_DEFAULT_IMAGES_DIR)
.remove(Constants.PREF_HAVE_UPDATED_TRANSLATIONS)
Expand Down Expand Up @@ -436,4 +444,12 @@ public void setCheckedPartialImages(String pageType) {
perInstallationPrefs.edit()
.putStringSet(Constants.PREF_CHECKED_PARTIAL_IMAGES, setToSave).apply();
}

public void setLastAudioSettings(AudioPlaybackSettings audioSettings) {
prefs.edit().putString(Constants.PREF_AUDIO_PLAYBACK_SETTINGS, gson.toJson(audioSettings)).apply();
}

public AudioPlaybackSettings getLastAudioSettings() {
return gson.fromJson(prefs.getString(Constants.PREF_AUDIO_PLAYBACK_SETTINGS, ""), AudioPlaybackSettings.class);
}
}
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ buildscript {
}

dependencies {
classpath 'com.android.tools.build:gradle:4.0.0'
classpath 'com.android.tools.build:gradle:4.0.1'
classpath "io.fabric.tools:gradle:1.31.2"
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion"
classpath "net.ltgt.gradle:gradle-errorprone-plugin:1.2.1"
Expand Down