From 6b9745c8000dd76849d7ac14cc104ea2f3b793d6 Mon Sep 17 00:00:00 2001 From: Catfriend1 Date: Thu, 11 Jul 2019 12:23:17 +0200 Subject: [PATCH] Reduce unnecessary applyCustomRunConditions after service state change (#435) Ref.: #https://github.com/Catfriend1/syncthing-android/pull/182/commits/7e7b2c9850d13a5102558b8ee2d2d8d1175b7ca0 Improve RunConditionMonitor#updateShouldRunDecision ( https://github.com/Catfriend1/syncthing-android/pull/182/commits/7e7b2c9850d13a5102558b8ee2d2d8d1175b7ca0) --- .../service/RunConditionMonitor.java | 41 ++++++++++++++----- .../service/SyncthingService.java | 26 +++--------- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/com/nutomic/syncthingandroid/service/RunConditionMonitor.java b/app/src/main/java/com/nutomic/syncthingandroid/service/RunConditionMonitor.java index 7fbda59d..b3823928 100644 --- a/app/src/main/java/com/nutomic/syncthingandroid/service/RunConditionMonitor.java +++ b/app/src/main/java/com/nutomic/syncthingandroid/service/RunConditionMonitor.java @@ -253,19 +253,39 @@ public class RunConditionMonitor { * We then need to decide if syncthing should run. */ public void updateShouldRunDecision() { - // Check if the current conditions changed the result of decideShouldRun() - // compared to the last determined result. boolean newShouldRun = decideShouldRun(); - if (newShouldRun != lastDeterminedShouldRun) { - if (mOnShouldRunChangedListener != null) { - mOnShouldRunChangedListener.onShouldRunDecisionChanged(newShouldRun); - } - lastDeterminedShouldRun = newShouldRun; + if (newShouldRun) { + /** + * Trigger: + * a) Sync pre-conditions changed + * a1) AND SyncthingService.State should remain ACTIVE + * a2) AND SyncthingService.State should transition from INIT/DISABLED to ACTIVE + * b) Sync pre-conditions did not change + * b1) AND SyncthingService.State should remain ACTIVE + * because a reevaluation of the run conditions was forced from code. + * Action: + * SyncthingService will evaluate custom per-object run conditions + * and pause/unpause objects accordingly. + */ + if (mOnSyncPreconditionChangedListener != null) { + mOnSyncPreconditionChangedListener.onSyncPreconditionChanged(this); + } } - // Notify about changed preconditions. - if (mOnSyncPreconditionChangedListener != null) { - mOnSyncPreconditionChangedListener.onSyncPreconditionChanged(this); + /** + * Check if the current conditions changed the result of decideShouldRun() + * compared to the last determined result. + */ + if (newShouldRun != lastDeterminedShouldRun) { + /** + * Notify SyncthingService in case it has to transition from + * a) INIT/DISABLED => STARTING => ACTIVE + * b) ACTIVE => DISABLED + */ + if (mOnShouldRunChangedListener != null) { + mOnShouldRunChangedListener.onShouldRunDecisionChanged(newShouldRun); + lastDeterminedShouldRun = newShouldRun; + } } } @@ -461,6 +481,7 @@ public class RunConditionMonitor { /** * Check if an object's individual sync conditions are met. + * Precondition: Object must own pref "...CustomSyncConditionsEnabled == true". */ public Boolean checkObjectSyncConditions(String objectPrefixAndId) { // Sync on mobile data? diff --git a/app/src/main/java/com/nutomic/syncthingandroid/service/SyncthingService.java b/app/src/main/java/com/nutomic/syncthingandroid/service/SyncthingService.java index 964c06b2..7ceea6db 100644 --- a/app/src/main/java/com/nutomic/syncthingandroid/service/SyncthingService.java +++ b/app/src/main/java/com/nutomic/syncthingandroid/service/SyncthingService.java @@ -252,19 +252,6 @@ public class SyncthingService extends Service { return START_NOT_STICKY; } - /** - * Send current service state to listening endpoints. - * This is required that components know about the service State.DISABLED - * if RunConditionMonitor does not send a "shouldRun = true" callback - * to start the binary according to preferences shortly after its creation. - * See {@link mLastDeterminedShouldRun} defaulting to "false". - */ - if (mCurrentState == State.DISABLED) { - synchronized (mStateLock) { - onServiceStateChange(mCurrentState); - } - } - if (mPrefBroadcastServiceControl) { Log.i(TAG, "onStartCommand: mPrefBroadcastServiceControl == true, RunConditionMonitor is disabled."); /** @@ -584,9 +571,6 @@ public class SyncthingService extends Service { } onServiceStateChange(State.ACTIVE); } - if (mRestApi != null && mRunConditionMonitor != null) { - mRestApi.applyCustomRunConditions(mRunConditionMonitor); - } if (mEventProcessor == null) { mEventProcessor = new EventProcessor(SyncthingService.this, mRestApi); @@ -705,8 +689,10 @@ public class SyncthingService extends Service { * @see #unregisterOnServiceStateChangeListener */ public void registerOnServiceStateChangeListener(OnServiceStateChangeListener listener) { - // Make sure we don't send an invalid state or syncthing might show a "disabled" message - // when it's just starting up. + /** + * Initially send the current state to the new subscriber to make sure it doesn't stay + * in undefined state forever until the state next change occurs. + */ listener.onServiceStateChange(mCurrentState); mOnServiceStateChangeListeners.add(listener); } @@ -726,9 +712,9 @@ public class SyncthingService extends Service { private void onServiceStateChange(State newState) { if (newState == mCurrentState) { Log.d(TAG, "onServiceStateChange: Called with unchanged state " + newState); - } else { - Log.i(TAG, "onServiceStateChange: from " + mCurrentState + " to " + newState); + return; } + Log.i(TAG, "onServiceStateChange: from " + mCurrentState + " to " + newState); mCurrentState = newState; mHandler.post(() -> { mNotificationHandler.updatePersistentNotification(this);