Android auto support #5880

snaggen · 2016-11-21T15:44:51Z

This pull request adds Android Auto support. I have implemented it according to the guidelines https://developer.android.com/training/auto/messaging/index.html
I have tested it using the Android Auto emulator and used it for two month in a car.

I didn't read the bit about the BitHub reward until i had done all my commits, but they are intended to be freebies.

Fixes #5619
//FREEBIE

First time contributor checklist

I have read how to contribute to this project
I have signed the Contributor Licence Agreement

Contributor checklist

I am following the Code Style Guidelines
I have tested my contribution on these devices:

Volkswagen Passat 2016 with Android Auto using a Nexus 5X
Android Auto Emulator using a Nexus 5X

My contribution is fully baked and ready to be merged as is
I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax
I have made the choice whether I want the BitHub reward or not by omitting or adding the word FREEBIE in the commit message of my first commit

Description

This adds android auto support accordign to https://developer.android.com/training/auto/messaging/index.html#messaging However, since android auto is not officially supported in my country, the functionality is limited. Which means that I have not been able to fully test everything yet. What work is: * Message notification is shown. * When you click on it, the message is read.

I didn't notice any difference from before, but the developer documentation explicitly states that it should be set. https://developer.android.com/training/auto/messaging/index.html#messaging

moxie0 · 2016-11-21T17:43:46Z

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

+import java.util.List;
+
+/**
+ * Get the response text from the Wearable Device and sends an message as a reply


Looks like you copied this from the wearable receiver? Can you update the comment?

moxie0 · 2016-11-21T17:44:00Z

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

+
+  public static final String TAG                 = AndroidAutoHeardReceiver.class.getSimpleName();
+  public static final String HEARD_ACTION        = "org.thoughtcrime.securesms.notifications.ANDROID_AUTO_HEARD";
+  public static final String THREAD_IDS_EXTRA     = "car_heard_thread_ids";


align please

moxie0 · 2016-11-21T17:44:55Z

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

+import java.util.List;
+
+/**
+ * Get the response text from the Wearable Device and sends an message as a reply


update comment please

moxie0 · 2016-11-21T17:45:07Z

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

+    final long[]       recipientIds = intent.getLongArrayExtra(RECIPIENT_IDS_EXTRA);
+    final long threadId = intent.getLongExtra(THREAD_ID_EXTRA, -1);
+    final CharSequence responseText = getMessageText(intent);
+    final Recipients   recipients   = RecipientFactory.getRecipientsForIds(context, recipientIds, false);


align please

moxie0 · 2016-11-21T17:46:17Z

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

+            return remoteInput.getCharSequence(VOICE_REPLY_KEY);
+        }
+        return null;
+    }


two space indents

moxie0 · 2016-11-21T17:46:22Z

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

+
+  private CharSequence getMessageText(Intent intent) {
+        Bundle remoteInput =
+            RemoteInput.getResultsFromIntent(intent);


moxie0 · 2016-11-21T17:46:33Z

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

@@ -233,6 +233,8 @@ private static void sendSingleThreadNotification(@NonNull  Context context,
                       notificationState.getMarkAsReadIntent(context),
                       notificationState.getQuickReplyIntent(context, notifications.get(0).getRecipients()),
                       notificationState.getWearableReplyIntent(context, notifications.get(0).getRecipients()));
+    builder.addAndroidAutoAction(notificationState.getAndroidAutoReplyIntent(context, notifications.get(0).getRecipients()),
+            notificationState.getAndroidAutoHeardIntent(context, notifications.get(0).getRecipients()), notifications.get(0).getTimestamp());


align please

moxie0 · 2016-11-21T17:47:47Z

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

+      return;
+
+    RemoteInput remoteInput = new RemoteInput.Builder(AndroidAutoReplyReceiver.VOICE_REPLY_KEY)
+            .setLabel("Reply").build();


moxie0 · 2016-11-21T17:48:05Z

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

+    RemoteInput remoteInput = new RemoteInput.Builder(AndroidAutoReplyReceiver.VOICE_REPLY_KEY)
+            .setLabel("Reply").build();
+
+    NotificationCompat.CarExtender.UnreadConversation.Builder unreadConvBuilder =


please don't abbreviate variable names

moxie0 · 2016-11-21T17:48:22Z

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

+                    .addMessage(mContentText.toString())
+                    .setLatestTimestamp(timestamp)
+                    .setReadPendingIntent(androidAutoHeardIntent)
+                    .setReplyAction(androidAutoReplyIntent, remoteInput);


align please

//FREEBIE

snaggen · 2016-11-22T06:51:56Z

Thanks for the feedback! Since this was my first android, the lazy coder in me based it of the Ware code and forgot the comments :) However, I have fixed that, and the rest of your feedback.

FeuRenard

I request two indentation changes.

FeuRenard · 2016-11-22T14:02:29Z

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

+
+  public static final String TAG                 = AndroidAutoHeardReceiver.class.getSimpleName();
+  public static final String HEARD_ACTION        = "org.thoughtcrime.securesms.notifications.ANDROID_AUTO_HEARD";
+  public static final String THREAD_IDS_EXTRA    = "car_heard_thread_ids";


Here are three unnecessary white spaces in front of each equal sign.

FeuRenard · 2016-11-22T14:04:04Z

AndroidManifest.xml

@@ -107,6 +107,9 @@
    <meta-data android:name="org.thoughtcrime.securesms.mms.TextSecureGlideModule"
               android:value="GlideModule" />

+    <meta-data android:name="com.google.android.gms.car.application"
+        android:resource="@xml/automotive_app_desc" />


android:resource and android:name should start in the same column.

//FREEBIE

snaggen · 2016-11-22T19:18:22Z

I have fixed the indentation changes.

moxie0 · 2016-11-23T04:47:41Z

AndroidManifest.xml

@@ -455,6 +458,19 @@
        </intent-filter>
    </receiver>

+    <receiver android:name=".notifications.AndroidAutoHeardReceiver">


do these need to be exported? (default true if you have an intent filter defined iirc)

wouldn't that allow any app on the device to send a signal message?

I have tested to set export=false for the ReplyReciever and that seems to work. So I have pushed a fix for this.

looks like this one is still exported though?

Yes, I didn't change that since is basically doing the same thing as the MarkReadReceiver that is explicitly set to exported=true. So setting AndroidAutoHeardReciever doesn't really change anything in regards to what external apps can do. But I can change and test this if you like it not to be exported.

as a rule, no broadcast receivers should ever be exported unless they absolutely have to be

I have now tested this, and it worked fine. I have pushed a fix.

moxie0 · 2016-11-23T04:48:48Z

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

@@ -233,6 +233,8 @@ private static void sendSingleThreadNotification(@NonNull  Context context,
                       notificationState.getMarkAsReadIntent(context),
                       notificationState.getQuickReplyIntent(context, notifications.get(0).getRecipients()),
                       notificationState.getWearableReplyIntent(context, notifications.get(0).getRecipients()));
+    builder.addAndroidAutoAction(notificationState.getAndroidAutoReplyIntent(context, notifications.get(0).getRecipients()),
+                       notificationState.getAndroidAutoHeardIntent(context, notifications.get(0).getRecipients()), notifications.get(0).getTimestamp());


please align this. notificationState needs to be below notificationState on the line above.

I have pushed a fix for this.

//FREEBIE

This adds android auto support accordign to https://developer.android.com/training/auto/messaging/index.html#messaging However, since android auto is not officially supported in my country, the functionality is limited. Which means that I have not been able to fully test everything yet. What work is: * Message notification is shown. * When you click on it, the message is read. Closes #5880

moxie0 · 2016-11-28T03:33:38Z

in 3.24.0

riyapenn · 2016-12-13T22:41:50Z

One user is stating that with "SMS enabled, the notifications are doubling up. One from the Android system for SMS and the second from Signal." I suspect the SMS is being intercepted by the default messenger first. https://gist.github.com/fdc3e86e7ca5947959cef4235438c646

snaggen · 2016-12-14T08:33:19Z

I have seen this too. But I fail to see how this is a bug in Signal, since that is the application selected to handle SMS.... which it does. I think this is a bug in either Android Auto, Android or Google Messanger, since they clearly sends out a SMS notification when they should not.
Also, the documentation for the messaging doesn't mention any special treatment of SMS.
https://developer.android.com/training/auto/messaging/index.html

And... if Signal doesn't send the android auto notification, the possibility of replying to messages from within Signal, and marking the Signal conversation as read/heard will not be available. In short, the application handling the SMS, must send the notification to be able to mark messages as read within the application and reply within the conversation.

snaggen · 2016-12-14T08:55:17Z

Have asked about this on the android auto forum
https://productforums.google.com/forum/#!topic/android-auto/PJYVAA11OI0;context-place=forum/android-auto

snaggen · 2016-12-15T08:04:46Z

One person from the Android Auto Team replied, and confirmed that Android Auto overrides the users preferences regarding SMS handling. To work around the issue with duplicate SMS notifications he proposed the following:

If you switch off the SMS permission for Android Auto, you will only get the notifications from Signal which will handle the read and replies for SMS messages as you mentioned.

rubin110 · 2016-12-18T19:40:40Z

Is there some documentation on how this integration is supposed to work? Can we access Signal through the Android Auto app, or is this just to simply pass notifications and interact with Signal through there? Thanks!

snaggen · 2016-12-19T06:55:19Z

It is simply passing notifications and interacting through them. It is a straight forward implementation of the Android auto messaging api linked in the pull request. So take a look at that for info about how it works. Den sön 18 dec. 2016 20:40Rubin Starset <[email protected]> skrev:

…

Is there some documentation on how this integration is supposed to work? Can we access Signal through the Android Auto app, or is this just to simply pass notifications and interact with Signal through there? Thanks! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5880 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGH4ny5diEZI7AUTEQtqTy7IUwFHj59zks5rJYxAgaJpZM4K4We_> .

rubin110 · 2016-12-19T06:58:57Z

@snaggen: Thank you for the response. I ended up using it today while driving a friend to the airport. I think this is a great feature and will hopefully keep some people from texting and driving (and if it does theni it's worth it). Thank you for implementing this.

I searched around but couldn't find a good answer, is there any way to include Signal in the list of messaging apps Android Auto supports when you give it a voice commend to send a message to someone? Currently for me it only verbally lists Hangouts and SMS.

snaggen · 2016-12-19T09:08:52Z

@rubin110 I have not seen any Android Auto Api to define voice commands and things like that... what I currently have implemented seems to be what the api allows. But if you find out something about this that I have been missing, just let me know.

rubin110 · 2016-12-19T18:05:33Z

@sneggen: I went searching again and you're correct, it doesn't seem like Google has exposed an API to do this yet.

The behavior I'm talking about is when you issue the voice command "Send a message to Dana." The Android then verbal responds asking which messaging app to use and then lists them out, which for me is currently Hangouts and SMS.

This adds android auto support accordign to https://developer.android.com/training/auto/messaging/index.html#messaging However, since android auto is not officially supported in my country, the functionality is limited. Which means that I have not been able to fully test everything yet. What work is: * Message notification is shown. * When you click on it, the message is read. Closes signalapp/Signal-Android#5880 Upstream commit: signalapp/Signal-Android@9148b7d

FeuRenard · 2017-02-19T20:36:21Z

btw: #5619 should be closed (but it's locked, so I can't inform you there 😉 )

snaggen added 7 commits September 14, 2016 23:58

Explicitly set UnreadConversation Message and Timestamp

a9c6861

I didn't notice any difference from before, but the developer documentation explicitly states that it should be set. https://developer.android.com/training/auto/messaging/index.html#messaging

Fix the reply code to use OutgoingTextMessage

da36733

Merge branch 'master' into android_auto

34c60bc

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

11167ba

Adapt Android Auto Intents to upstream changes

86ea1fb

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

e74752a

moxie0 reviewed Nov 21, 2016

View reviewed changes

Android Auto: Fix codingstyle and add localization

bdf3c29

//FREEBIE

FeuRenard suggested changes Nov 22, 2016

View reviewed changes

Indentation changes according to feedback

1c7b17e

//FREEBIE

moxie0 reviewed Nov 23, 2016

View reviewed changes

snaggen added 2 commits November 24, 2016 11:53

Avoid exporting AndroidAutoReplyReceiver and ws fixes

bf4b40d

//FREEBIE

Don't export the AndroidAutoHeardReceiver

5e8c885

//FREEBIE

moxie0 closed this Nov 28, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android auto support #5880

Android auto support #5880

snaggen commented Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

moxie0 Nov 21, 2016

snaggen commented Nov 22, 2016

FeuRenard left a comment

FeuRenard Nov 22, 2016

FeuRenard Nov 22, 2016

snaggen commented Nov 22, 2016

moxie0 Nov 23, 2016

snaggen Nov 24, 2016

moxie0 Nov 24, 2016

snaggen Nov 24, 2016

moxie0 Nov 24, 2016

snaggen Nov 24, 2016

moxie0 Nov 23, 2016

snaggen Nov 24, 2016

moxie0 commented Nov 28, 2016

riyapenn commented Dec 13, 2016 •

edited

Loading

snaggen commented Dec 14, 2016 •

edited

Loading

snaggen commented Dec 14, 2016

snaggen commented Dec 15, 2016

rubin110 commented Dec 18, 2016

snaggen commented Dec 19, 2016 via email

rubin110 commented Dec 19, 2016

snaggen commented Dec 19, 2016

rubin110 commented Dec 19, 2016

FeuRenard commented Feb 19, 2017

Android auto support #5880

Android auto support #5880

Conversation

snaggen commented Nov 21, 2016

First time contributor checklist

Contributor checklist

Description

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snaggen commented Nov 22, 2016

FeuRenard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snaggen commented Nov 22, 2016

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

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

moxie0 commented Nov 28, 2016

riyapenn commented Dec 13, 2016 • edited Loading

snaggen commented Dec 14, 2016 • edited Loading

snaggen commented Dec 14, 2016

snaggen commented Dec 15, 2016

rubin110 commented Dec 18, 2016

snaggen commented Dec 19, 2016 via email

rubin110 commented Dec 19, 2016

snaggen commented Dec 19, 2016

rubin110 commented Dec 19, 2016

FeuRenard commented Feb 19, 2017

riyapenn commented Dec 13, 2016 •

edited

Loading

snaggen commented Dec 14, 2016 •

edited

Loading