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

[plugin.video.piped] 1.0.0 #4445

Merged
merged 7 commits into from
Feb 11, 2024
Merged

[plugin.video.piped] 1.0.0 #4445

merged 7 commits into from
Feb 11, 2024

Conversation

syhlx
Copy link

@syhlx syhlx commented Jan 6, 2024

Description

New addon which allows users to access any Piped instance, login and manage their playlists and watch history.

More information about Piped:
Piped GitHub Repository
Piped Official Instance

Checklist:

  • My code follows the add-on rules and piracy stance of this project.
  • I have read the CONTRIBUTING document
  • Each add-on submission should be a single commit with using the following style: [plugin.video.foo] v1.0.0

Additional information :

  • Submitting your add-on to this specific branch makes it available to any Kodi version equal or higher than the branch name with the applicable Kodi dependencies limits.
  • add-on development wiki page.
  • Kodi pydocs provide information about the Python API
  • PEP8 codingstyle which is considered best practice but not mandatory.
  • This add-on repository has automated code guideline check which could help you improve your coding. You can find the results of these check at Codacy. You can create your own account as well to continuously monitor your python coding before submitting to repo.
  • Development questions can be asked in the add-on development section on the Kodi forum.
  • If you see no activity on your PR after a week (so at least one weekend has passed) then please go to the #kodi-dev freenode IRC channel to reach out to the team

@basrieter
Copy link
Contributor

Please fix the issues pointed out by the add-on checker. Then I will have a further look.

@syhlx
Copy link
Author

syhlx commented Jan 18, 2024

This should fix all the issues pointed out by the add-on checker

Copy link
Contributor

@basrieter basrieter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you have a look at the code for the services? It does not seem to handle aborts correctly.

httpservice.daemon = True
scheduler.daemon = True
player = Player()
player.isPlaying()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this call? Makes no real sense if you don't use the output?

player = Player()
player.isPlaying()

while not self.abortRequested():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If abortRequested() results in True, the httpservice and scheduler aren't stopped properly and could cause Kodi to wait for a long time.

if self.waitForAbort(5):
while scheduler.is_alive():
scheduler.stop()
xbmc.sleep(200)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If scheduler.stop() does not finish, the script waits for 200ms and then stops it again? In the Scheduler.stop() you call self.join() which should actually block until the thread stopped.

@syhlx
Copy link
Author

syhlx commented Jan 22, 2024

Before I commit anything, does this seem right to you?

class Service(xbmc.Monitor):
  def __init__(self):
    httpservice = Thread(target=HttpService)
    scheduler = Scheduler()
    httpservice.daemon = True
    scheduler.daemon = True
    player = Player()

    while not self.waitForAbort(5):
      try:
        if not httpservice.is_alive(): httpservice.start()
        if not scheduler.is_alive(): scheduler.start()
      except:
        pass

    scheduler.stop()

@basrieter
Copy link
Contributor

Yes that makes more sense. Also make sure the httpservice is stopped in the line under the scheduler.stop()

@syhlx
Copy link
Author

syhlx commented Jan 24, 2024

Done

@syhlx syhlx requested a review from basrieter February 3, 2024 15:57
@basrieter basrieter merged commit b59d044 into xbmc:matrix Feb 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants