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

feat(including bugs): minor changes that increases functionality #1

Merged
merged 16 commits into from
Nov 17, 2021
Merged

feat(including bugs): minor changes that increases functionality #1

merged 16 commits into from
Nov 17, 2021

Conversation

im-coder-lg
Copy link
Member

Hi @sumeshir26,

This is a really great project! Although it has a few glitches(listed below), it's a chance for me to learn some new things too. So, after long research(and switching between computers), I have made a series of minor changes in the source file(main.py) and removed the spec listing from the .gitignore(later on that). I have also generated a newer executable with the TimerX icon you provided in the assets directory. And, I think you don't need the setup.py file anymore since PyInstaller can build everything for you(more on that later).

Now, the glitches are as follows:

  1. You need to click the Play button twice to start the timer.
  2. The timer takes one second to start and the rest 5 seconds as countdown, so reducing the extra second seems like a good idea.
  3. This has been happening for me, but whenever the timer ends, I get a driver error(it doesn't play the audio but my driver's all cool, I can see YT videos, and hear audio content). Does the same happen for you too?

Next, why did I remove the spec file from the .gitignore?

I think you used Pyinstaller the first time you built it. And there is an additional tool with PyInstaller called pyi-makespec, which generates SPEC files for making building the app easier. Basically, clone the repo, get PyInstaller, and run this: pyinstaller main.spec in the root. You get an absolutely smashing executable with the TimerX logo(put it in the root dir to run that, so that it will detect the assets), and it has no consoles open(although, a console might open during errors like running the app from the dist dir due to assets).

So, I'm done here. I need your opinions on this, and also about the driver error I mentioned above.

im-coder-lg and others added 6 commits November 11, 2021 15:53
Need the VNC image for the GUI app
On Linux, if you enter `ls -a`, you get a directory known as `.`, which is the root dir. Works the same on macOS and Windows, so by adding that, it works universally.
@sumeshir26
Copy link
Member

@im-coder-lg no, I do not get any driver errors🤔🤔

@sumeshir26
Copy link
Member

❤️Awsome work! Do you want me to merge?

@im-coder-lg
Copy link
Member Author

im-coder-lg commented Nov 11, 2021

❤️Awesome work! Do you want me to merge?

Well not yet because I am still analyzing the code... The icon seems zoomed in when it's a plain exe... Need to check that, and can you try sharing the original TimerX icon file? Maybe I can try making it into an .ico file and test with that...

@im-coder-lg no, I do not get any driver errors🤔🤔

As expected because you run Windows 11 which has advanced drivers, so I think this could be the time to upgrade to Linux soon... But other than that, great! I just need to find how to make macOS builds and add that here so that Mac users don't need to feel left alone or also they won't need to install Wine(compatibility layer) to run TimerX... Will see that tomorrow.

@im-coder-lg
Copy link
Member Author

Do you know anyone who has a Mac @sumeshir26? Maybe you can test TimerX's Mac build on that. It seems that you can use py2exe as well as PyInstaller, so if we could get a hand at macOS, we could try building TimerX from the terminal. If it succeeds, we are good on a Mac app file.

We need builds for each OS since TimerX could be cross-platform. Rather than starting a file and having Python installed, you can get an executable file for your OS.
@im-coder-lg
Copy link
Member Author

The Linux build is failing. I have a Raspberry Pi running Raspbian Buster(Debian 10 for Raspberry Pi) and tried building it. The build started well, but the file doesn't execute. I mean, in Geany, if you execute main.py, it'll throw up an error saying that assets/logo.ico isn't found. Linux doesn't use .ico files, so we need to find a proper icon file for Linux. Think macOS has this same error?

@im-coder-lg
Copy link
Member Author

asciicast

The video is called an Asciicast, used for showing console logs as videos. On the Pi, I have configured it to show a Neofetch result at the beginning to show the OS. That's proof it's Raspbian. After that, I changed the directory to TimerX's dir, showed my branch(fork not the base TimerX), and entered python3 main.py and I ended up with the error. Can you try sharing the original logo image? Maybe I can make macOS and Linux icons from that.

Change py2exe to pyinstaller
@sumeshir26
Copy link
Member

@im-coder-lg the original logo is in the assets/images dir

@im-coder-lg
Copy link
Member Author

im-coder-lg commented Nov 12, 2021

We got some issues.

  1. The .ico code works only on Windows, which means we need different files+icons for each OS.
  2. The logo doesn't work as a JPEG or as a JPG, it works as a PNG, and the PNG I generated was low-quality. Making an icon is the best idea.
  3. Your about window's icon doesn't work due to the JPEG error, so we need different icons and different files for each OS, which makes 3 main.py files, 3 configurator.py files and 3 main.spec files, which squares the file load.

We need a solution to this soon. I could change the About page's icon to a bitmap icon(.ico) but Macs use .icns and Linux uses .icon(I guess). How can we fix this?

@im-coder-lg
Copy link
Member Author

I just hopped onto the GAUDC project(that gave me a lot of ideas to implement here) and saw the app/res/icons folder and I saw a horde of icons! And as expected, there was an ICNS. So, Linux seems to have PNGs or SVG images, but you need an ICNS for Mac. So, we hit something here, and maybe I could get a fresh ICNS for Mac. @sumeshir26 does Python recognize OSes? Like, if I run TimerX from source on Linux, will the file know that it runs on Linux? If it does, we could use if and elif commands to manage the icon issue. And also, if the system is neither Windows nor MacOS or Linux, it will resort to the PNG(for now). Is this possible?

@im-coder-lg
Copy link
Member Author

We could use this solution I found in Stack Overflow:

from sys import platform
if platform == "linux" or platform == "linux2":
    # linux
elif platform == "darwin":
    # OS X
elif platform == "win32":
    # Windows...

We also need a requirements.txt so that users get the deps like so: pip install -r rquirements.txt

@sumeshir26
Copy link
Member

@im-coder-lg Let me add it real quick..

@im-coder-lg
Copy link
Member Author

Wait what? You are adding the code? We need the icon files! An ICNS, a Linux icon file and an improved PNG(no more JPEG, it throws up an error)

@im-coder-lg
Copy link
Member Author

It is hard to make an ICNS file. I have used online converters and always fail under that.

@sumeshir26
Copy link
Member

@im-coder-lg Let me try to make a icns

@im-coder-lg
Copy link
Member Author

Does py2exe support SPEC files? If they do, we could use py2xe for the generation of files and pyinstaller for modifying the SPEC file. Could that be possible?

@im-coder-lg
Copy link
Member Author

Mac failed. Maybe we need to use a different build platform for Macs. What about PyInstaller?

@im-coder-lg
Copy link
Member Author

@sumeshir26 jackpot. I made the ICNS using an online tool, it's on my device, uploading here now.

@im-coder-lg
Copy link
Member Author

The thing is since it was made from the PNG, how'd the quality be?

@sumeshir26
Copy link
Member

@im-coder-lg The build fails on line 63 of setup.py

@sumeshir26
Copy link
Member

Anyway, i got the icns

@sumeshir26
Copy link
Member

Uploding in the assets directory on your fork...

@sumeshir26
Copy link
Member

@im-coder-lg I'll try after I get the dark mode working

@im-coder-lg
Copy link
Member Author

The icon can be used from the Material Icons library(open-source here on GitHub).

@sumeshir26
Copy link
Member

@im-coder-lg What icon?

@sumeshir26
Copy link
Member

I think I will just use darkdetct to detect the system theme

@im-coder-lg
Copy link
Member Author

@im-coder-lg What icon?

Dark mode toggle. If you use an icon instead of text, it'd just blend into the layout(not meaning "hide", it'd be like it belongs there). Make sure to use a lighter shade of black so that users can see things properly.

@sumeshir26 sumeshir26 marked this pull request as ready for review November 15, 2021 06:10
@sumeshir26
Copy link
Member

@im-coder-lg What OS are you on?

@im-coder-lg
Copy link
Member Author

Windows 7, but I am hoping to switch to Zorin OS within the next few weeks. I also have a Raspberry Pi running Raspbian Bullseye, the Linux tests I said were from that.

@sumeshir26
Copy link
Member

@im-coder-lg Should I merge?

@sumeshir26
Copy link
Member

Idk anyone with a mac

@im-coder-lg
Copy link
Member Author

@im-coder-lg Should I merge?

Not yet

Idk anyone with a mac

Same. I hate getting a Hackintosh as a VM, so we need to make Windows and Linux builds only, till you find a person using a Mac. I know I can't find one, but I will try.

@sumeshir26 sumeshir26 marked this pull request as draft November 15, 2021 13:36
@sumeshir26
Copy link
Member

What else do you want to add?

@sumeshir26
Copy link
Member

@im-coder-lg

@im-coder-lg
Copy link
Member Author

Sorry man, I was too busy+had headache, I will push the changes by tonight.

@sumeshir26
Copy link
Member

@im-coder-lg No problem 😉 Get well soon…

@sumeshir26 sumeshir26 added bug Something isn't working enhancement New feature or request labels Nov 16, 2021
@im-coder-lg
Copy link
Member Author

@im-coder-lg No problem 😉 Get well soon…

Thanks man, got rid of them, happens, now I am ready to get the PR for merge 😀

@im-coder-lg
Copy link
Member Author

im-coder-lg commented Nov 16, 2021

Whoa. Never expected to see the fact that, once I made breaking changes to configuration.py, and thought I pushed the changes here, the PR is ready for merge.

Edit: sorry touchslip, was commenting via phone.
We need to do the next steps:

  • Fix checks

  • Make a successful build deployer via GH actions, since a VM'd be costly if in GCP and on a local machine, it'd be advised to not do that or it'd just slow the computer down.

  • Arrange the entire codebase and fix about page.

  • Make the required files to make TimerX a community-friendly repository

We must do these steps soon. The repo is still young but we must be up for 100 commits due to my commit, revert and repeat (foolishness xD) method, and periodic push. But we are ready for the next steps.

@im-coder-lg im-coder-lg reopened this Nov 16, 2021
@im-coder-lg im-coder-lg marked this pull request as ready for review November 16, 2021 14:52
@sumeshir26 sumeshir26 self-requested a review November 16, 2021 15:05
@sumeshir26
Copy link
Member

I don’t see any changes?

@im-coder-lg
Copy link
Member Author

Well because there are no changes. I thought I made some breaking changes so I asked for time.

The PR is ready for merge.

@sumeshir26 sumeshir26 merged commit f35a489 into Futura-Py:master Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants