Python refactor #1477
Python refactor #1477
Conversation
Solves problem when installing package directly from git (pipenv install -e )
- Except NULL for cdefs (slightly optimizes performance) - Handle reading NULLs from Options (caused segfault) - Improve handling of parameters for char* options - accepts string-like objects, returns bytes() - Use cython magic for __options - Handle None as NULL in SwapOnVSync (allows wait for sync usage) - Use C++ Canvas object to handle common operations - Use @Property decorator syntax - prevent creation of uninitialized objects from Python code (__new__) - prevent cython classes without corresponding C++ class (raise exception on all paths that can cause it) - Trace displayed FrameCanvas in SwapOnVSync - class passed to SwapOnVSync is returned on second call - Recycle FrameCanvas when python object is freed - FrameCanvas class keeps RGBMatrix alive (RGBMatrix is not deleted before FrameCanvas)
Export Serialize using Python Buffer Protocol Currently only read-only view is supported
``` green = Color(0, 255, 0) rgbmatrix.Fill(*green())
generated using: python3 -m cython -3 --cplus *.pyx
|
is this still needed after @chrisgilldc and @ty-porter 's changes? |
|
This looks great, supporting canvas serialization would be a welcome addition to the Python bindings! This PR might need to be rebased, looks like it is touching the same code as #1817 but I believe it should be compatible. |
| def SetPixel(self, int x, int y, uint8_t red, uint8_t green, uint8_t blue): | ||
| self.__matrix.SetPixel(x, y, red, green, blue) | ||
| cdef cppinc.Canvas* __getCanvas(self) except NULL: | ||
| assert self.__matrix != NULL |
There was a problem hiding this comment.
Generally: Are these assertions required? If so, can we at least add an error message?
|
yeah, this needs a rebase now, but if you're good with it on principle, happy to merge after that (please test the full code after rebase to make sure nothing broke) |
|
@ledvinap and @ty-porter I'm not able to tag or add you to the master bug to keep track of python stuff. Can you add yourselves as assignees to #1749 or just reply to the issue, and then I'll probably be able to add you so you have the option to review and vet new python changes? |
|
Is this PR still being worked on? If not, I'll close it. It's not that I don't want to to see this get in, but we don't want stale/abandoned PRs to stay around forever either. |
|
I'd defer to @ty-porter, I don't think my changes would have intersected here and I don't have Cython expertise. |
4 participants
(For discussion)
Add support for FrameCanvas::Serialize: