Get Sunrise and Sunset times from CoreBrightness
thompsonate opened this issue · 16 comments
While I have a temporary solution in place that should determine the location if CoreLocation doesn't, getting the times from CoreBrightness will be more accurate. This will also mean Shifty will not need location permissions, which is a plus. I really don't want to take this out of beta without getting the times from the system, but I don't think I can figure this out myself.
For background, see discussion in #14
I'll see if I have time over Thanksgiving to look at this.
I also might take a look at it. I imagine this is better-suited to being written in Objective-C, since interacting with the objc runtime from Swift seems needlessly difficult.
It seems like the macOS SDK, for whatever reason, had chosen not to export SunriseSunsetProvider in the TBD file for CoreBrightness at /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/PrivateFrameworks/CoreBrightness.framework/CoreBrightness.tbd. I've fixed this in saagarjha@399784b –I'll take a look at getting the rest of the work a little bit later.
@saagarjha Hey just checking in. Did you end up making any more progress on this?
Sorry for the lack of an update–I did a bit of work on this, and I think I've figured out a way to do this without doing any sort of sunrise/sunset calculations or doing geolocation. I've just been delaying implementing it since it would require ripping out a significant portion of StatusMenuController and rewriting it to handle a bunch of edge cases (read: I'm still using a build from 7b5a12e, don't need any of the new features, and am too lazy to update it since it does what I need). I keep telling myself I'll finish it during the holidays, so we'll see :)
No problem. I'm just marching towards a 1.0 release and this is the last big thing that needs tackled before I feel it's ready to leave beta.
Why is StatusMenuController significantly affected? I know I've added more features, but I don't see how those would make you add edge cases to handle the sunrise and sunset times. As long as the sunrise and sunset times aren't nil, I don't really think there would be any problems.
Let me know what I can do to help.
I took a quick look at this since I saw @thompsonate comment on pushing an update today.
SunriseSunsetProvidermethod copySunsetSunriseInfo: return null because the method tries to make a call to the CoreDuet Context framework, but to retrieve information from that storage the application needs a special entitlement com.apple.coreduetd.context (which obviously can only be granted by Apple). So yeah, that's unfortunately a dead end.
But don't despair :D I found that it's actually super easy to obtain sunrise and sunset times using BrightnessSystemClient which is exported by the included TBP of the CoreBrightness framework, so no trickery need there.
Here's the sample code
//
// main.m
// test
//
// Created by Enrico Ghirardi on 01/01/2018.
// Copyright © 2018 Enrico Ghirardi. All rights reserved.
//
#import <Foundation/Foundation.h>
@class BrightnessSystemClientInternal;
@interface BrightnessSystemClient : NSObject
{
BrightnessSystemClientInternal *bsci;
}
- (void)registerNotificationBlock:(void (^)(void))callback forProperties:(NSArray *)properties;
- (void)registerNotificationBlock:(void (^)(void))callback;
- (BOOL)isAlsSupported;
- (id)copyPropertyForKey:(CFStringRef)key;
- (BOOL)setProperty:(id)property forKey:(CFStringRef)key;
- (void)dealloc;
- (id)init;
@end
int main(int argc, char *argv[])
{
BrightnessSystemClient *client = [[BrightnessSystemClient alloc] init];
NSDictionary *infos = [client copyPropertyForKey:(__bridge CFStringRef)@"BlueLightSunSchedule"];
NSDate *sunrise = [infos objectForKey:@"sunrise"];
NSDate *sunset = [infos objectForKey:@"sunset"];
NSLog(@"Sunrise %@", sunrise);
NSLog(@"Sunset %@", sunset);
[client release];
return 0;
}There are also other properties in that dictionary, yesterday and tomorrow sunrise and sunset days.
@choco Thank you so much! Would you mind adding the objc file in a PR?
@thompsonate I was putting off reading my emails so I missed your follow up. The real reason why I wanted to rewrite StatusMenuController was not because of the edge cases related to sunrise/sunset times, but because of the way it's written currently: the core logic is split up between a couple dozen functions, which makes it difficult to understand how one affects the other. In an event-driven model like this one, a lot of the bugs that arise are due to edge cases that we failed to consider. The simplest way I see of managing the complexity (and there is a lot; we need to do things like selectively cancel timers, update UIs, and handle sleep) is simply building a huge table of every possible response to an event given the current state (which is why this was taking a long time) and coding it up as a large conditional. This way, if something breaks, you can fix it by just going to the table and seeing what you forgot to do. In effect, we'd have a bunch of event functions that flipped appropriate flags in our "stored state", then called a function that would just "do the right thing" based on the current event. Updating the UI would be trivial since it could be done once and any changes gleaned directly from the current state. Basically, the current functions were difficult to update, so I'm trying to make it so that fixing bugs or changing details in the future doesn't make me chase every edge case to make sure it works correctly. I'm not sure if I'm making sense, so please let me know if you need more clarification.
Regarding getting sunrise and sunset from CoreBrightness, I see that @choco has already found a way that's not gated behind CoreDuetContext entitlements. You are free to use this solution, but I've tried to pivot away from this towards dropping any sort of reliance on location or knowledge of the current schedule state whatsoever. The reason for this is that we can reduce dependency on CoreBrightness. One, it's a private API so less reliance on it is always better, but two, and more seriously, we currently need to use hacks when we're doing any sort of scheduling because Night Shift does things like "ramp up" to full strength and whatnot. I'd rather us have CoreBrightness do the the heavy lifting in determining whether we "should be" in Night Shift or not since it can take all of these into account without hacks and using the actual system-provided values. By setting the schedule back to its original value and careful measurements of the Night Shift status before and after, we should be able to determine the "underlying schedule" without any use of sunrise/sunset data and match the system behavior exactly.
I agree that the current implementation isn't great and that it would be better served by something that more resembles a state machine. I have no idea where to start with something like this, but if you'd like to take the lead on this, I'd love to make this work. It's pretty hard to understand currently and it doesn't help that it was originally written for a much smaller functionality then more and more features were added.
As for relying on private APIs, less is definitely better, but I don't see how we'd be able to Night Shift's scheduled state otherwise. There are certain times Night Shift needs to be set to the scheduled state, like after it's been disabled with Shifty. For example, if it's disabled while it's scheduled to be switched from on to off, Shifty wouldn't know that it's scheduled to be left off since there's no BlueLightNotification in this case, as it's already disabled.
It's pretty hard to understand currently and it doesn't help that it was originally written for a much smaller functionality then more and more features were added.
You hit the nail on the head–this is exactly what I had to deal with.
There are certain times Night Shift needs to be set to the scheduled state, like after it's been disabled with Shifty. For example, if it's disabled while it's scheduled to be switched from on to off, Shifty wouldn't know that it's scheduled to be left off since there's no BlueLightNotification in this case, as it's already disabled.
Would this be an issue? Night Shift will remain disabled like it should, at least until a scheduled notification comes along to reenable Night Shift (unless there is none? I was under the impression that there was, since it's how I remember Apple's switch in System Preferences working, but I may be wrong). If this isn't the case, we might have to tap into some more of CoreBrightness's API for advanced scheduling to deal with things like timers and user disabling–I believe NightModeControl may be useful, but hopefully it doesn't come to that.
Either way, it's pretty obvious that we really need to think about what happens in cases like these, so I'm going to sit down and make a big ol' table of every possible case and what we need to do to response. Once we have that, we should be able to translate it into code relatively easily.
I was actually working on refactoring Shifty pretty hard almost a month ago, but I kept that on hold since other projects have taken priority.
I was trying to port Shifty to the ReSwift paradigm since it seems to fit pretty well the use case. First thing I did was cleaning up around the edges, and basically refactoring out code from where it wasn't supposed to be (before separating it again), adding some consistency. I plan on working some more on this in the near future, simply because I grew interest in ReSwift and wanted to try it out, but it may be also useful to others https://github.com/choco/Shifty/commits/refactor
@saagarjha There are notifications when Night Shift switches states based on the schedule. What I was talking about is a specific case when you disable for time/apps/whatever in Shifty. In that case, since it's off, you wouldn't get off notification at sunrise because it's already disabled. This is why I don't see how we could completely abandon the private BrightnessSystemClient API, but it wouldn't be a bad idea to at least fall back on a different method.
@choco I hadn't realized that's the kind of refactor you were doing. I don't know anything about ReSwift other than what I just skimmed from the Readme, but it seems like that's what we're looking for.
This conversation should probably be taken somewhere more appropriate. I just created #36, but maybe a Slack or a group iMessage would work better. Let me know what you guys think.
There are notifications when Night Shift switches states based on the schedule. What I was talking about is a specific case when you disable for time/apps/whatever in Shifty. In that case, since it's off, you wouldn't get off notification at sunrise because it's already disabled.
When you turn Night Shift off manually from System Preferences (or Notification Center) while the schedule is left enabled, it reverts back to the schedule eventually, doesn't it? Am I just remembering this wrong?
maybe a Slack or a group iMessage would work better
I'd prefer iMessage over Slack because it's native and easier to access, but I'm available for either through the email on my profile.
When you turn Night Shift off manually from System Preferences (or Notification Center) while the schedule is left enabled, it reverts back to the schedule eventually, doesn't it? Am I just remembering this wrong?
Yes, it does. You’re correct about that. In this case, I think it’s basically turning it off until it’s scheduled to come back on again. But if you’re disabling it in Shifty, you want it to come back on sooner than that, whether it’s in an hour or when you switch apps.
Originally, I was setting night shift to on after the timer expired. Then I realized that I didn’t have any way of knowing if there was a sunrise during that time since there’s no notification when it thinks it should be off and it already is.
Bleh this is so difficult to talk about.
I’d prefer iMessage too, but I usually have Slack open on my Mac anyways.
Yes, this is the case where we can avoid using BrightnessSystemClient if we're clever (at least, I thought it was clever; you may disagree).
In this case, I think it’s basically turning it off until it’s scheduled to come back on again.
Yep, I'd expect there to be a notification at sunset that it responds to. Keep in mind that I haven't checked this thoroughly, but I don't see any sort of "special case" code that handles for any other way of doing it so I assume this is how it's done.
But if you’re disabling it in Shifty, you want it to come back on sooner than that, whether it’s in an hour or when you switch apps.
Yes–for example, if you have a timer expire, etc. Note, however, that 1. setting this up must be done with Shifty and 2. leaving this state is also done with Shifty. There is no way to change states here without Shifty knowing.
Originally, I was setting night shift to on after the timer expired.
Yeah, we shouldn't do that, since it might leave Night Shift on in the middle of the day.
Then I realized that I didn’t have any way of knowing if there was a sunrise during that time since there’s no notification when it thinks it should be off and it already is.
This is where we abuse the API we have currently: to check if we're supposed to have Night Shift on, we can just retrieve the current schedule and immediately "set" it as the schedule again. This should cause macOS to update the Night Shift state to what it "should" be. Internally it's probably querying the sunrise/sunset times, but we don't have to deal with it.
Bleh this is so difficult to talk about.
Agreed.