> 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