> On 2009-11-03 17:31:01, Will Stephenson wrote:
> > You could work out the "Current Conditions" and "Condition Icon" data on 
> > each update using the current system time and the UTC time in the forecast 
> > periods, and set these on the data source.  This would AFAICS get rid of 
> > the 'missing icon' icon and make the applet usable in panels.
> 
> Thilo-Alexander Ginkel wrote:
>     I thought about that before, but found it rather weird to call an 
> interpolated value from the forecast a "current" weather condition. Of 
> course, it would be much more pleasing to the eye to have an icon to display. 
> An alternative would be to fix the weather applet to display no icon instead 
> of a broken one if no data is supplied for the current condition.
> 
> Shawn Starr wrote:
>     I used dptrs because I didn't want to break the internal API, which is 
> public in a way. As more people use the backend changing things get more 
> risky. the dptrs are for the member data vs functions. But, if we don't need 
> the dptrs in the ions, we can get 'em out.
> 
> Aaron Seigo wrote:
>     in classes which are linked to by the ion plugins (e.g. Ion), it makes 
> sense. for the ion plugins themselves it doesn't: those symbols aren't 
> exported or used by anything that needs binary compatibility in their 
> interfaces.
> 
> Will Stephenson wrote:
>     (Umm, threading, folks?)
>     
>     Re Current Conditions - I see your point.  ATM on wetter.com the current 
> weather is 'cloudy' here but both their morning and midday values are 'rainy' 
> so an interpolation would be off.  Is it worth asking wetter.com to expose 
> this too, since they obviously have it on the site.  If other providers do 
> give this data directly, perhaps they will feel it's ok to.
>     
>     However what icon should the applet use when it's in a panel with no 
> Current Conditions data?
> 
> Thilo-Alexander Ginkel wrote:
>     I asked wetter.com for a way to retrieve the current weather conditions a 
> couple of weeks ago and got the reply that these are only available through 
> an API for paying subscribers. Now, one could fall back to screen scraping, 
> but that is no option as it violates their TOS.
>     
>     The panel use-case - admittedly - is kind of tricky. What about an icon 
> that mixes multiple weather conditions (so it is obvious that this isn't the 
> current weather)?

I have removed all the dptrs from the ions in kdebase. I will do that in 
playground, but those other ions need more work elsewhere.


- Shawn


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


On 2009-11-03 19:51:12, Thilo-Alexander Ginkel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2026/
> -----------------------------------------------------------
> 
> (Updated 2009-11-03 19:51:12)
> 
> 
> Review request for kdelibs, Plasma and Shawn Starr.
> 
> 
> Summary
> -------
> 
> This change brings support for wetter.com (a weather forecast web site 
> especially popular in Germany) as a weather data source to KDE.
> The Ion makes use of wetter.com's officially-supported REST API. The API 
> supports forecasts for up to three days, so the Ion does not supply any 
> information about the current weather conditions. The forecast for the 
> current day is split between night and day whereas the following two days are 
> provided as an aggregated value for the whole day, respectively. 
> 
> Any feedback is much appreciated!
> 
> 
> This addresses bug 204219.
>     https://bugs.kde.org/show_bug.cgi?id=204219
> 
> 
> Diffs
> -----
> 
>   /trunk/kdereview/plasma/dataengines/CMakeLists.txt 1043003 
>   /trunk/kdereview/plasma/dataengines/weather/CMakeLists.txt PRE-CREATION 
>   /trunk/kdereview/plasma/dataengines/weather/ions/CMakeLists.txt 
> PRE-CREATION 
>   /trunk/kdereview/plasma/dataengines/weather/ions/ion-wettercom.desktop 
> PRE-CREATION 
>   /trunk/kdereview/plasma/dataengines/weather/ions/ion_wettercom.h 
> PRE-CREATION 
>   /trunk/kdereview/plasma/dataengines/weather/ions/ion_wettercom.cpp 
> PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/2026/diff
> 
> 
> Testing
> -------
> 
> Tested the Ion under a current KDE installation from trunk, fixed all Krazy 
> warnings before the move to kdereview. Profiled it for memory leaks using 
> Valgrind, which did not bring up any leaks in the control of the Ion. Those 
> two still look somewhat suspicious, so I'd like to get your opinion about 
> them:
> 
> 42 bytes in 1 blocks are definitely lost in loss record 372 of 1,156
>    at 0x4C25153: malloc (vg_replace_malloc.c:195)
>    by 0x60EDCB4: QString::QString(int, Qt::Initialization) (qstring.cpp:1027)
>    by 0x61BD5EE: QUtf8::convertToUnicode(char const*, int, 
> QTextCodec::ConverterState*) (qutfcodec.cpp:169)
>    by 0x60EFDFA: QString::fromUtf8(char const*, int) (qstring.cpp:3850)
>    by 0x614C052: fromPercentEncodingMutable(QByteArray*) (qurl.cpp:223)
>    by 0x614EFB3: QUrlPrivate::parse(QUrlPrivate::ParseOptions) const 
> (qurl.cpp:3781)
>    by 0x61534FE: QUrl::isValid() const (qurl.cpp:4124)
>    by 0x5A23B5A: KUrl::_setEncodedUrl(QByteArray const&) (kurl.cpp:1538)
>    by 0x5A23F19: KUrl::KUrl(QString const&) (kurl.cpp:411)
>    by 0x1FAE37D6: WetterComIon::getForecast(QString const&) 
> (ion_wettercom.cpp:504)
>    by 0x1FAE5BAA: WetterComIon::updateIonSource(QString const&) 
> (ion_wettercom.cpp:320)
>    by 0x1F267CBE: IonInterface::sourceRequestEvent(QString const&) 
> (ion.cpp:47)
> 
> 176 (32 direct, 144 indirect) bytes in 1 blocks are definitely lost in loss 
> record 882 of 1,156
>    at 0x4C2596C: operator new(unsigned long) (vg_replace_malloc.c:220)
>    by 0x1F267795: IonInterface::IonInterface(QObject*, QList<QVariant> 
> const&) (ion.cpp:36)
>    by 0x1FAE151D: WetterComIon::WetterComIon(QObject*, QList<QVariant> 
> const&) (ion_wettercom.cpp:68)
>    by 0x1FAEE366: QObject* KPluginFactory::createInstance<WetterComIon, 
> QObject>(QWidget*, QObject*, QList<QVariant> const&) (kpluginfactory.h:461)
>    by 0x5B3367B: KPluginFactory::create(char const*, QWidget*, QObject*, 
> QList<QVariant> const&, QString const&) (kpluginfactory.cpp:191)
>    by 0x5594EB5: Plasma::DataEngineManager::loadEngine(QString const&) 
> (kpluginfactory.h:515)
>    by 0x1F6C813F: WeatherEngine::loadIon(QString const&) 
> (weatherengine.cpp:92)
>    by 0x1F6C8C7C: WeatherEngine::sourceRequestEvent(QString const&) 
> (weatherengine.cpp:226)
>    by 0x55913AC: Plasma::DataEnginePrivate::requestSource(QString const&, 
> bool*) (dataengine.cpp:670)
>    by 0x5591421: Plasma::DataEngine::connectSource(QString const&, QObject*, 
> unsigned int, Plasma::IntervalAlignment) const (dataengine.cpp:89)
>    by 0x1F05608D: WeatherPopupApplet::connectToEngine() 
> (weatherpopupapplet.cpp:234)
>    by 0x1F05815E: WeatherPopupApplet::init() (weatherpopupapplet.cpp:223)
> 
> 
> Thanks,
> 
> Thilo-Alexander
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to