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

fix: restructure #41

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

fix: restructure #41

wants to merge 14 commits into from

Conversation

piksel
Copy link
Owner

@piksel piksel commented Aug 7, 2022

This PR restructures the app for better confinement, less cross-dependencies and (hopefully) better names.
The main purpose for this is to enable automatic CI builds and releases.

@tyalie
Copy link
Collaborator

tyalie commented Aug 7, 2022

The changes look good so far, even thoo I haven't tested it. Changing the requirements.txt to include pybluez everywhere is definitely more sensible, I didn't knew that it supported Windows and macOS.

But I've one critique thoo: you introduced the pyproject.toml but still carry the setup.py with you. Can't the latter be completely replaced by the former? Is there anything I overlooked? And if were are on it, why don't we use the more modern PyInstaller that works for macOS, Windows and Linux?

EDIT: Works so far

@piksel
Copy link
Owner Author

piksel commented Aug 7, 2022

Well, the only reason for keeping setup.py right now is for py2app and py2exe. Buut, if pyinstaller works sufficiently well, that can be removed as well. It is currently building in the CI with pyinstaller, but I have not verified the artifacts fully on all platforms...

@piksel
Copy link
Owner Author

piksel commented Aug 7, 2022

Current status for the pyinstaller packages:

  • Windows: Is detected as a virus by Defender, but seems to work otherwise
  • macOS: Fails to start, log shows "Unknown error 111"
  • Linux: seems to work fine on LTS Ubuntu

@tyalie
Copy link
Collaborator

tyalie commented Aug 7, 2022

Okay. Hmh. I don't really have a running macOS computer here.

But I think it's fixable, pyinstaller is pretty well known. Because as of right now the setup.py is presumably pretty much broken. It doesn't contain all the packages nor does it link the correct paths.

@tyalie
Copy link
Collaborator

tyalie commented Aug 7, 2022

Bt maybe we should open another issue for this?

@MortenVinding
Copy link

MortenVinding commented Dec 11, 2023

Okay. Hmh. I don't really have a running macOS computer here.

I tried to build on my Macbook Air M2 (Sonoma 14.1.2) but got the following error:

(venv) morten@MacBook-Air pytouch-cube % python3 pytouch3.py
Traceback (most recent call last):
  File "/Users/morten/git/pytouch-cube/pytouch3.py", line 9, in <module>
    from gui import EditorWindow
  File "/Users/morten/git/pytouch-cube/gui/__init__.py", line 1, in <module>
    from .editor_window import EditorWindow
  File "/Users/morten/git/pytouch-cube/gui/editor_window.py", line 11, in <module>

    from labelmaker import USABLE_HEIGHT
  File "/Users/morten/git/pytouch-cube/labelmaker/__init__.py", line 8, in <module>
    from labelmaker.comms import PrinterDevice, SerialPrinterDevice
  File "/Users/morten/git/pytouch-cube/labelmaker/comms.py", line 10, in <module>
    import bluetooth
ModuleNotFoundError: No module named 'bluetooth'

Is there anything I can help with, please let me know

/Morten

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

Successfully merging this pull request may close these issues.

None yet

3 participants