-
Notifications
You must be signed in to change notification settings - Fork 306
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
Update of bleak on android and the android kivy example #1398
Conversation
The current develop branch of p4a uses the setup.py to install the recipe and doesn't seem compliant with PEP517 yet. The temporary fix is to provide a setup.py during the build stage for android. Also, the recipe is now more generic, so it can easily be used in your own projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing things up here.
Part of the reason that maintenance on the android backend has fallen behind is that there is not an easy way to build and test the current development snapshot Since this PR is changing things to only build v0.20.2, then I expect the situation will only continue to get worse. So that feels like a step in the wrong direction. Is there any way we can make it easier to build and test local changes to the bleak source for development?
The commits that I added change the recipe to pull from the github develop branch if no local source path is provided. After looking in the p4a code, I found that a runtime environment can be set to use local sources instead of pulling see p4a (I have tested/validated this). Kind regards, robgar2001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates. I think this looks quite good now. Just one more suggestion (see inline comment)
And it looks like we need to run black to format the code.
test_suite="tests", | ||
include_package_data=True, | ||
license="MIT", | ||
classifiers=[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this file is just the minimum needed to install bleak, it would be nice to leave out all of the non-essential parts, e.g. classifiers, entry_points, test_suite, url, author, description, ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these are the only ones left?
setup(
name=NAME,
version=VERSION,
packages=find_packages(exclude=("tests", "examples", "docs", "BleakUWPBridge")),
)
Not sure about packages, aldough not strictly necessary, it might give problems/be less clean without it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think those are the required ones. the packages can probably be changed to find_packages(include=["bleak*"])
rather than the long list of excludes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will leave it at the excludes list, it's the same as in the setup.py at the time of creation of the bleak recipe. So maybe not bad to leave it that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BleakUWPBridge
was removed years ago, so is no longer applicable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed in the latest commit
The bleak demo app still installs/builds perfectly. Any further suggestions? Kind regards, robgar2001 |
Looks good, just need to resolve the merge conflicts. |
Thank you for reviewing! |
BleakUWP has been removed 😃 |
merged, thanks! |
Update of bleak on android and the android kivy example
When trying to build my app with bleak, the android kivy example seemed out of date.
Commmit: 51b277e :
Commit: 17afc95 :
The recipe was quite project directory structure dependant. This works for the example, but not for your own projects and if docker mounts come into play (as is the case for me). To solve this, the recipe now has a url to the latest release of bleak. So now to use bleak in your own app, just copy the recipe and point to it and add the necessary requirements to buildozer.spec.
The current develop branch of p4a installs recipes based on the setup.py file. See p4a PythonRecipe. Which is not available anymore in the current develop branch of bleak. As a temporary fix, fix_setup.py is copied into the bleak build directory.
Looking forward to a response,
Kind regards,
robgar2001