thompsonate / Shifty

☀️ A macOS menu bar app that gives you more control over Night Shift.
http://shifty.natethompson.io
GNU General Public License v3.0
1.24k stars 33 forks source link

Shifty 0.3 is unable to get location #14

Closed nicolinuxfr closed 6 years ago

nicolinuxfr commented 6 years ago

On my Mac, using latest macOS Sierra version, I can't use the auto-activation of Night Shift based on location. I only have this message when I choose the option :

capture d ecran 2017-11-02 a 13 26 11

I checked in System Preference, but Shifty can indeed access my location. And my Mac is located correctly in Plans, for instance.

Let me know if you need more information.

thompsonate commented 6 years ago

This alert is shown when you have allowed location permissions but the location can't be retrieved, likely because of an internet connection issue. This definitely shouldn't be showing when another app can get your location.

Just to confirm, if you press "Try again" does the alert keep popping back up? Also, have you tried restarting your computer?

nicolinuxfr commented 6 years ago

Yes, I tried several times and the error was still popping back up every time.

I thought of a system wide issue, but Plans can access my location at the same time without any problem. I did not try to reboot my Mac yet.

thompsonate commented 6 years ago

Let me know if a reboot changes anything. I tested this pretty extensively when I implemented it and didn't run into an issue. I'll test it again later today when I get a chance.

Moolfreet commented 6 years ago

Same issue on my macosSierra up to date. A reboot didn’t change anything. Switched back to the previous version that work perfectly.

thompsonate commented 6 years ago

@Moolfreet @nicolinuxfr You are both running Sierra 10.12.x, correct? I probably should have tested it in Sierra.

Moolfreet commented 6 years ago

Yes, Sierra 10.12.6

Le 2 nov. 2017 à 19:50, Nate Thompson notifications@github.com a écrit :

@Moolfreet @nicolinuxfr You are both running Sierra 10.12.x, correct? I probably should have tested it in Sierra.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

-- Jean

nicolinuxfr commented 6 years ago

Same here.

thompsonate commented 6 years ago

Fortunately, I was able to reproduce the issue in 10.12.6. Hopefully I can get a fix out soon.

saagarjha commented 6 years ago

I'm getting this on macOS High Sierra 10.13.2 Beta (17C60c) as well, just FYI.

thompsonate commented 6 years ago

Thanks for the heads up @saagarjha. I wonder what's different between the versions since it's working fine in 10.13.0 and 10.13.1 as far as I can tell.

I've looked into it some but haven't found the issue yet. If you'd like to take a look at it, I'll be eternally grateful.

saagarjha commented 6 years ago

Why do we need location access, anyways? Can't we get sunrise/sunset times from CoreBrightness?

thompsonate commented 6 years ago

No, the schedule in CBBlueLightClient, which is in CoreBrightness, only contains the times set for the Custom schedule. I guess sunrise and sunset times could be hidden elsewhere in CoreBrightness or another framework though.

saagarjha commented 6 years ago

-[SunriseSunsetProvider copySunsetSunriseInfoFromContext] in CoreBrightness looks very promising.

thompsonate commented 6 years ago

That looks interesting. The problem is we have no idea what that function returns. If we can figure out how to get the times from the framework though, it'll be both easier and more accurate.

cc: @calda

saagarjha commented 6 years ago

I'm 99% sure it's an NSDictionary:

screen shot 2017-11-06 at 01 17 43

A quick peek in lldb should be enough to figure it out.

thompsonate commented 6 years ago

I tried but I keep running into this error. I'm not that familiar with this stuff.

screen shot 2017-11-06 at 4 50 35 am
saagarjha commented 6 years ago

Let me take a look. How often do you need sunrise/sunset updates?

thompsonate commented 6 years ago

Ideally I would have a computed property that checks the system for the times whenever I need them. Otherwise, it would have to be once daily and whenever the user’s location changes significantly.

saagarjha commented 6 years ago

Actually, I have an even bigger question: why does Shifty have to know when sunset/sunrise is? Why can't it just have Night Shift itself handle it?

nicolinuxfr commented 6 years ago

Same here : I thought the idea was to complement Night Shift, leaving the system decide when to activate or not.

If possible, wouldn’t it be just simpler to let macOS decide when it needs to use Night Shift ?

thompsonate commented 6 years ago

Two reasons:

  1. When Night Shift is disabled in Shifty and the timer ends, the scheduled state could be off and I previously had no way of knowing if that was the case.
  2. I wanted dark mode to match the schedule instead of just Night Shift’s state. Matching dark mode’s state with Night Shift’s was especially weird when it’s disabled for an app and you’re switching back and forth between apps.

I’m still relying on Night Shift to set its state based on the schedule. I just want to be able to tell what the schedule is.

saagarjha commented 6 years ago

I'm pretty sure CoreBrightness gives us enough information to do this without performing any calculations ourselves. Let me see if I can figure it out.

thompsonate commented 6 years ago

That would be great. I'd love to get rid of checking the location and calculating the times ourselves.

saagarjha commented 6 years ago

Just thought I'd give you a brief update of the work I've done on this. I'm getting the same linker error as you are, but dynamically loading SunriseSunsetProvider at runtime, using dlopen/NSClassFromString seems to be working. I'm not quite sure why the symbol can't be found, since nm shows it just fine–I'll take a look at why this is happening later. Regarding actually retrieving the sunrise/sunset information, the setup of a SunriseSunsetProvider seems to be a lot more complicated than I originally thought. It appears to be communicating with CoreDuet, macOS's general "knowledge base"/proactive/Continuity service (at least, that's what I think it is–there isn't a whole lot of information about it online), which probably allows it to get the location or whatever is needed to perform solar updates. Unfortunately this is failing when I try to do this, probably because we need to do something specific to make CoreDuet happy. Without proper initialization, -[SunriseSunsetProvider copySunsetSunriseInfoFromContext] returns nil. I'm going to take a quick break from working on this since I have something else I need to work on, but I'll take another look in a couple days.

thompsonate commented 6 years ago

I looked at it with my friend @LumingYin and got as far as loading SunriseSunsetProvider too. I meant to let you know so you wouldn't have to figure out the same thing.

It's definitely more complicated than I thought it would be. I don't think @LumingYin made it any farther than that, but if you happen to be working on it at the same time, maybe you can bounce ideas off of each other.

Thanks for trying to figure this out.

thompsonate commented 6 years ago

I decided to use a (hopefully) temporary solution to fix this problem. If the location can't be determined with CoreLocation, I'm using an API that will approximate the location from the computer's IP address.