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

Changes to settings do not take effect immediately #560

Closed
dagwieers opened this issue Oct 30, 2019 · 6 comments · Fixed by #563, #575 or #608
Closed

Changes to settings do not take effect immediately #560

dagwieers opened this issue Oct 30, 2019 · 6 comments · Fixed by #563, #575 or #608
Labels
bug Something isn't working help wanted Extra attention is needed
Milestone

Comments

@dagwieers
Copy link
Collaborator

Describe the bug

Most of the changes to the settings require you to restart the add-on (which means first starting another add-on, then return to VRT NU). This is not very user-friendly (i.e. most people would think restarting the add-on would do the trick).

So it's safe to say that people would consider this a bug (changing settings doesn't have any effect). And especially for logging this is problematic, if you enable debug-logging in Kodi, our add-on wouldn't act on it.

We need to address this before releasing v2.3.0.

To Reproduce

Steps to reproduce the behavior:

  1. Change logging settings
  2. Continue using the add-on
  3. Or restart the add-on
  4. Logging changes do not have any effect

Expected behavior

We need to rethink this. Either reevaluate config-options on use (slower) or reevaluate settings when changes are detected.

Additional context

  • Operating system: LibreELEC 9.2 Beta2 (9.1.502)
  • Kodi version: 18.4
  • Addon version: 2.3.0 (devel branch)
  • Using a VPN: no
  • Country you are using the addon from: BE
@dagwieers dagwieers added the bug Something isn't working label Oct 30, 2019
@dagwieers dagwieers added this to the v2.3.0 milestone Oct 30, 2019
@mediaminister mediaminister reopened this Nov 13, 2019
@mediaminister
Copy link
Collaborator

mediaminister commented Nov 13, 2019

This is not fixed yet. When enabling and disabling useresumepoints setting I get wrong values which causes problems with menu refreshing and the Kodi watched status.

I think the pluginsource entrypoint should be restructured according to Kodi's recommendations:

The <reuselanguageinvoker> element is a feature introduced with Kodi 18.0 that changes the way the python invoker works in Kodi - trying to reuse the invoker instances as much as possible. As a result, the addon performance is greatly improved. However, note that for the element to work some changes may be required in your addon. Namely, since the invoker is reused, make sure sys.argv is always passed to your entrypoint and propagated throughout your codebase. Do not store it as a class variable.

I suspect declaring KodiWrapper as a class variable causes problems.

@mediaminister mediaminister added the help wanted Extra attention is needed label Nov 13, 2019
@mediaminister
Copy link
Collaborator

mediaminister commented Nov 13, 2019

@dagwieers I found out that removing the global kodi variable in addon.py fixes this problem.

@dagwieers
Copy link
Collaborator Author

So one solution might be that:

  • get_setting() reads from the Window property cache first (and uses ADDON second)
    • (and also updates the Window property cache if it access ADDON settings)
  • service.py populates the Window property cache when onSettingsChanged
    • iterating over all known settings

The drawback here is that we add quite some complexity for something that really ought to be something simple.

@dagwieers
Copy link
Collaborator Author

dagwieers commented Nov 30, 2019

Another more simple solution is to instantiate Addon from addon_entry.py so that it is at least reread once every invocation.

Update: In fact, the routing module also instantiates Addon twice, so this could in fact be the best solution for all Kodi add-ons. We instantiate Addon from addon_entry, pass it to addon.run() which would pass it to Plugin(), and then we could call plugin.addon to access it.

@dagwieers
Copy link
Collaborator Author

Another more simple solution is to instantiate Addon from addon_entry.py so that it is at least reread once every invocation.

I tried this, declare ADDON = Addon() in addon_entry.py, then import it in kodiutils.py. You would expect this to ensure ADDON is always fresh for every invocation. But it isn't for some strange reason.

@dagwieers
Copy link
Collaborator Author

The irony is that the easiest solution is to disable reuselanguageinvoker. I don't see any noticeable performance degradation because we designed the add-on in such a way that it is pretty lean already. Only import the stuff we really need.

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