Add Custom Vibrate Patterns to Settings #1169

agrajaghh · 2014-03-15T10:46:24Z

I added the ability to choose custom vibrate Patterns. This fixes issue #230 You can chose between:

default
2x short
2x long
3x short
3x long
custom (you can specify off-time, on-time)

Of course I'm open to different standard vibrate patters or other changes.

This is my first pull request and I'm quite new to Java and Android, so please be gentle ;)

lorenzhs · 2014-03-15T11:16:42Z

src/org/thoughtcrime/securesms/preferences/VibratePatternListPreference.java

@@ -0,0 +1,151 @@
+/**
+ * Copyright (C) 2011 Whisper Systems


hehe, fixed...

agrajaghh · 2014-03-19T23:34:00Z

I tested it on my Nexus 4 and on a Android 2.3 emulator (couldn't test the vibrate, just the dialogs).

What do you think?

moxie0 · 2014-03-21T01:10:32Z

instead of having a checkbox to enable/disable vibrations, and then a separate preference for the vibration pattern, let's just have one preference where one of the options is "do not vibrate" or whatever

moxie0 · 2014-03-21T01:12:16Z

Also, the preference description should display the configured value rather than describing what the preference is.

agrajaghh · 2014-03-21T11:41:04Z

@moxie0 Okay, you are right, thats better! I fixed it.
I'm still not that happy with one other thing: I can't prevent the dialog where you enter the custom pattern from closing. So when you enter a wrong pattern oder click on the "Test" Button the dialog gets closed, and I open it again. It would be better if it wouldn't close... Do you have an advice how I can do that?

edit: done now

moxie0 · 2014-04-05T20:50:45Z

src/org/thoughtcrime/securesms/notifications/MessageNotifier.java

+        builder.setVibrate(new long[]{0,300,200,300,200,300});
+      } else if (vibratePattern.equals("3x_long")) {
+        builder.setVibrate(new long[]{0,750,400,750,400,750});
+      }


Please have the stored preference be the eventual value that's passed to setVibrate so that you don't have to do this rule matching.

okay, I changed the behaviour, can you check if its okay?

agrajaghh · 2014-04-23T23:40:11Z

I squashed the commits into one and I think I fixed your points

moxie0 · 2014-05-17T03:30:24Z

src/org/thoughtcrime/securesms/ApplicationPreferencesActivity.java

@@ -122,6 +122,8 @@ protected void onCreate(Bundle icicle) {
      .setOnPreferenceChangeListener(new ListSummaryListener());
    this.findPreference(TextSecurePreferences.LED_BLINK_PREF)
      .setOnPreferenceChangeListener(new ListSummaryListener());
+    this.findPreference(TextSecurePreferences.VIBRATE_PREF)
+            .setOnPreferenceChangeListener(new ListSummaryListener());


This should be aligned like the lines above it.

agrajaghh · 2014-05-17T11:10:12Z

I fixed the variable names and alignment

mcginty · 2014-06-12T09:56:20Z

res/values/arrays.xml

+    <item>@string/preferences__pref_disabled</item>
+  </string-array>
+
+  <string-array name="pref_vibrate_values" translatable="false">


default/custom/disabled are all translatable

But this are the vaules, they are never displayed. The pref_vibrate_entries above are displayed. Should I really remove the translatable=false?

no you're totally right, sorry I mixed the entries with values when I was speed reading :)

mcginty · 2014-06-12T10:09:04Z

If I have a normal pattern set, choose custom and then hit "Cancel" in the Custom Vibrate Pattern dialog, my setting still shows up as "custom" when it should probably not be set.

mcginty · 2014-06-12T10:17:41Z

May be a good idea to also in the summary show the pattern they have set instead of just "Custom." Perhaps something like "Custom: 0,500,100,500"

mcginty · 2014-06-12T10:18:18Z

src/org/thoughtcrime/securesms/preferences/VibratePatternListPreference.java

+    }
+  }
+
+    private void showDialog() {


reverse-indent :)

agrajaghh · 2014-06-12T14:47:51Z

@mcginty I fixed the "Cancel" or back-button behaviour and added the custom value to the summary like this "Custom: 0,500,100,500"

Sorry for the formatting, lower/uppercase and indent mistakes...

moxie0 · 2014-06-13T01:58:40Z

also how are you going to migrate people's existing vibrate preference to this?

agrajaghh · 2014-06-13T06:12:02Z

@moxie0 The default pattern will be set regardless of the current setting. You are right, thats not good...
Can you give me a hint where I can insert a migration during update?

agrajaghh · 2014-06-19T17:02:39Z

Should I put it in the DatabaseUpgradeActivity to prevent a loss of the setting when vibrate is currently disabled?

mcginty · 2014-06-19T21:50:31Z

src/org/thoughtcrime/securesms/util/TextSecurePreferences.java

-  public static boolean isNotificationVibrateEnabled(Context context) {
-    return getBooleanPreference(context, VIBRATE_PREF, true);
+  public static String getNotificationVibrate(Context context) {
+    return getStringPreference(context, VIBRATE_PREF, "default");


Couldn't you just change this line to be backward compatible?

getStringPreference(context, VIBRATE_PREF, getBooleanPreference(context, VIBRATE_PREF, true) ? "default" : "disabled");

yes, I thought of a check there too, but I thought its more clean when we don't check every time a notification arrives, and just once during update. But perhaps you're right and this is easier...

I tried it, but the problem is that the setting will be displayed wrong nevertheless. getNotificationVibrate is not called when you open the settings and the summary is displayed. I think the real migration is the better way....

On 06/20/2014 03:41 PM, agrajaghh wrote:

I tried it, but the problem is that the setting will be displayed wrong
nevertheless. getNotificationVibrate is not called when you open the
settings and the summary is displayed. I think the real migration is the
better way....

You can't be guaranteed that activity runs first. Migrations should be
avoided whenever possible.

http://www.thoughtcrime.org

agrajaghh · 2014-06-20T22:38:36Z

The existing preference is now migrated in DatabaseUpgradeActivity

agrajaghh · 2014-06-22T18:05:57Z

src/org/thoughtcrime/securesms/util/TextSecurePreferences.java

@@ -243,8 +244,24 @@ public static String getNotificationRingtone(Context context) {
    return getStringPreference(context, RINGTONE_PREF, null);
  }

-  public static boolean isNotificationVibrateEnabled(Context context) {
-    return getBooleanPreference(context, VIBRATE_PREF, true);
+  public static void migrateNotificationVibrate(Context context) {


I added the migration here. Is this better?

claenn · 2014-07-01T18:26:50Z

love your vibration feature, thanks a lot.

pejakm · 2014-12-06T13:21:16Z

@agrajaghh Can we have a rebase, please?

agrajaghh · 2014-12-06T13:26:10Z

@pejakm I'm waiting for #1635 to be in

pejakm · 2014-12-06T13:59:28Z

@agrajaghh Ah, I see what you mean. I've applied the patch and compiled successfully, and it looks great! Thank you for this!

moxie0 · 2014-12-11T23:26:56Z

Thanks @agrajaghh, but I think we're going to skip this for now. It's a lot of code and includes a preference migration, and we're not sure that it's something the majority of users will find useful. We want to focus on customization for the core settings first.

pejakm · 2014-12-12T10:33:29Z

@agrajaghh Could you please rebase it anyway, for people who want this in their own build?

agrajaghh · 2014-12-12T10:56:28Z

@pejakm done at https://github.com/agrajaghh/TextSecure/tree/VibratePattern

But I won't keep it up to date, sry...

edit: not tested btw...

pejakm · 2014-12-12T11:10:57Z

Thank you very much! :)

pejakm · 2014-12-12T13:24:29Z

@agrajaghh Tested and works, thank you once again.

claenn · 2014-12-12T14:34:04Z

Thanks for the rebase. It's very useful. I've had to use an automation tool
(automagic, tasker) before.

Mladen Pejaković notifications@github.com schrieb am Fr., 12. Dez. 2014
14:24:

@agrajaghh https://github.com/agrajaghh Tested and works, thank you
once again.

—
Reply to this email directly or view it on GitHub
#1169 (comment)
.

lorenzhs reviewed Mar 15, 2014
View reviewed changes

moxie0 reviewed Apr 5, 2014
View reviewed changes

Add Custom Vibrate Patterns to Settings

dccfa8a

moxie0 reviewed May 17, 2014
View reviewed changes

agrajaghh added 2 commits May 17, 2014 12:07

fix variable names and alignment

dcb953d

Merge remote-tracking branch 'upstream/master' into VibratePattern

70def80

fix variable names

392cd5e

mcginty reviewed Jun 12, 2014
View reviewed changes

Fix lowercase/uppercase

0da46b3

mcginty reviewed Jun 12, 2014
View reviewed changes

src/org/thoughtcrime/securesms/preferences/VibratePatternListPreference.java

}

}

private void showDialog() {

Copy link

Contributor

mcginty Jun 12, 2014

Choose a reason for hiding this comment

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

reverse-indent :)

agrajaghh added 3 commits June 12, 2014 12:26

Fix xml formatting

6b4591c

Fix formatting

7502fcf

Fix indent

fa19b8c

Fix cancel behaviour and add the custom value to the summary

e7c81f1

mcginty reviewed Jun 19, 2014
View reviewed changes

Add setting migration

e1575d8

migrateNotificationVibrate()

852b217

agrajaghh reviewed Jun 22, 2014
View reviewed changes

moxie0 assigned mcginty Dec 11, 2014

moxie0 closed this Dec 11, 2014

generalmanager mentioned this pull request Jul 9, 2015

Adding per-contact notification preferences #354

Closed

Add Custom Vibrate Patterns to Settings #1169

Add Custom Vibrate Patterns to Settings #1169

Conversation

agrajaghh commented Mar 15, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrajaghh commented Mar 19, 2014

moxie0 commented Mar 21, 2014

moxie0 commented Mar 21, 2014

agrajaghh commented Mar 21, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrajaghh commented Apr 23, 2014

Choose a reason for hiding this comment

agrajaghh commented May 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcginty commented Jun 12, 2014

mcginty commented Jun 12, 2014

Choose a reason for hiding this comment

agrajaghh commented Jun 12, 2014

moxie0 commented Jun 13, 2014

agrajaghh commented Jun 13, 2014

agrajaghh commented Jun 19, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrajaghh commented Jun 20, 2014

Choose a reason for hiding this comment

claenn commented Jul 1, 2014

pejakm commented Dec 6, 2014

agrajaghh commented Dec 6, 2014

pejakm commented Dec 6, 2014

moxie0 commented Dec 11, 2014

pejakm commented Dec 12, 2014

agrajaghh commented Dec 12, 2014

pejakm commented Dec 12, 2014

pejakm commented Dec 12, 2014

claenn commented Dec 12, 2014