Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report xbmc.abortRequested deprecation #70

Open
razzeee opened this issue Apr 16, 2018 · 11 comments
Open

Report xbmc.abortRequested deprecation #70

razzeee opened this issue Apr 16, 2018 · 11 comments

Comments

@razzeee
Copy link
Member

razzeee commented Apr 16, 2018

The old format should only be used in gotham or earlier. Instead we should encourage using the new one. See our docs here:

https://kodi.wiki/view/Service_add-ons

@mzfr
Copy link
Contributor

mzfr commented Apr 16, 2018

@razzeee Can you please provide an example for any addon using old format and the one using new format.

@razzeee
Copy link
Member Author

razzeee commented Apr 16, 2018

Old one would be script.trakt

@razzeee
Copy link
Member Author

razzeee commented Apr 16, 2018

This branch should be using only the new one https://github.com/Razzeee/script.trakt/tree/code-cleanup

@enen92
Copy link
Member

enen92 commented Apr 16, 2018

Any ideas on how to implement this properly? Can this be used with kodistubs somehow by setting @deprecated decorators on methods? I assume pylint for instance has built-in or standard ways of checking for deprecations on modules.
Other option might be to do it statically using the ast python module:
https://stackoverflow.com/a/27336824

Either way, there are a few more deprecations that can benefit of a common design approach

@mzfr
Copy link
Contributor

mzfr commented May 24, 2018

@razzeee @enen92 I was looking into this and I think we can do it in simple way.
So the docs says that waitforabort() is new from helix. Maybe we can just look for the keyword waitforabort(). I am assuming that this will have to be added wtihin the entrypoint files and we can get those from addon.xml

@razzeee
Copy link
Member Author

razzeee commented May 24, 2018

But only service addons need to have this. Not sure if others could theoretically also use this, might make sense and be totally valid for other addons.

@enen92
Copy link
Member

enen92 commented May 24, 2018

Not only service addons use such blocking calls, I've seen a few scripts that also do the same. In fact it is a common practice to suggest the inclusion of abortRequested() in all while statements that can run for some time so that Kodi can quit gracefully when requested to exit. IIRC Kodi will wait for 5 seconds for a Kodi addon to exit before killing the script

@mzfr
Copy link
Contributor

mzfr commented May 25, 2018

@razzeee @enen92 ok if it is suggested that all add-ons must use this then there will not be any problem in doing it the way I suggested.

And even if we want them for particular type like service add-on then also we can do this method, the only thing we'll have to add is a check for the name of the addon which we can easily get from addon.xml

so I'll start work on this if you don't have any other problem with this method.

@razzeee
Copy link
Member Author

razzeee commented May 25, 2018

Well but it doesn't mean that each addon has to have waitforabort(). It's not necessary. That's how I understood your idea and that won't work in my opinion.

@mzfr
Copy link
Contributor

mzfr commented May 25, 2018

But doc says:

abortRequested() and waitForAbort() are new in Helix. 
In Gotham and earlier, use xbmc.sleep and check the 
xbmc.abortRequested attribute periodically. 

So doesn't that means every addon above gotham will have waitForAbort() ?

@razzeee
Copy link
Member Author

razzeee commented May 25, 2018

If you need to wait for stuff yes. But addons like context addons or plugins don't need to do that usually. There might be exceptions but it's not every addon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants