added --activity-class-name and --activity-package parameters #2248
added --activity-class-name and --activity-package parameters #2248AndreMiras merged 1 commit intokivy:developfrom
Conversation
AndreMiras
left a comment
AndreMiras
left a comment
There was a problem hiding this comment.
Thanks for the PR, looking good!
I left some minor suggestion. Also I'm wondering if part of it could be unit tested in tests/test_toolchain.py showing the newly provided arguments get used later on. I haven't looked if it was a painful thing to do.
Note the linter is complaining:
pythonforandroid/recipes/android/src/android/permissions.py:12:1: F401 'android.config.JAVA_NAMESPACE' imported but unused
pythonforandroid/recipes/android/src/android/activity.py:2:1: F401 'android.config.JAVA_NAMESPACE' imported but unused
pythonforandroid/recipes/android/src/android/activity.py:2:1: F401 'android.config.JNI_NAMESPACE' imported but unused
pythonforandroid/recipes/android/src/android/runnable.py:7:1: F401 'android.config.JAVA_NAMESPACE' imported but unused
pythonforandroid/toolchain.py
Outdated
|
|
||
| generic_parser.add_argument( | ||
| '--activity-class-name', '--activity_class_name', | ||
| dest='activity_class_name', default='PythonActivity', |
There was a problem hiding this comment.
I'm wondering if both default couldn't be shared with the one defined in build.py to keep a single source of truth. Maybe using a const somewhere.
Also matter of taste but I would ditch the underscored version of the argument
There was a problem hiding this comment.
i will check about unit tests Andre. i was a bit in the rush when started it.
There was a problem hiding this comment.
Thanks, we have pretty low coverage in both p4a and buildozer, but I'm trying to push for increasing it slowly
There was a problem hiding this comment.
added some basic assertion in the existing unit test to check that Context was populated properly with new value of activity_class_name
| onRequestPermissionsResult in PythonActivity. | ||
| """ | ||
| __javainterfaces__ = ['org.kivy.android.PythonActivity$PermissionsCallback'] | ||
| __javainterfaces__ = [ACTIVITY_NAMESPACE + '/' + ACTIVITY_CLASS_NAME + '$PermissionsCallback'] |
There was a problem hiding this comment.
I guess we got that wrong since the beginning , but it seems that both doted and slashed notation are working then, right?
There was a problem hiding this comment.
yes. exactly... i was in doubt, but checked with py-jnius docs and it says it must be '/'
| 'JNI_NAMESPACE': jni_ns, | ||
| 'ACTIVITY_CLASS_NAME': self.ctx.activity_class_name, | ||
| 'ACTIVITY_PACKAGE': self.ctx.activity_package, | ||
| 'ACTIVITY_NAMESPACE': self.ctx.activity_package.replace('.', '/'), |
There was a problem hiding this comment.
How about introducing a ACTIVITY_FULL_CLASS_NAME value that is already concatenating both activity_class_name and activity_package?
e.g.
{
"ACTIVITY_FULL_CLASS_NAME": f"{self.ctx.activity_package}.{self.ctx.activity_class_name}",
}That way you don't need to carry over the concatenation all over later in the code
There was a problem hiding this comment.
👍 i will check that!
There was a problem hiding this comment.
ok... now i just added one input parameter activity_class_name which must be a full name of the Java class.
vesellov
force-pushed
the
master
branch
3 times, most recently
from
bec6d11 to
ac476cc
Compare
…m PythonActivity java class
AndreMiras
left a comment
AndreMiras
left a comment
There was a problem hiding this comment.
Very nice 👏
Thank you for addressing the comments and thanks for the unit tests 💪
Will merge after the build turns green
|
|
||
| generic_parser.add_argument( | ||
| '--activity-class-name', | ||
| dest='activity_class_name', default='org.kivy.android.PythonActivity', |
There was a problem hiding this comment.
I wish we had a single source of truth shared with https://github.com/kivy/python-for-android/pull/2248/files#diff-c439e980dc587602fda87459815f09feR450 for the org.kivy.android.PythonActivity value, but I can live without
There was a problem hiding this comment.
@AndreMiras you mean that there are two settings now that are storing same value? : android.entrypoint and android.activity_class_name ?
|
CI happy, I've also tried the |
|
👍 |
2 participants
to make possible to use custom PythonActivity java class