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

[pythonscripting][wip] initial release #18229

Open
wants to merge 165 commits into
base: main
Choose a base branch
from

Conversation

HolgerHees
Copy link
Contributor

@HolgerHees HolgerHees commented Feb 4, 2025

Initial release with detailed description in marketplace

https://community.openhab.org/t/python3-scripting/161945

and it has still one major known issues #18054 (comment)

in the final version, before I merge, I will rebase the project to clean the history.

HolgerHees and others added 30 commits December 18, 2024 16:26
Working with "info is null"

Working - occasional "info" is null
Graal engine working - occasional "info" is null
Working with "info is null"

Working - occasional "info" is null
added "py" extension in getExtension

First pass at mirroring jsscripting
Created own GraalPythonScriptEngine based on the GraalJSScriptEngine

Working - with exception of regex language
updated working/lib directories to find modules in lib folder
added python standard libraries

Working version
added "py" extension to getExtension in factory
Added injection of scope hashmap
inserted scope variables from the scopeValues method in the component
… - not needed since one is created in OpenhabGraalPythonScriptEngine

cleanup
Eliminate creating a default engine in GraalPythonScriptEngineFactory…
@HolgerHees
Copy link
Contributor Author

python import wrapper is implemented and python code is moved to https://github.com/HolgerHees/openhab-python . This could be converted to a openhab repo later.

Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Thanks for adding another automation, that's awesome! While you await a review and still are working on this, you could have a look also at compiler and checkstyle warnings. See target/code-analysis/report.html.

I have added a few quick minor comments, but only scratching the surface while peeking.

@HolgerHees
Copy link
Contributor Author

HolgerHees commented Feb 25, 2025

Thanks for adding another automation, that's awesome! While you await a review and still are working on this, you could have a look also at compiler and checkstyle warnings. See target/code-analysis/report.html.

I have added a few quick minor comments, but only scratching the surface while peeking.

thanks for the valuable feedback. I fixed all places related to your commends. Most things are leftovers where I was not sure if it is really needed. Now, I checked these places again and refactored or cleaned it.

Also most of the hints from code analysis is fixed. Except 7 places with the recommendation to set "NonNullByDefault" to classes. Most of theses classes extending other classes which makes it difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants