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

Code structure #94

Open
emilsvennesson opened this issue Aug 24, 2019 · 6 comments
Open

Code structure #94

emilsvennesson opened this issue Aug 24, 2019 · 6 comments
Labels
Milestone

Comments

@emilsvennesson
Copy link
Owner

emilsvennesson commented Aug 24, 2019

The code in inputstreamhelper.py is starting to become rather complex and it's time we start splitting parts into different files. The new API @dagwieers implemented definietely belongs in its own file, and there are probably other parts that we could split up as well to make everything look a bit cleaner. I have something like this in mind:

  • One file (api.py) for the API stuff.
  • One file (common.py/utils.py) that includes all/most of the class methods and other code we can use across all files (JSON RPC requests, HTTP get functions etc).
  • A folder (widevine) with three files: arm.py, x86.py and widevine.py. widevine.py would obviously include code that works on both platforms, and we could then import arm.py/x86.py depending on which platform the user is on. That way, we'll be able to simplify things and restructure the ugly install_widevine() wrapper method.
  • inputstreamhelper.py includes code related to inputstream.adaptive such as enabling/disabling the add-on, HLS checks etc.

In the future, we could also split up the logging code and make use of the Python logging module. Another issue should be opened to discuss this though! :-)

-- Thoughts?

@emilsvennesson emilsvennesson added this to the 0.5.0 milestone Aug 24, 2019
@mediaminister
Copy link
Collaborator

I think we first need to increase the quality of the code in the current single file.
The code is too complex, using too much global variables, duplicating functionality over different functions, using a lot of external libraries while Kodi has a built-in Python API: http://romanvm.github.io/Kodistubs/, etc...

@dagwieers
Copy link
Collaborator

And we need more/better tests as well. We don't have any unit tests at the moment.

@dagwieers
Copy link
Collaborator

dagwieers commented Aug 24, 2019

We cannot just move it to api.py. The problem is that currently everything is in inputstreamhelper.py, so we cannot change that interface. If we wanted an api.py it should go into inputstreamhelper/api.py but that means that Helper() will have to live in inputstreamhelper/__init__py.

So for the time being (in #98) I move the API to inpustreamhelper_api.py. In itself it doesn't matter where it lives, since nobody should be calling this directly (in contrast to inpustreamhelper.py).

We could alsoI opted to keep the API stuff in addon.py and don't bother with the recommendation to keep it under 15 lines. It doesn't matter to us anyway. I prefer this over inputstreamhelper_api.py as it won't be exposed to developers. (So I will revert it back ;-))

@dagwieers
Copy link
Collaborator

Currently we have a function to get us specific paths, and that function als creates the directory if it does not exist. We should store those paths in a variable, and only create the directory when it is needed where it is needed. That will be more efficient, and the code will be easier to understand.

@dagwieers
Copy link
Collaborator

Half of the restructuring has been implemented (out of necessity).
Now we need to plan the second half...

@dagwieers
Copy link
Collaborator

dagwieers commented Sep 6, 2019

Replacing kodi-six is part of this IMO #102

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

No branches or pull requests

3 participants