After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 682113 - Use sunrise/sunset times to determine day and night
Use sunrise/sunset times to determine day and night
Status: RESOLVED FIXED
Product: gnome-clocks
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Clocks maintainer(s)
Clocks maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-08-17 17:32 UTC by Alex Anthony
Modified: 2012-08-25 20:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Forgot to add patch (787 bytes, patch)
2012-08-17 17:33 UTC, Alex Anthony
none Details | Review
Changed to add a separate function in utils (2.20 KB, patch)
2012-08-17 19:07 UTC, Alex Anthony
none Details | Review
Just adds a test case using gweather (1.34 KB, patch)
2012-08-18 12:40 UTC, Alex Anthony
none Details | Review
Very rough patch (8.11 KB, patch)
2012-08-18 16:48 UTC, Alex Anthony
none Details | Review

Description Alex Anthony 2012-08-17 17:32:59 UTC
At the moment it tries to use a non-existent AM/PM part of the time string and anyway, uses AM = day, PM = night.

Patch corrects it to use the same margins as clocks (8am-7pm is day).
Seems as valid a time as any for day/night, unless we want to use sunrise/sunset times (which we don't fetch yet)
Comment 1 Alex Anthony 2012-08-17 17:33:23 UTC
Created attachment 221646 [details] [review]
Forgot to add patch
Comment 2 Alex Anthony 2012-08-17 19:07:00 UTC
Created attachment 221666 [details] [review]
Changed to add a separate function in utils

As discussed on IRC, now adds a function in utils. Useful for if we use sunrise/sunset later.
Comment 3 Paolo Borelli 2012-08-17 19:41:49 UTC
I changed the patch a bit and pushed. Thanks!
Comment 4 Allan Day 2012-08-17 21:36:38 UTC
I haven't been following the conversation, but shouldn't day/night reflect whether the sun is up?
Comment 5 Alex Anthony 2012-08-17 21:47:30 UTC
(In reply to comment #4)
> I haven't been following the conversation, but shouldn't day/night reflect
> whether the sun is up?

Using standard times is temporary until fetching sunrise/sunset times is implemented
Comment 6 Paolo Borelli 2012-08-17 22:13:16 UTC
(In reply to comment #4)
> I haven't been following the conversation, but shouldn't day/night reflect
> whether the sun is up?

The bug was about alarm not respecting day/night at all, and that is now fixed using the same logic used in the world clock (that is from 7pm to 7pm is day).


However since you bring that up, let's hijack this bug and make it about using the rigth sunrise/sunset times. I think we can get that information from libgweather (which we already use): see the weather_info_get_sunrise/sunset

http://developer.gnome.org/libgweather/stable/libgweather-weather.html#WeatherInfo
Comment 7 Alex Anthony 2012-08-17 22:20:19 UTC
(In reply to comment #6)
> (In reply to comment #4)
> > I haven't been following the conversation, but shouldn't day/night reflect
> > whether the sun is up?
> 
> The bug was about alarm not respecting day/night at all, and that is now fixed
> using the same logic used in the world clock (that is from 7pm to 7pm is day).
> 
> 
> However since you bring that up, let's hijack this bug and make it about using
> the rigth sunrise/sunset times. I think we can get that information from
> libgweather (which we already use): see the weather_info_get_sunrise/sunset
> 
> http://developer.gnome.org/libgweather/stable/libgweather-weather.html#WeatherInfo

That's what I'm looking at at the moment. It's not playing nice at the moment.

I think we're going to have to require libgweather 3.5.x. The GWeatherInfo class doesn't seem to be introspected in 3.4.1
Comment 8 Alex Anthony 2012-08-18 12:40:58 UTC
Created attachment 221694 [details] [review]
Just adds a test case using gweather

Just adds a little function for now, but it fails.
Each time get_sunrise_sunset is run, it outputs this: http://pastebin.com/3esQPwwf

This happens with libgweather 3.5.5 on Arch and with git master in jhbuild (so built against master libsoup too)
Comment 9 Giovanni Campagna 2012-08-18 12:51:20 UTC
Ok, so the problem with that warning is that you're calling .update() twice.
The expected usage of the API from introspected binding is:
info = GWeather.Info(location=location, world=world)
info.connect('updated', ...)
info.update()

(yes, you must pass the world explicitly if you pass a location, the C API is wrong...)

In any case, this should not affect get_sunrise() / get_sunset(). Investigating...
Comment 10 Giovanni Campagna 2012-08-18 13:06:28 UTC
Ok found the problem: the info is invalid until it emits the 'updated' signal (which is after reading METAR data).
You need to listen for that.
Comment 11 Alex Anthony 2012-08-18 16:48:42 UTC
Created attachment 221705 [details] [review]
Very rough patch

This patch makes it fetch the sunrise and sunset times, and uses them for the DigitalClockStandalone and determining whether to use the day/night image.

At the moment, it's specific for the world clock part. Would need to define a current location to use this for alarms, and we're considering not having day/night images for alarms anyway.

Code needs a cleanup. It calls the DigitalClock.update function, rather than the other way around. Maybe update() needs splitting into parts, a quick part to update the time and a part to do less for other times.

Also, uses the same logic for changing sunrise and sunset times to other locations time as the current time, so is also subject to https://bugzilla.gnome.org/show_bug.cgi?id=682167
I have rolled this logic out into a different function though.
Comment 12 Seif Lotfy 2012-08-25 20:03:20 UTC
Ok so I updated your patches and pushed them (Sorry for forgetting about it). Thanks so much for that. It is actually useful now :D
Cheers
Seif