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

Provide OS and system properties to python addons #17265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peak3d
Copy link
Contributor

@peak3d peak3d commented Jan 25, 2020

Description

using the xbmc module 2 new functions are available:

  • getOsName() -> returns the name of the os kodi is running
  • getSystemPropertyString -> returns device dependent system properties

Motivation and Context

I'm working on an addon which requires some system properties to build a token.

How Has This Been Tested?

Sample script which includes print(xbmc.getOsName())

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

@peak3d peak3d added this to the Matrix 19.0-alpha 1 milestone Jan 25, 2020
@phunkyfish phunkyfish added the PR Cleanup: Needs Review This PR is good and ready for a review. Someone please review. label Mar 10, 2020
@phunkyfish phunkyfish requested review from ronie and enen92 April 19, 2020 11:15
/// @brief Returns an Os dependend system property
///
/// @return value assigned to propertyName
///
Copy link
Member

Choose a reason for hiding this comment

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

could you add a few examples here?
it's not clear to me what 'system properties' are...

also, is this android only?

@DaveTBlake
Copy link
Member

All OS? Also if addons/Python interface gets this then so should JSON API :)

@enen92
Copy link
Member

enen92 commented Apr 19, 2020

I am not sure if I'm able to review this.
Which system properties are we talking about? Do you mean those properties that are injected from the JVM and get available in the app context only in android?

For the os name method, I'm wondering if it really belongs in our python API. Can't the same be achieved from the python bultin platform module? (e.g. platform.system()?)

For the property method I agree with ronie. It needs better documentation to explain which properties we're trying to obtain and in which platforms the method returns usable data. I'm also not sure if ifdefing there is the best approach. Sure the implications are small but if every method did the same we end up with some sort of spaguetti. Maybe a Platform() class in xbmc module which is then inserted from the build system would be a better alternative? Or is that too much work?

@CastagnaIT
Copy link
Collaborator

CastagnaIT commented Apr 24, 2020

i can try to explain something

method getSystemPropertyString:
is platform dependent, in this PR is implemented only for android,
see if def: https://github.com/xbmc/xbmc/pull/17265/files#diff-2f806c83b24e5dd8d8a26076af9d511eR779

This method (on android) is a sort of wrapper to query android build prop info, can be used as:
my_value = getSystemPropertyString("ro.product.model")
then my_value contains the product model value example "Nexus 6"

If used on other type of OS, should return null value
(Of course, it can also be implemented later to obtain information from other operating systems)

method getOsName:
returns the name of the operating system used

@DaveTBlake DaveTBlake added v20 Nexus and removed PR Cleanup: Needs Review This PR is good and ready for a review. Someone please review. v19 Matrix labels Oct 30, 2020
@DaveTBlake DaveTBlake removed this from the Matrix 19.0-alpha 3 milestone Oct 30, 2020
@jenkins4kodi jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Apr 29, 2021
@jenkins4kodi
Copy link
Contributor

@peak3d this needs a rebase

@fuzzard fuzzard removed the v20 Nexus label Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change: Python Component: Python Rebase needed PR that does not apply/merge cleanly to current base branch Type: Feature non-breaking change which adds functionality Wiki: Needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants