GNOME Bugzilla – Bug 708498
better integration of libgweather in glocks
Last modified: 2021-06-01 22:44:30 UTC
Inspired by the warning message: (gnome-clocks:5256): GWeather-WARNING **: Failed to get IWIN forecast data: 1 Cancelled facts: we create each minute in the tick function of each clock item a new gweather info instance: weather_info = new GWeather.Info (location, GWeather.ForecastType.LIST); from info we use is_daytime, get_value_sunrise, get_value_sunset which require a location as input and to update time internally to start a non trivial calculation. the time is updated by the reset function. instead we can create a new info instance by calling object.new in the item constructor. the remaining questions are: - how - how often and - where should we update the info instance with the current time? -how: we can call set_location (location); which overwrites location member and forces the call of internal reset function. or we can ask for a function to update this data? -how often: I would prefer only two times, first by the creation and by the end of the day in the time of the current location. the calculation done to get the sun rise/set time are heavy and should not actually be performed that often. -where: may be each item should add a timeout for that? first step in this direction is done in the wip/cityimages branch: commit c987c9f109baad45c6508ae5f62c6b172c1b6412
I hit another issue: it's probably because in libgweather they seem not to specify initial value for location to NULL, and thus when trying to free it causes SF. https://git.gnome.org/browse/libgweather/tree/libgweather/weather.c#n2170 so something like in C: gweather_info_new(NULL,0); gweather_info_set_location(l); should cause a segmentation fault because set_location will free the previous location instance
Created attachment 255467 [details] [review] patch This is what I had in mind, and seems to work fine in my limited testing
(In reply to comment #2) > Created an attachment (id=255467) [details] [review] > patch > > This is what I had in mind, and seems to work fine in my limited testing Sure, it works, but it only deactivates the forecast service. Should not it be enough to just specify the location parameter?
(In reply to comment #2) > Created an attachment (id=255467) [details] [review] > patch > > This is what I had in mind, and seems to work fine in my limited testing Can you please confirm that this causes a segmentation fault? weather_info = (GWeather.Info) Object.new (typeof (GWeather.Info), location: null, enabled_providers: GWeather.Provider.NONE); weather_info.setlocation (location); and this works again: weather_info = (GWeather.Info) Object.new (typeof (GWeather.Info), location: location, enabled_providers: GWeather.Provider.NONE); weather_info.setlocation (location);
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new enhancement request ticket at https://gitlab.gnome.org/GNOME/gnome-clocks/-/issues/ Thank you for your understanding and your help.