Don't try to get location if sunset based scheduling is disabled

[jonashaag]

Reproduce: Enable only brightness based mode switching.

Expected behaviour: Dynamic Dark Mode works without trying to retrieve location (as it's unused), or at least doesn't complain if location can't be retrieved.

Actual behaviour: Dynamic Dark Mode complains that it can't retrieve location.

[ApolloZhu]

Hmmm, #76 also mentioned about this. I’ll try to figure out what went wrong as soon as possible

[jonashaag]

Hey @ApolloZhu can I buy you a cup of coffee to work on this with higher prio? :-)

[ApolloZhu]

Hi @jonashaag, thank you for your interest in this project and support. But unfortunately, I'm studying in the US under F1 visa, which means I can't be employed (or do any work for some kind of return) without permission.

However, there is a solution: issuehunt. You can fund this issue at https://issuehunt.io/r/ApolloZhu/Dynamic-Dark-Mode/issues/78. Then, whoever solves your problem can get that bounty.

That aside, I'm on winter break now so I should be able to find some time to work on this, hopefully soon.

[jonashaag]

So, the problem is that scheduler always checks location on system events like clock updated, and the singleton scheduler instance is always created even if it is unused.

Here's a patch that fixes this mostly. It does not work correctly when you switch from location based to brightness-only based mode, since in that case the scheduler instance is not stopped.

diff --git a/Dynamic/Auxiliary/Connectivity.swift b/Dynamic/Auxiliary/Connectivity.swift
index 0bca4b4..81a7b68 100644
--- a/Dynamic/Auxiliary/Connectivity.swift
+++ b/Dynamic/Auxiliary/Connectivity.swift
@@ -55,7 +55,7 @@ public final class Connectivity {
     public func scheduleWhenReconnected() {
         startObserving { [weak self] in
             self?.task = Plan.after(5.seconds).do(queue: .main) {
-                Scheduler.shared.schedule()
+                Scheduler.getSingleton().schedule()
             }
         }
     }
diff --git a/Dynamic/Components/AppleInterfaceStyle+Coordinator.swift b/Dynamic/Components/AppleInterfaceStyle+Coordinator.swift
index a89e297..ed544bb 100644
--- a/Dynamic/Components/AppleInterfaceStyle+Coordinator.swift
+++ b/Dynamic/Components/AppleInterfaceStyle+Coordinator.swift
@@ -41,11 +41,11 @@ public class AppleInterfaceStyleCoordinator: NSObject {
             return ScreenBrightnessObserver.shared.startObserving()
         }
         Connectivity.default.scheduleWhenReconnected()
-        Scheduler.shared.schedule(startBrightnessObserverOnFailure: true)
+        Scheduler.getSingleton().schedule(startBrightnessObserverOnFailure: true)
     }

     public func tearDown(stopAppearanceObservation: Bool = true) {
-        Scheduler.shared.cancel()
+        Scheduler.getSingleton().cancel()
         Connectivity.default.stopObserving()
         ScreenBrightnessObserver.shared.stopObserving()
         if stopAppearanceObservation {
diff --git a/Dynamic/Components/Preferences.swift b/Dynamic/Components/Preferences.swift
index 791e1cc..42f0c81 100644
--- a/Dynamic/Components/Preferences.swift
+++ b/Dynamic/Components/Preferences.swift
@@ -72,10 +72,10 @@ extension Preferences {
             observe(\.scheduled) { change in
                 if change.newValue == true {
                     if #available(OSX 10.15, *), preferences.AppleInterfaceStyleSwitchesAutomatically { return }
-                    Scheduler.shared.schedule()
+                    Scheduler.getSingleton().schedule()
                     Connectivity.default.scheduleWhenReconnected()
                 } else {
-                    Scheduler.shared.cancel()
+                    Scheduler.getSingleton().cancel()
                     Connectivity.default.stopObserving()
                 }
             },
@@ -91,22 +91,22 @@ extension Preferences {
                         if SLSGetAppearanceThemeSwitchesAutomatically() {
                             SLSSetAppearanceThemeSwitchesAutomatically(false)
                         } else if preferences.scheduled, change.oldValue == Zenith.system.rawValue {
-                            return Scheduler.shared.updateSchedule { _ in }
+                            return Scheduler.getSingleton().updateSchedule { _ in }
                         }
                     }
                 }
                 if preferences.scheduled {
-                    Scheduler.shared.schedule()
+                    Scheduler.getSingleton().schedule()
                 } // else do nothing
             },
             observe(\.scheduleStart) { _ in
                 if preferences.scheduled && !preferences.scheduleZenithType.hasSunriseSunsetTime {
-                    Scheduler.shared.schedule()
+                    Scheduler.getSingleton().schedule()
                 }
             },
             observe(\.scheduleEnd) { _ in
                 if preferences.scheduled && !preferences.scheduleZenithType.hasSunriseSunsetTime {
-                    Scheduler.shared.schedule()
+                    Scheduler.getSingleton().schedule()
                 }
             },
             observe(\.showToggleInTouchBar, observeInitial: true) { change in
diff --git a/Dynamic/Components/Scheduler/Scheduler.swift b/Dynamic/Components/Scheduler/Scheduler.swift
index a53fe75..241451e 100644
--- a/Dynamic/Components/Scheduler/Scheduler.swift
+++ b/Dynamic/Components/Scheduler/Scheduler.swift
@@ -12,7 +12,14 @@ import Solar
 import Schedule

 public final class Scheduler: NSObject {
-    public static let shared = Scheduler()
+    public static var singleton: Scheduler? = nil
+    
+    public static func getSingleton() -> Scheduler {
+        if singleton == nil {
+            singleton = Scheduler()
+        }
+        return singleton!
+    }

     private var task: Task?

diff --git a/Dynamic/View Controller/Settings/SettingsViewController.swift b/Dynamic/View Controller/Settings/SettingsViewController.swift
index d5e6b7c..1b992c5 100644
--- a/Dynamic/View Controller/Settings/SettingsViewController.swift 
+++ b/Dynamic/View Controller/Settings/SettingsViewController.swift 
@@ -55,7 +55,7 @@ class SettingsViewController: NSViewController {
         preferences.removePersistentDomain(forName: name)
         Preferences.stopObserving()
         StatusBarItem.only.stopObserving()
-        Scheduler.shared.cancel()
+        Scheduler.getSingleton().cancel()
         ScreenBrightnessObserver.shared.stopObserving()
         close(self)
         Welcome.show()

[ApolloZhu]

@jonashaag this project is already using the standard Swift singleton implementation, but thanks!

I was trying to refactor the decision making part over the break but it was more complicated than I thought it was. I guess I'll just find a simple fix in the near future for now.

[jonashaag]

@jonashaag this project is already using the standard Swift singleton implementation, but thanks!

Difference is that the implementation using getSingleton is "lazy" i.e. it doesn't create an instance unless it's needed

[ApolloZhu]

To cite https://docs.swift.org/swift-book/LanguageGuide/Properties.html#ID264:

Stored type properties are lazily initialized on their first access. They are guaranteed to be initialized only once, even when accessed by multiple threads simultaneously, and they do not need to be marked with the lazy modifier.

[jonashaag]

Oh, nice, didn't know that and should've done more research before making the statement above!

I wonder why my patch changes anything in terms of behaviour then (it did in my tests)

[jonashaag]

Hey @ApolloZhu just wondering if this bug fix will ever make it into the program, otherwise I'll continue using my fork.

[whazor]

Location Services uses Wi-Fi to establish the location and quickly interrupts your internet connection in this process. Very annoying for video calls or cloud gaming. I would love to have an alternative way of configuring my location.