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

[screensaver.turnoff] v0.10.2 #1213

Merged
merged 1 commit into from
Nov 23, 2019

Conversation

dagwieers
Copy link

Description

  • Create separate entrypoint
  • Improve stability
  • Add more unit tests
  • Add sanity tests, unit tests and coverage support
  • Use JSON-RPC for all built-ins
  • Improvements for Python 3
  • Support Odroid-C2 display method
  • Support RPi touchscreen display method
  • Improve mute and unmuting audio using JSON-RPC
  • Fix translations
  • Fix an issue when stopping the screensaver

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: [script.foo.bar] 1.0.0

@TravisBuddy
Copy link

Travis Buddy

Hey Dag Wieers,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

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.

Looks OK to me. @enen92 Were your comments on #1196 included here?


Next to managing your display, it can also manage your device power state, log your profile off or mute audio to avoid sounds through your A/V receiver.
</description>
<license>GNU General Public License, v2+</license>
Copy link
Member

Choose a reason for hiding this comment

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

Please use spdx

Copy link
Author

@dagwieers dagwieers Nov 23, 2019

Choose a reason for hiding this comment

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

I fixed it now.

It doesn't seem to be a requirement though, as it is missing from the docs and just recently other add-ons were merged that did not use SPDX for their license-tag.
xbmc/addon-check#126

Copy link
Member

Choose a reason for hiding this comment

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

I documented on the wiki some months ago: https://kodi.wiki/view/Addon.xml#.3Clicense.3E
It's not mandatory but it is nice to start having some sort of standardization. Might be added to the addon checker if someone steps up.
Thanks for adjusting

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I agree with having SPDX as a standard.

def run():
''' Runs the screensaver '''

# If player has media, avoid running
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is because of audio playing?

Copy link
Author

Choose a reason for hiding this comment

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

Or when testing the screensaver when media is playing.

This was giving me issues, when I discovered this is not supposed to work in Kodi.
(The screensaver test button should be disabled when media is playing IMO)

- Create separate entrypoint
- Improve stability
- Add more unit tests
- Add sanity tests, unit tests and coverage support
- Use JSON-RPC for all built-ins
- Improvements for Python 3
- Support Odroid-C2 display method
- Support RPi touchscreen display method
- Improve mute and unmuting audio using JSON-RPC
- Fix translations
- Fix an issue when stopping the screensaver
@TravisBuddy
Copy link

Travis Buddy

Hey Dag Wieers,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

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.

4 participants