Differential D8966 Diff 30361 src/org/kde/kdeconnect/BackgroundService.java

Changeset View

Standalone View

View Options

src/org/kde/kdeconnect/BackgroundService.java

Show All 14 Lines
15 * GNU General Public License for more details. 15​ * GNU General Public License for more details.
16 * 16​ *
17 * You should have received a copy of the GNU General Public License 17​ * You should have received a copy of the GNU General Public License
18 * along with this program. If not, see <http://www.gnu.org/licenses/>. 18​ * along with this program. If not, see <http://www.gnu.org/licenses/>.
19*/ 19​*/
20 20​
21package org.kde.kdeconnect; 21​package org.kde.kdeconnect;
22 22​
23​import android.app.Notification;
24​import android.app.NotificationManager;
25​import android.app.PendingIntent;
23import android.app.Service; 26​import android.app.Service;
24import android.content.Context; 27​import android.content.Context;
25import android.content.Intent; 28​import android.content.Intent;
26import android.content.IntentFilter; 29​import android.content.IntentFilter;
27import android.content.SharedPreferences; 30​import android.content.SharedPreferences;
28import android.os.Binder; 31​import android.os.Binder;
29import android.os.Build; 32​import android.os.Build;
30import android.os.IBinder; 33​import android.os.IBinder;
34​import android.support.v4.app.NotificationCompat;
35​import android.text.TextUtils;
31import android.util.Log; 36​import android.util.Log;
32 37​
33import org.kde.kdeconnect.Backends.BaseLink; 38​import org.kde.kdeconnect.Backends.BaseLink;
34import org.kde.kdeconnect.Backends.BaseLinkProvider; 39​import org.kde.kdeconnect.Backends.BaseLinkProvider;
35//import org.kde.kdeconnect.Backends.BluetoothBackend.BluetoothLinkProvider; 40​//import org.kde.kdeconnect.Backends.BluetoothBackend.BluetoothLinkProvider;
36import org.kde.kdeconnect.Backends.LanBackend.LanLinkProvider; 41​import org.kde.kdeconnect.Backends.LanBackend.LanLinkProvider;
37import org.kde.kdeconnect.Helpers.SecurityHelpers.RsaHelper; 42​import org.kde.kdeconnect.Helpers.SecurityHelpers.RsaHelper;
38import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper; 43​import org.kde.kdeconnect.Helpers.SecurityHelpers.SslHelper;
44​import org.kde.kdeconnect.UserInterface.MaterialActivity;
45​import org.kde.kdeconnect_tp.R;
39 46​
40import java.util.ArrayList; 47​import java.util.ArrayList;
41import java.util.HashSet; 48​import java.util.HashSet;
42import java.util.Set; 49​import java.util.Set;
43import java.util.concurrent.ConcurrentHashMap; 50​import java.util.concurrent.ConcurrentHashMap;
44import java.util.concurrent.locks.Lock; 51​import java.util.concurrent.locks.Lock;
45import java.util.concurrent.locks.ReentrantLock; 52​import java.util.concurrent.locks.ReentrantLock;
46 53​
47public class BackgroundService extends Service { 54​public class BackgroundService extends Service {
55​ public static final int FOREGROUND_NOTIFICATION_ID = 1;
48 56​
49 private static BackgroundService instance; 57​ private static BackgroundService instance;
50 58​
51 public interface DeviceListChangedCallback { 59​ public interface DeviceListChangedCallback {
52 void onDeviceListChanged(); 60​ void onDeviceListChanged();
53 } 61​ }
54 62​
55 private final ConcurrentHashMap<String, DeviceListChangedCallback> deviceListChangedCallbacks = new ConcurrentHashMap<>(); 63​ private final ConcurrentHashMap<String, DeviceListChangedCallback> deviceListChangedCallbacks = new ConcurrentHashMap<>();
▲ Show 20 Lines • Show All 73 Lines • ▼ Show 20 Line(s)136 public void unpaired() {
129 onDeviceListChanged(); 137​ onDeviceListChanged();
130 } 138​ }
131 }; 139​ };
132 140​
133 public void onDeviceListChanged() { 141​ public void onDeviceListChanged() {
134 for (DeviceListChangedCallback callback : deviceListChangedCallbacks.values()) { 142​ for (DeviceListChangedCallback callback : deviceListChangedCallbacks.values()) {
135 callback.onDeviceListChanged(); 143​ callback.onDeviceListChanged();
136 } 144​ }
145​
146​ //Update the foreground notification with the currently connected device list
147​ NotificationManager nm = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE);
148​ nm.notify(FOREGROUND_NOTIFICATION_ID, createForegroundNotification());
137 } 149​ }
138 150​
139 private void loadRememberedDevicesFromSettings() { 151​ private void loadRememberedDevicesFromSettings() {
140 //Log.e("BackgroundService", "Loading remembered trusted devices"); 152​ //Log.e("BackgroundService", "Loading remembered trusted devices");
141 SharedPreferences preferences = getSharedPreferences("trusted_devices", Context.MODE_PRIVATE); 153​ SharedPreferences preferences = getSharedPreferences("trusted_devices", Context.MODE_PRIVATE);
142 Set<String> trustedDevices = preferences.getAll().keySet(); 154​ Set<String> trustedDevices = preferences.getAll().keySet();
143 for (String deviceId : trustedDevices) { 155​ for (String deviceId : trustedDevices) {
144 //Log.e("BackgroundService", "Loading device "+deviceId); 156​ //Log.e("BackgroundService", "Loading device "+deviceId);
▲ Show 20 Lines • Show All 129 Lines • ▼ Show 20 Line(s)271 public void onCreate() {
274 //Link Providers need to be already registered 286​ //Link Providers need to be already registered
275 addConnectionListener(deviceListener); 287​ addConnectionListener(deviceListener);
276 288​
277 for (BaseLinkProvider a : linkProviders) { 289​ for (BaseLinkProvider a : linkProviders) {
278 a.onStart(); 290​ a.onStart();
279 } 291​ }
280 } 292​ }
281 293​
294​ private Notification createForegroundNotification() {
295​ Intent intent = new Intent(this, MaterialActivity.class);
296​ PendingIntent pi = PendingIntent.getActivity(this, 0, intent, PendingIntent.FLAG_UPDATE_CURRENT);
297​ NotificationCompat.Builder notification = new NotificationCompat.Builder(this);
298​ notification
299​ .setSmallIcon(R.drawable.ic_notification)
300​ .setColor(getResources().getColor(R.color.primary))
301​ .setContentTitle(getString(R.string.kde_connect))
302​ .setOngoing(true)
nicolasfellaUnsubmitted
Not Done

Why set a explicit color? I would go with the default and remove the line

nicolasfella: Why set a explicit color? I would go with the default and remove the line
mtijinkAuthorUnsubmitted
Not Done

This way the notification color ties in with the app color (orange), see https://developer.android.com/reference/android/app/Notification.html#color. More apps do this (and personally, I like it 😉).

If you disagree, I can change it.

mtijink: This way the notification color ties in with the app color (orange), see https://developer.
303​ .setContentIntent(pi)
304​ .setPriority(NotificationCompat.PRIORITY_MIN)
305​ .setShowWhen(false)
306​ .setAutoCancel(false);
307​
308​ ArrayList<String> connectedDevices = new ArrayList<>();
309​ for (Device device : getDevices().values()) {
310​ if (device.isReachable() && device.isPaired()) {
311​ connectedDevices.add(device.getName());
312​ }
313​ }
314​
315​ if (connectedDevices.isEmpty()) {
316​ notification.setContentText(getString(R.string.foreground_notification_no_devices));
317​ } else {
318​ notification.setContentText(getString(R.string.foreground_notification_devices, TextUtils.join(", ", connectedDevices)));
319​ }
320​
321​ return notification.build();
322​ }
nicolasfellaUnsubmitted
Not Done

Also I expect a method called createForegroundNotification to do nothing but creating the notification. This has nothing to do with creating but actually using the the notification, thus is a side-effect which should be avoided.

According to the docs this must be called after the service is started. Not sure if this is the case here (especially when createForegroundNotification is called from oncreate(). Maybe it should be moved to onStartCommand()

nicolasfella: Also I expect a method called createForegroundNotification to do nothing but creating the…
323​
282 void initializeSecurityParameters() { 324​ void initializeSecurityParameters() {
283 RsaHelper.initialiseRsaKeys(this); 325​ RsaHelper.initialiseRsaKeys(this);
284 SslHelper.initialiseCertificate(this); 326​ SslHelper.initialiseCertificate(this);
285 } 327​ }
286 328​
287 @Override 329​ @Override
288 public void onDestroy() { 330​ public void onDestroy() {
331​ stopForeground(true);
289 for (BaseLinkProvider a : linkProviders) { 332​ for (BaseLinkProvider a : linkProviders) {
290 a.onStop(); 333​ a.onStop();
291 } 334​ }
292 super.onDestroy(); 335​ super.onDestroy();
293 } 336​ }
294 337​
295 @Override 338​ @Override
296 public IBinder onBind(Intent intent) { 339​ public IBinder onBind(Intent intent) {
Show All 18 Lines355 public int onStartCommand(Intent intent, int flags, int startId) {
315 try { 358​ try {
316 for (InstanceCallback c : callbacks) { 359​ for (InstanceCallback c : callbacks) {
317 c.onServiceStart(this); 360​ c.onServiceStart(this);
318 } 361​ }
319 callbacks.clear(); 362​ callbacks.clear();
320 } finally { 363​ } finally {
321 mutex.unlock(); 364​ mutex.unlock();
322 } 365​ }
366​
367​ startForeground(FOREGROUND_NOTIFICATION_ID, createForegroundNotification());
323 return Service.START_STICKY; 368​ return Service.START_STICKY;
324 } 369​ }
325 370​
326 public static void Start(Context c) { 371​ public static void Start(Context c) {
327 RunCommand(c, null); 372​ RunCommand(c, null);
328 } 373​ }
329 374​
330 public static void RunCommand(final Context c, final InstanceCallback callback) { 375​ public static void RunCommand(final Context c, final InstanceCallback callback) {
Show All 18 Lines