Review Request: Fix the 0600GMT=sunrise, 1800GMT=sunset bug in the BBC Weather Ion

[Petri Damstén]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/684/#review1147
-----------------------------------------------------------


Would it have been easier to just use time datengine? There is code in libplasmaweather that determines if it's day and it's just two lines:
        Plasma::DataEngine::Data data = timeEngine->query(
                QString("Local|Solar|Latitude=%1|Longitude=%2").arg(latitude).arg(longitude));
        bool day = (data["Corrected Elevation"].toDouble() > 0.0);
Dataengines don't support calling other dataengines like wallpapers and applets do, but a copy of dataengineconsumer_p.h should take care of that (Could this be a public header?). Also if moon rise/set (http://reviewboard.kde.org/r/680/) gets in at some point it would not need any changes in weather ions if time dataengine is used.


- Petri


On 2009-05-15 15:59:05, Andrew Coles wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/684/
> -----------------------------------------------------------
> 
> (Updated 2009-05-15 15:59:05)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> To give an icon depicting the current weather, the BBC UK Met. Office backend decides whether or not it is day or night using a simple rule: if it's after 0600GMT but before 1800GMT, it is daytime; otherwise, it's night time.  This causes two known issues:
> 
> i) For people in the UK, the weather icons indicate night even though sunset is not until well after 2000GMT at this time of year
> ii) For people outside the UK, day and night are set to an approximation of what they are in the UK.  So, for instance, for people in New York, the weather icon goes to night at lunchtime.
> 
> The attached diff fixes this bug by calculating the actual sunrise/sunset times for the weather location, and using /these/ to decide whether it's day or night.  How is this done?  Whilst the BBC don't provide sunrise/sunset time in the current observations, they do provide latitude and longitude information.  Given we know:
> 
> i) The date
> ii) The latitude/longitude
> 
> ...we can then calculate sunrise and sunset times using a bit of maths.  If you look, the diffs for the ion itself are very small: just enough to handle the lat/long information, and then calls to sunrise/sunset calculation to decide whether it's day or night.  This code has been reused from the time DataEngine - so we're not importing lots of new code at this point.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/dataengines/time/CMakeLists.txt 968518 
>   /trunk/KDE/kdebase/workspace/plasma/dataengines/time/solarposition.h 968518 
>   /trunk/KDE/kdebase/workspace/plasma/dataengines/time/time_solar_export.h PRE-CREATION 
>   /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/CMakeLists.txt 968518 
>   /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_bbcukmet.h 968518 
>   /trunk/KDE/kdebase/workspace/plasma/dataengines/weather/ions/ion_bbcukmet.cpp 968518 
> 
> Diff: http://reviewboard.kde.org/r/684/diff
> 
> 
> Testing
> -------
> 
> I use the BBC weather ion for my weather data, so I tested it for my current location (fine) and a few others from around the world (also fine).  Has been fine for well over a week, becoming increasingly more useful as sunset pushes towards 10pm.
> 
> 
> Thanks,
> 
> Andrew
> 
>