Bug #128
closedRemove weather from xbmc4xbox and replace with a plugin(addon)
Added by nuka1195 about 14 years ago. Updated over 10 years ago.
0%
Description
I have a plugin that completely eliminates the need for weather inside xbmc4xbox.
i have managed only to stop xbmc4xbox from downloading weather. i'm finding it difficult to completely remove weather.
are you interested? it uses the same source as xbmc, weather.com's api, but i parse all the weather info. so it is stable. it also replaces the need to enter cities in xbmc4xbox. they're handled by the plugins settings.
if interested i will post a diff to xbmc4xbox so you can see what i've done and hopefully you can finish that part of it.
Files
Updated by xbs about 14 years ago
What are the advantages of a weather plugin vs. the current built in weather?
Updated by nuka1195 about 14 years ago
1. easier to maintain
2. more info available to skinners without editing xbmc4xbox
3. smaller xbe, though probably not much
4. easier for other plugins to use the weather info and localizing functions
Updated by nuka1195 about 14 years ago
The diff is incomplete. it works as it's suppose to. (it does not download weather, the plugin does that)
the plugin handles everything including entering towns and localizing.
in the zip:
Weather.com (standard) is the plugin (q:\plugins\weather)
script.module.weather is a module (q:\scripts\.modules)
i did it like that so other scripts can take advantage of the standard weather and localizing.
needs to be done.
1. all unused code needs to be removed. (wasn't sure how to handle that)
2. the infolabels (eg Weather.Location) either need changing or removed as Window(Weather).Property(Location) works and is the same.
3. the skins need updating. maybe cc xbs, the available properties can be printed to the log from the plugin settings.
4. remove weather.rar (the icons are included with the plugin, i also did moon phase icons)
5. double check the language strings, some from xbmc overlap.
maybe more?
you can see that XBMC4XBOX now gets all settings from the plugin, i set it to 5 towns, which if 3 is all the api allows, then that can be changed. i also upped the refresh time to 15 minutes. the plugin caches the data, so it only refreshes every 15 minutes, unless you press refresh, it then forces a refresh. there xml's look to expire every 5 minutes, so i thought 15 was ok.
Updated by nuka1195 about 14 years ago
attachments were too big to upload the weather and moon icons. i'll get the moon ones to you.
the weather icons go in the q:\scripts\weather.com (standard)\resources\media\weather\
there just the ones from weather.rar
Updated by nuka1195 about 14 years ago
oh the reason i include the icons with the plugin is. if someone does a plugin for say accuweather, the codes may not be right anyways.
Updated by nuka1195 about 14 years ago
one more thing. the plugin will work with current XBMC4XBOX as is, if you wanted to test it. it just duplicates the download of weather.
Updated by nuka1195 about 14 years ago
have you decided this is too big? :)
i have an updated plugin, i'll upload it to xbmc-addons repo and post a link if you're still considering this. it will include the moon and weather icons.
Updated by arnova about 14 years ago
No, it's not too big ;-) I just bought a new house and it's taking about all of my spare time :-(. If you like I can hook you up with SVN write access, just mail (or pm) me if you do.
Updated by nuka1195 about 14 years ago
good luck with the new home.
no thank you on adding me, soon i may not be around for a while. but the main problem is i don't know all that needs to be ripped out of xbmc4xbox for this.
i'll upload my updated plugin tonight and post a link.
Updated by nuka1195 about 14 years ago
http://xbmc-addons.googlecode.com/svn/packages/xbmc4xbox-weather.zip
this has the updated script and modules.
Updated by nuka1195 about 14 years ago
I updated the plugin/modules/strings. same link as above.
fixes bugs and adds a feature for weather fanart
Updated by nuka1195 almost 14 years ago
this trac is f'd up. i can't login from one of my computers
http://xbmc-addons.googlecode.com/svn/packages/XBMC4XBOX-Weather.zip
final version with updated files for confluence.
this version is stable, i also added an ip based geo location feature. the first town is set default to that "*". it works good for me, not sure how it works elsewhere, but the user can always input a new location. it has a fallback to NY, NY by default.
i put the icons with the module, not the plugin, thought it made more sense.
Updated by arnova almost 14 years ago
- Status changed from New to Closed
- Resolution set to fixed
Added in r30762. Please verify it's working correctly and/or there isn't anything missing. Thanks a lot, Nuka1195!
Updated by nuka1195 almost 14 years ago
- Status changed from Closed to Feedback
- Resolution deleted (
fixed)
it works, but these changes are needed. (included patch) some are fixes, some are minor changes and some are minor additions.
with the skin changes, weather is no longer fetched until you enter the weather window(bad). what could be done is do not include the new skin changes, then in guiinfomanager make the weather.* infolabels return the proper weather window property.
eg.
Weather.Condition -> Window(Weather).Property(Current.ConditionIcon)
also skins that use fanartcode for weather fanart, can allow the user to point to the fanart in weather settings so it is available throughout skin changes.
xbs can view all the available weather properties and a description, by going into settings and under maintenance is a print weather properties option.
here is a ss of it working and properly finding my location.
now that it works are you going to be ripping out weather?
Updated by nuka1195 almost 14 years ago
also note i removed all those scripts from scripts window and changed the release_ltdg.bat file to include scripts folder otherwise weather doesn't work out of the box.
those scripts could be backedup if you think they are important.
Updated by nuka1195 almost 14 years ago
arnova, i have a way to keep the existing infolabels Weather.Conditions...
give me a few days and i'll get a patch up. with the all changes to confluence needed if any.
Updated by arnova almost 14 years ago
Replying to [comment:18 nuka1195]:
arnova, i have a way to keep the existing infolabels Weather.Conditions...
give me a few days and i'll get a patch up. with the all changes to confluence needed if any.
Excellent. I won't touch it then until you've submitted the changes.
Btw. I don't know much about the internals of Weather inside XBMC also but do you think to rip it out we can simply remove the whole CWeather Class or is there still stuff in there being used (for eg. retrieval)?
Updated by buzz almost 14 years ago
spoke to arnova. Will apply the last patch in a moment. It didn't include the changes to copy the script folder over in the batch files, so have added this myself to all 3 batches. I copies the old script examples to docs/script-examples rather than deleting them, in case they are still of use/and for reference.
Updated by nuka1195 almost 14 years ago
ok, i have it ripped out as much as you can and still have it:
1. download weather only on request.
2. keep the existing weather.* infolabels working.
*note the script did normalize hightemp and lowtemp labels. LowTemp -> LowTemperature, HighTemp -> HighTemperature so skins need to update.
i included some more changes to confluence that fixes weather fanart not showing. fanart for confluence is now handled by the script. so user sets the path in weather settings. this is good if changing skins the path doesn't need to be set for each skin, but skins need to support this.
you may have noticed provider is Team XBMC4XBOX. i figured you would take over. shouldn't be anything needed to maintain.
any feedback on the geo location working outside the US?
Updated by buzz almost 14 years ago
geolocation didnt work here. gave me new york (im in uk). i was going to look at this, but happy for you to also ;-) might be easier for me to debug being outside though. this is all in the python code i assume, so shouldnt be a problem for me. Thanks for the patches!
Updated by nuka1195 almost 14 years ago
new york is the fallback. in search.py under the module the geo url. just post that in a browser and see if the result returns city and ip.
and yes i have no way to debug that. could be my regex doesn't work for your location.
Updated by buzz almost 14 years ago
Just to add, am i right in that this doesnt yet support other skins than confluence ? It will need to work on the other skins we ship with as well as on lower resolutions than 720p.
Updated by nuka1195 almost 14 years ago
here's a patchthat fixes all three skins. pmiii and pm3d do not support fanart, soit was a minor change.
this of course doesn't add all the other weather info available. eg night outlooks
Updated by buzz almost 14 years ago
aah ok. looks like the one before that wasnt yet patched. i see that older skins still get the values so that all works. thanks. Ill apply this shortly.
Updated by buzz almost 14 years ago
xbmc4xbox-weather.2.diff in as r30774. Thanks
Updated by nuka1195 almost 14 years ago
weather.com's terms of use states a 25 minute cache not 20 minutes as i had. so i also changed the weather update to 15 minutes. this allows other api's to update sooner if allowed, but doesn't update three times for every one allowed update for weather.com.
hope that made sense.
exobuzz any luck on the geo location?
Updated by arnova almost 14 years ago
Patch added in r30776. 2 things that came to mind (just now):
1) I'm worried about the added memory consumption since the plugin uses Python (obviously). Doesn't this cause any problems in certain cases (eg. when also doing other memory intensive "stuff" ?). Or does it really only run when I open the Weather window (haven't really had the time to look into it)?
2) I'm probably doing something wrong but for me the plugin keeps failing:
22:20:09 M: 38060032 INFO: -->Python script returned the following error<--
22:20:09 M: 38060032 DEBUG: DB: Registering database table <access>
22:20:09 M: 38060032 ERROR: Error Type: exceptions.IOError
22:20:09 M: 38060032 DEBUG: UM: Loading User Configuration from file <umconfig.txt>
22:20:09 M: 38060032 ERROR: Error Contents: (2, 'No such file or directory', 'Q:\\addon.xml')
22:20:09 M: 38060032 DEBUG: DB: About to read data file <Q:\web\umconfig.txt>
22:20:09 M: 38060032 DEBUG: webs: Listening for HTTP requests at address 192.168.1.64
22:20:09 M: 38047744 NOTICE: Webserver: Started
22:20:09 M: 38047744 DEBUG: UM: Writing User Configuration to file <umconfig.txt>
22:20:09 M: 38047744 ERROR: Traceback (most recent call last):
File "Q:\plugins\weather\Weather.com (standard)\default.py", line 7, in ?
Addon = xbmcaddon.Addon( id="script.weather.standard" )
File "Q:\scripts\.modules\script.module.xbmcaddon\lib\xbmcaddon.py", line 34, in init
self._set_addon_info( cwd, id )
File "Q:\scripts\.modules\script.module.xbmcaddon\lib\xbmcaddon.py", line 57, in _set_addon_info
xml = open( os.path.join( cwd, "addon.xml" ), "r" ).read()
IOError: (2, 'No such file or directory', 'Q:\\addon.xml')
22:20:09 M: 38047744 DEBUG: DB: About to save database to file
22:20:09 M: 38047744 INFO: -->End of Python script error report<--
Updated by nuka1195 almost 14 years ago
1. how it works: the script is only called when weather is requested and no more often than 15 minutes, unless you hit refresh or change cities. so eg you first boot. on home window there is weather. so the script is run, fetches weather, sets the window properties and exits. now if you stay on home window for 15 minutes and no ss kicks in. the script is called again. it checks if the cache has expired. it hasn't so it uses the cached weather (25 minutes). now you stay on home, no ss, 15 minutes later the script is called and this time it fetches new weather. the script only runs when called and usually it takes less than 5 seconds. here on xbox probably 3 seconds. if you're watching a movie, no weather is requested so the script never runs(never called). when the movie ends and you go to a window where weather is shown. the script is called. so it shouldn't have a negative affect. but, what happens if two scripts are running at the same time? see issue 2.
2. i just built using the ltcg build bat and weather ran fine on first boot. pmiii weather strings were missing for descriptions, but the weather showed. switched to confluence and all strings worked fine. i wonder if you have a conflict, eg is the recentlyadded.py script running at boot also? i will test later. it's acting like os.getcwd() is not working right for you. the xbmcaddon module uses that to find addon.xml of weather.com (standard) script to get the required info. make sure all files copied. i had issues with evox ftp failing on the folders that start with a period.
you can remove weather.rar for xbmc/media also.
Updated by nuka1195 almost 14 years ago
i enabled recentlyadded and the weather worked. i got a permission error accessing the cached xml file, but then it loaded the weather fine. not sure what happened.
18:31:54 M: 36462592 NOTICE: -->Python Initialized<-- 18:31:55 M: 26599424 NOTICE: XBFileZilla: Starting... 18:31:55 M: 26558464 NOTICE: ES: Starting event server 18:31:55 M: 25423872 NOTICE: ES: Starting UDP Event server on 0.0.0.0:9777 18:31:55 M: 25690112 NOTICE: UDP: Listening on port 9777 18:31:57 M: 21610496 NOTICE: XBFileZilla: Started 18:31:59 M: 22142976 ERROR: Error Type: exceptions.OSError 18:31:59 M: 22142976 ERROR: Error Contents: (13, 'Permission denied', 'q:\\UserData\\script_data\\Weather.com (standard)\\source\\weather-USMI0564.xml') 18:31:59 M: 22142976 ERROR: Traceback (most recent call last): File "Q:\plugins\weather\Weather.com (standard)\default.py", line 22, in ? weather.Weather( addon=Addon, index=sys.argv[ 1 ], refresh=sys.argv[ 2 ] == "1", localize=localize.Localize() ).fetch_weather() File "Q:\scripts\.modules\script.module.weather\lib\xbmcweather\weather.py", line 103, in fetch_weather os.remove( xbmc.translatePath( self.base_path ) ) OSError: (13, 'Permission denied', 'q:\\UserData\\script_data\\Weather.com (standard)\\source\\weather-USMI0564.xml') 18:34:12 M: 29614080 NOTICE: 200 FTP SITE command called [command=help, args=]
Updated by arnova almost 14 years ago
Replying to [comment:30 nuka1195]:
2. i just built using the ltcg build bat and weather ran fine on first boot. pmiii weather strings were missing for descriptions, but the weather showed. switched to confluence and all strings worked fine. i wonder if you have a conflict, eg is the recentlyadded.py script running at boot also? i will test later. it's acting like os.getcwd() is not working right for you. the xbmcaddon module uses that to find addon.xml of weather.com (standard) script to get the required info. make sure all files copied. i had issues with evox ftp failing on the folders that start with a period.
Well I used a non-ltcg build, but that shouldn't matter I guess. I can't recall seeing a "recentlyadded.py" script running anywhere, I assume that's part of Confluence (don't have the sources/Xbox here right now). I copied over all files using XBMC itself. I tested this on 2 of my Xboxes and both suffer from the same issue. I hope to find some time one of these days to look into it myself....
Updated by nuka1195 almost 14 years ago
here is a patch i've been running on xbmc to see if it handles issues where the original weather.* infolabels can get out of sync with the window properties.
i tested on xbox and it seems to work good.
Updated by arnova almost 14 years ago
Replying to [comment:33 nuka1195]:
here is a patch i've been running on xbmc to see if it handles issues where the original weather.* infolabels can get out of sync with the window properties.
i tested on xbox and it seems to work good.
Thanks! :-) There seems to be a problem at sourceforge which doesn't allow me to download the diff :/
Btw. all of a sudden the plugin issue on my Xbox vanished (and I didn't touch it) which makes me wonder whether there's something not properly initialised(?)
Updated by nuka1195 almost 14 years ago
your log showed a problem with the addon.xml not being found and the path it showed made me think os.getcwd() wasn't working properly.
but who knows, i wasn't sure if i removed weather properly. if it was an unitialised variable, i removed most of them in this patch and directly grab the window properties. so maybe it won't happen again if you use this patch.
Updated by nuka1195 almost 14 years ago
i noticed at boot, the weather plugin is being called twice.
it must be my changes in weather.cpp, since the info isn't fetched by xbmc. the isfetched() which calls getInfo() then causes the message to guiwindowweather.
not sure if that's exactly whats happening, but it should be fixed and i'm not sure how.
about the other issue you were having with os.getcwd() not working right. i had that issue happen once. very odd. mine was looking in q:\skin\addon.xml, like it was trying to run a skin specific script (recentlyadded) at the same time as the weather. if this is a problem that can not be fixed in xbmc4xbox, i may have a fix in the xbmcaddon python module. i'm testing it now, but it happened only once, so not sure if it fixes it or just hasn't happened again.
Updated by arnova almost 14 years ago
Replying to [comment:37 nuka1195]:
i noticed at boot, the weather plugin is being called twice.
it must be my changes in weather.cpp, since the info isn't fetched by xbmc. the isfetched() which calls getInfo() then causes the message to guiwindowweather.
not sure if that's exactly whats happening, but it should be fixed and i'm not sure how.
about the other issue you were having with os.getcwd() not working right. i had that issue happen once. very odd. mine was looking in q:\skin\addon.xml, like it was trying to run a skin specific script (recentlyadded) at the same time as the weather. if this is a problem that can not be fixed in xbmc4xbox, i may have a fix in the xbmcaddon python module. i'm testing it now, but it happened only once, so not sure if it fixes it or just hasn't happened again.
Unfortunately I still didn't have any time to really look into the issue, but it is still there. I do have a suspicion about its cause. I recall we had this issue before when we were still part of xbmc.org. I think it has to do with multiple (successive) scripts running. So it could be either a race-condition, a locking issue or still something uninitialised. In my case several scripts are called from autoexec.py at start. I will test whether disabling those fixes it. A problem debugging this issue is that debugging Python doesn't work well on Xbox :/
Updated by arnova almost 14 years ago
btw. it would already help a lot if we would know which part of the script trigger the issue...
Updated by nuka1195 almost 14 years ago
xbmc does seem to handle multiple scripts better, but it also has issues with os.getcwd()
this is in xbmcaddon.py (script.module.xbmcaddon/lib/
this has my test fix, what i did was change from using os.getcwd() to sys.argv[ 0 ] which is set for each run script. but this may be an issue also since sys.argv is global.
it very well may be a race condition.
def _get_root_dir( self ): print "****************************" print sys.argv print xbmc.translatePath( os.path.dirname( sys.argv[ 0 ] ) ) print "****************************" print os.getcwd() print "****************************" # get current working directory cwd = xbmc.translatePath( os.path.dirname( sys.argv[ 0 ] ) )#os.getcwd() # check if we're at root folder of addon if ( not os.path.isfile( os.path.join( cwd, "addon.xml" ) ) ): # we're not at root, assume resources/lib/ cwd = os.path.dirname( cwd )#os.path.dirname( os.getcwd() ) ) # return result return cwd
Updated by nuka1195 almost 14 years ago
use this http://xbmc-addons.googlecode.com/svn/packages/script.module.zip
it's the new module, you also had to import sys if using above code
Updated by arnova almost 14 years ago
This indeed fixes the issue (for me). And again, I do recall seeing this issue before. I think it's not so much the fact whether it's global but more the fact hat getcwd() is emulated in the dll loader (and sysv.argv[] isn't) so this might as well be a proper fix. But indeed: it also runs twice for me here, so up to the next issue ;-)
Updated by arnova almost 14 years ago
Just did some more testing:
1) Appears that, although the errors are gone, I don't get any weather information. Maybe I need to update my skin, I'll check that now
2) I think the issue of the plugin running twice is caused by the fact that weather is (also) refreshed) as soon as the network is up (from CNetwork in utils/network.cpp). I don't know the best approach but I think the refresh should check whether the data has already expired....?
Updated by arnova almost 14 years ago
Just fixed issue #2 in r30784 (I think). But issue #1 remains. Furthermore I'm wondering: In CWeather shouldn't we perform any "delete settings;" at the end of GetLocation() & GetMaxLocations() or is this done elsewhere?
Updated by nuka1195 almost 14 years ago
yes it only runs once now at boot.
no weather? do you get the source in q:\userdata\script_data\weather.com (standard)\source? or does it fail to fetch the source. if the first it's a skin issue, but the skins in svn work. if it's the later have you tried setting you location instead of the geo location. though it should fallback to ny, ny.
i thought about clearing properties in xbmc4xbox, but you can never know all the properties for enhanced plugins, so i thought it was best just to set the isfetched property and let skins handle the visibility. and a general clear all properties wouldn't be good as there are some you don't want to clear. eg addon.name, addon.logo, location)
two more issues i've found.
1. the weather doesn't refresh if you are in the weather window. a simple way to fix that is to put a Weather.IsFetched in the skin or any of the other Weather.*. i changed them all to the property() i think. the CWeather::IsFetched() calls GetInfo(). or figure out what i removed and put it back.
2. pressing refresh doesn't reset the timer. it should i think. the reason i changed that to call CallPlugin() is to avoid the one second delay. i think a call to ResetTimer() should handle that. i will test it.
Updated by nuka1195 almost 14 years ago
1. fixed
2. fixed
with some more code cleanup.
also fixed the plugin if geo location doesn't work and cleaned it up.
and as a test, i commented out the call to reset() in network.cpp and the weather still fetched. not sure if it's something i changed or if that's there for other reasons, but your fix stopped the double run anyways. that change is not in this patch.
Updated by arnova almost 14 years ago
Few questions:
1) Why did you make it a float in "float timeToCallPlugin = 1000;" ? No point in making it floating point IMO (might as well leave it a DWORD or a int/uint16_t)
2) We also need to modify PM3/PM3HD, right?
3) You probably missed my previous comment but: In CWeather shouldn't we perform any "delete settings;" at the end of GetLocation?() & GetMaxLocations?() or is this done elsewhere else we'll leak....
Updated by arnova almost 14 years ago
Oh and
4) Is this ok: CStdString m_szLocationr10; ??? Since CStdString is auto-sizing I'm whether this(r10) is necessary and even works at all?
Updated by nuka1195 almost 14 years ago
1. i looked at what GetElapsedMilliseconds() returned and thought it would avoid a warning? yeah it could be an unsigned integer, that's what it is in xbmc.
2. they were already correct, so nothing to change.
3.(nope i answered in previous comment) "i thought about clearing properties in xbmc4xbox, but you can never know all the properties for enhanced plugins, so i thought it was best just to set the isfetched property and let skins handle the visibility. and a general clear all properties wouldn't be good as there are some you don't want to clear. eg addon.name, addon.logo, location)"
4. that's an array limited to 10, i switched it to a cstdstring to avoid some code. should be ok
Updated by nuka1195 almost 14 years ago
not sure what happened, but here is a fix for xbmcaddon module syntax error. i screwed up the patch or tortoise did.
i also made timeToCallPlugin an unsigned int to match xbmc
Updated by buzz almost 14 years ago
I'm really thinking it would be much easier for everyone if you could commit directly yourself. I know you said you didnt want access, but it will save the two and froing, which can only be good.
Updated by arnova almost 14 years ago
Patch added in r30788. Note that I've made the float into a #DEFINE since it's a static value anyway.
Updated by nuka1195 almost 14 years ago
@exobuzz, really i just need to get things right the first time :)
a define does make sense.
Updated by arnova almost 14 years ago
Just tested it again, things get better but there are still a few issues:
1) In the weather window the cpu usage is at 85% which is absurd. Probably some looping that is infinately fast in the plugin?
2) It still failed on me without setting the proper city first (the fallback didn't work).
3) The Weather settings window doesn't work properly the first time. I think this is related to problem 2). What I witness in the plugin settings screen (of Weather) that a boolean toggle is shown next to the first city. There are also some extra options in the left vertical menu below "maintenance" which are NOT selectable. After selecting the first city all of this is fixed but obviously for ordinary users this should work out of the box.
Updated by nuka1195 almost 14 years ago
1. that can only be the chaznges in xbmc4xbox infolabels? the pl,ugin exits after the items are set.
2. ok anyway you can visit the geo url and post the source for me to look, unless #3
3. maybe you had an old settings.xml in userdata?
new issue. all weather.* are not working. i will test this and get back to you. odd i thought they did work.
Updated by nuka1195 almost 14 years ago
i'm testing the new mc360 skin and cpu is only 13% in weather.
confluence is 71%, must be all the visible conditions with infolabels.
Updated by arnova almost 14 years ago
I don't know what you mean by an "old settings.xml" in userdata but I don't assume/hope you mean that one has to delete ones settings.xml before you can upgrade to an xbmc4xbox version without the weather plugin to a version with plugin.
But I think something is totally wrong in the weather plugin settings window. It's acting really weird. Just tried to re-select city #1 and that dropped me back to the "previous window". And after trying again the GUI totally screwed up as if the skin got disabled, only the text of the settings was shown and no reference to an eg. AV in the log except for:
19:57:00 M: 25542656 DEBUG: unable to load resources/language/english/strings.xml: Failed to open file at line 0
19:57:00 M: 25542656 INFO: -->Python script returned the following error<--
19:57:00 M: 25542656 ERROR: Error Type: exceptions.IOError
19:57:00 M: 25542656 ERROR: Error Contents: (2, 'No such file or directory', 'addon.xml')
19:57:00 M: 25542656 ERROR: Traceback (most recent call last):
File "Q:\plugins\weather\Weather.com (standard)\default.py", line 7, in ?
Addon = xbmcaddon.Addon( id="script.weather.standard" )
File "Q:\scripts\.modules\script.module.xbmcaddon\lib\xbmcaddon.py", line 35, in init
self._set_addon_info( cwd, id )
File "Q:\scripts\.modules\script.module.xbmcaddon\lib\xbmcaddon.py", line 58, in _set_addon_info
xml = open( os.path.join( cwd, "addon.xml" ), "r" ).read()
IOError: (2, 'No such file or directory', 'addon.xml')
19:57:00 M: 25542656 INFO: -->End of Python script error report<--
Bottom line: there is still some major stability issue :/
Updated by nuka1195 almost 14 years ago
yup, well i thought it was a good idea. it works good for me, so i don't understand. the reason you need to delete settings.xml in userdata/weather.com (standard)/ is because it may have gotten corrupted, since i didn't know if no city was found it wouldn't just error. otherwise no you don't need to delete anything.
i have noticed after setting a city, navigation gets screwed up in settings.
it happens when the script calls openSettings(), before that navigation works fine for me.
i have some issues resolved with the weather.* infolabels. and the error you showed there may be fixed also. runscript() wasn't setting the sys.argv correctly. i also set hightemp and lowtemp now as some skins would need updating.
if you want to:
1. remove weather changes for now, i understand.
or
2. give me temp access and i can just commit these changes as i find them.
Updated by nuka1195 almost 14 years ago
i just tested xbmc with and without my settings changes and navigation didn't screw up in settings after i entered a new city.
i code in xbmc for testing, then i convert to xbox. it's a lot faster, i must have messed something up. i will go thru and try again.
Updated by nuka1195 almost 14 years ago
well i definately found missing code, sorry.
if you or exobuzz want to give me access, i will only commit fixes for settings and weather.
Updated by nuka1195 almost 14 years ago
fixed weather.* infolabels(not sure i like how i did this)
added missing code in guidialogpluginsettings, but the navigation issue continues. it only happens when a script calls opensettings, so for now i commented that out. once you change a city, you will have to enter settings again. (this works fine on xbmc with my patches and i don't see the difference. it must be deeper in the code.)
plugin now sets HighTemp and LowTemp as before so skins don't need updating.
updated confluence skin defaultcontrol for pluginsettings, others were fine.
fixed runscript() builtin not passing the scripts full path in sys.argvr0
again, if geo fails it should fallback, but if you have an old settings.xml you may have to delete it.
Updated by nuka1195 almost 14 years ago
wow, don't use that patch as is.
the way i did the weather.* infolabels introduced a huge mem leak.
maybe you can look at it and see what i was trying to do.
Updated by nuka1195 almost 14 years ago
i think i need to go back to setInfo() and set a member variable for them. or just return the values in guiinfomanager.cpp directly.
i'll play around and check tomorrow for any thoughts you have.
Updated by nuka1195 almost 14 years ago
ok that patch uses setinfo() again and no mem leak.
cpu % is 70% in confluence, that's the skin, the plugin isn't running.
i left a clog in there accidentaly if you would kindly remove it, but i wanted you to look at setInfo() and see the if check.
is that check faster than the one i commented out above it? choose the best one and remove the other.
Updated by arnova almost 14 years ago
Just added your patch in r30789
Some comments:
1) Let's keep the weather plugin for now, else all our work would be in vain. We can always remove it, if we keep having problems. We at least have some general improvements concerning plugins which is also very nice :-)
2) I don't think 0 *p or strcmp(p, "") makes an awful lot of difference and 0 *p doesn't look very nice either (especially since you can easily mistake it for a misspelled NULL == p). So now you know which one I chose ;-)
3) I've granted you SVN access for xbmc4xbox, maybe you still need it.
4) We do need to check the cpu usage thing. Maybe the issue was already there before, not sure.
5) And ofc we need to make sure it's really stable now.
Updated by arnova almost 14 years ago
Oh btw. strlen(p) == 0 is the proper way ofc.
Updated by nuka1195 almost 14 years ago
good to know, thanks.
cpu usage, yes that isn't good. with a skin that uses fewer window(weather).property(*) labels (JZ 720) cpu usage for me is 14% so i think it's those infolabels.
in xbmc with basically the same changes dual core amd. standard confluence cpu=6-14%, fps=60 steady, with confluence from xbmc4xbox 24%, fps=55 fluctuating.
stock xbmc from svn(no patches) basically the same result with stock confluence, with xbmc4xbox confluence cpu=36-39%, fps 57-58. this is with the builtin weather, so all the properties the skin is checking do not exists. maybe that makes a difference?
so using a lot of window.properties() is a cpu hog.
the script sets more, but basically is the same and if skins do not use all of them there seems to be no ill effects.
thanks for the svn access, promise i won't sneak in the disbaled control fade patch :)
Updated by arnova almost 14 years ago
Just tested the last patch but the settings window is still acting weird when eg. selecting city #1: it still jumps back to the previous window, which isn't normal I'm guessing...(?)
I think the cpu usage should really be close to what it was when weather was still builtin. having a cpu usage this high can really cause problem with eg. music playback (or other things i can't think of right now)...
Updated by nuka1195 almost 14 years ago
1. when selecting a city. the settings close and launches search.py to perform a search. this is normal, but if you don't get a keyboard then the script is erroring, what's in the log.
2. cpu usage is the same if you change the skin back to the original myweather.xml. just copy that from xbmc and you'll see. it's not the script. the builtin weather did the same thing set properties. just not as many.
i have some more fixes. those original weather.* labels are a pain to keep synced, but i think i have it.
Updated by nuka1195 almost 14 years ago
about closing the settings window to search for city.
how about we add an option action="" to settings that would runscript() then reload settings instead of closing dialog.
it would require something like WaitOnScriptResult() in PluginDirectory.cpp.
it would be nicer for user as the keyboard would come up immediately, it wouldn't solve the issue about navigation, just avoids it.
Updated by nuka1195 almost 14 years ago
this needs work still.
i was browsing the youtube plugin and weather saved it's source in the plugin_data/youtube.
there are definately issues using sys.argv and os.getcwd when two scripts are running.
the easiest solution is in guiweather.cpp where there is a check to see if the script is already running. to just check if any script is running.
any thoughts?
Updated by arnova almost 14 years ago
I think the fix looks fine. Btw. I also feel like we should always perform a StartZero() when calling the CallPlugin() this should also fix the run-twice issue at start since you really want Weather to sync in case the network comes back up (checking whether it was already recently fetched also makes sense there). Adding a lock/criticalsection to CallPlugin (at the beginning) should probably also make things safer...
Updated by nuka1195 almost 14 years ago
i added the criticalsection lock. i hope that's what you meant.
the startzero only applies if the window is active? it only checks in framemove(), so i didn't change that. i did move a Stop() into callplugin.
i reenable the refresh in network.cpp your right if you boot with no network it only refrehed when i entered the weather window. no dual run at start.
about recently fetched, if i understand correctly that is already handled by the plugin. if you meant something else, i'm not sure how to handle that.
Updated by buzz almost 14 years ago
I got a moment to look at why it doesn't pick up my location with "*"
The geo regular expression returns
"LONDON, ENGLAND (UK)" as the location which doesn't work against the weatherchannel search.
removing the (UK) fixes it. so you could either do that after, or include it in the original regular expression
Your IP Address: <strong>(.+?)</strong><br />\s.+?Located near: <strong>([^\(<]+)
(stop at first < or ()
I do think however, that for a "core" feature like this, scraping a site that may change without notice is prone to failure, and it would be far more reliable to use a webservice for this such as "http://ipinfodb.com/ip_location_api.php" which looks to be free once you get your api key. it can return json or xml, either of which should be easy to handle.
Cheers
Updated by arnova almost 14 years ago
Replying to [comment:74 nuka1195]:
i added the criticalsection lock. i hope that's what you meant.
the startzero only applies if the window is active? it only checks in framemove(), so i didn't change that. i did move a Stop() into callplugin.
i reenable the refresh in network.cpp your right if you boot with no network it only refrehed when i entered the weather window. no dual run at start.
about recently fetched, if i understand correctly that is already handled by the plugin. if you meant something else, i'm not sure how to handle that.
Looks good. What I mean is that when eg. fetching the weather when the network comes back up, it only makes sense to do that if it wasn't already done a few minutes ago. This is also what causes the double runs at start, I'm guessing...
Updated by nuka1195 almost 14 years ago
exobuzz, i have a key for that website, it just returns the wrong city for me. if everyone thinks it's better for all, then yes it can be change.
arnova, the dual run at start seems to have stopped now.
Updated by buzz almost 14 years ago
not great though if it's not accurate. we can stick with the scraper for now i guess, until something better comes along then.
Updated by arnova almost 14 years ago
There's still a problem when other scripts are (already) running and that's that Weather will then only be refreshed on the next timeout, right? I think it makes to lower the timeout in case this happens. In my setup it means that I don't get any weather info at startup because I have some stuff running from autoexec.py + my netwerk is set to use DHCP (which causes it to come up a bit later).
Updated by arnova almost 14 years ago
Btw. I think it makes sense to try to do CallPlugin() let's say every minute until the plugin is actually executed (in other words until we no longer hit any of the returns at the beginning of CallPlugin()).
Updated by arnova almost 14 years ago
Btw2. This way we also no longer need to call the Refresh() from CNetwork....
Updated by nuka1195 almost 14 years ago
you can confirm this is how it's working for you? i tested this and this is what happens for me.
i boot, weather is fetched. i go start the lyrics script, let it run for a half an hour. as soon as i exit and go back to a page where weather is requested. the weather is fetched imeediately.
maybe i'm missing what your issue is.
ot: i now have problems downloading attachements on trac. did you resolve your issue or did it fix itself?
Updated by arnova almost 14 years ago
Well that's the weird thing for me: this doesn't happen in the home window. I first have to go to the Weather window where it keeps saying "Accessing weather service". But a bit later it starts working all of a sudden...
ps. The attachement issue was something caused by SF. It resolved itself afaik...
Updated by nuka1195 almost 14 years ago
ok with an autoexec.py, i can confirm your issue.
Updated by buzz over 13 years ago
when i go into the settings for example on the iplayer plugin, despite known issues navigating and some other crashes / glitches on the current plugin screens (which I will look into), when exiting I get an error thrown by the weather plugin.
02:29:23 M: 35385344 INFO: initializing python engine.
02:29:23 M: 35385344 DEBUG: new python thread created. id=8
02:29:23 M: 34861056 DEBUG: CGUIWindowWeather::CallPlugin - Weather plugin called: special://home/plugins/weather/Weather.com (standard)/default.py (1,0)
02:29:23 M: 34861056 DEBUG: Python thread: start processing
02:29:23 M: 34861056 DEBUG: python thread 7 destructed
02:29:23 M: 34861056 DEBUG: XBPyThread::Process - The source file to load is special://home/plugins/weather/Weather.com (standard)/default.py
02:29:23 M: 34861056 DEBUG: XBPyThread::Process - Setting the Python path to Q:\plugins\weather\Weather.com (standard);Q:\scripts\.modules\script.module.weather\lib;Q:\scripts\.modules\script.module.xbmcaddon\lib;Q:\system\python\python24.zlib;Q:\system\python\DLLs;Q:\system\python\Lib;Q:\system\python\spyce
02:29:23 M: 34861056 DEBUG: XBPyThread::Process - Entering source directory Q:\plugins\weather\Weather.com (standard)
02:29:23 M: 34799616 DEBUG: LoadLibraryA('ws2_32')
02:29:23 M: 34779136 DEBUG: LoadLibrary('ws2_32.dll') returning: 007CDA60
02:29:23 M: 34779136 DEBUG: dllGetProcAddress(007CDA20(ws2_32.dll), 'getaddrinfo') => 002541F0
Previous line repeats 1 times.
02:29:23 M: 34779136 DEBUG: dllGetProcAddress(007CDA20(ws2_32.dll), 'getnameinfo') => 0025420B
02:29:23 M: 34779136 DEBUG: dllGetProcAddress(007CDA20(ws2_32.dll), 'freeaddrinfo') => 0025422F
02:29:28 M: 34779136 INFO: -->Python script returned the following error<--
02:29:28 M: 34779136 ERROR: Error Type: exceptions.ValueError
02:29:28 M: 34779136 ERROR: Error Contents: invalid literal for int():
02:29:28 M: 34779136 ERROR: Traceback (most recent call last):
File "Q:\plugins\weather\Weather.com (standard)\default.py", line 22, in ?
weather.Weather( addon=Addon, index=sys.argv[ 1 ], refresh=sys.argv[ 2 ] == "1", localize=localize.Localize() ).fetch_weather()
File "Q:\scripts\.modules\script.module.weather\lib\xbmcweather\weather.py", line 108, in fetch_weather
self._set_properties( info )
File "Q:\scripts\.modules\script.module.weather\lib\xbmcweather\weather.py", line 161, in _set_properties
self.WINDOW.setProperty( "Current.FanartPath", [ "", os.path.join( self.Addon.getSetting( "fanart_path" ), [ info[ "cc" ][ 5 ], self.location_id ][ int( self.Addon.getSetting( "fanart_type" ) ) ] ) ][ self.Addon.getSetting( "fanart_path" ) != "" ] )# Current condition or location fanart path (*see settings)
ValueError: invalid literal for int():
02:29:28 M: 34779136 INFO: -->End of Python script error report<--
02:29:28 M: 34779136 INFO: Python script stopped
02:29:28 M: 35303424 DEBUG: python thread 8 destructed
i guess the new code is doing something that breaks the settings ? - why should the weather plugin even be called after i exit settings for another plugin ? i love the new plugin, but unless we can get this stuff resolved, im inclined to think we should temporarily remove it at least for the stable release and sort it for the next one ?
Updated by buzz over 13 years ago
it might also be worth setting up a branch for this, and merging later ? hoping it can be fixed but if not, we now have to cherry pick a bunch of commits to revert, which would be simpler if on it's own branch + nuka can commit more test stuff for us then too on a branch.
cheers all. input/feedback is always welcome and please dont look on this as a criticism - just want it to work :)
Updated by nuka1195 over 13 years ago
is the problem you entering settings while the weather plugin is running? i have no issues entering any settings.
i won't be around for a while, sorry this caused so many issues.
Updated by buzz over 13 years ago
perhaps it was updating, but in that case we need to suspend the weather and reset the paths in this case ? however it was when i exited i had the error, and it happpened more than once to strange. ill dig more.
Updated by arnova over 13 years ago
I also still witnessed problems concerning the plugin settings. The first the plugin runs without touching the settings, it never works. When navigating to the plugin settings you can see the setting for the first city is screwed up, like I mentioned before.
I must agree with Exobuzz that I think it's better for now to branch what we have now and revert back to the old weather implementation until we've sorted all the issues. I hate to see our stable release been overshadowed by weather plugin problems.
Updated by arnova over 13 years ago
Before I forget: it would be nice though if we would keep the addon support in trunk of course...
Updated by nuka1195 over 13 years ago
reverting does not solve the inherit problem, which is two plugins running at the same time causes issues. it only became apparent because it normally wouldn't have happened before moving weather to python.
i can not work on this, but if you're interested. if i remember correctly. when a plugin is called it's settings and strings are loaded in a global variable. maybe this could be changed to a map, with an id of the plugin as the key.
that would not solve the sys.argv issue, but it would help. which i think xbmc suffers from also, it is global.
in xbmc for windows, the weather works fine even while running another plugin at the same time. the addon system, keeps settings and strings seperate, so maybe a change similar to the above mentioned would be enough.
about your other issue with settings, i have no idea what that is. i don't see how the weather changes would cause different control types to be used. too bad you didn't get a screenshot.
Updated by buzz over 12 years ago
- Target version changed from 3.0 to Future / Pending
Updated by buzz over 10 years ago
- Resolution set to wontfix
- Affected Version set to 3.0