Slightly Nicer Windows Experience - #22
jlpoltrack wants to merge 11 commits into
Conversation
|
I guess I am not understanding this first off, if we require py to be isntalled, I see no benefit of an .exe ... rather to the contrary, because a user may think it's ok to just call the .exe what is the pyinstaller doing here as regards venv, I like using it somehow ... however I don#t like it as it is for the mac ... I think it could be possible to somehow add it into mLRSFlasher.py, so that it is setup also as part of that py, and a user just needs to run that one) |
As I see it - for a user, the only steps are installing Python then running the exe - one isn't left to install all of the necessary modules or deal with .py file associations.
It is used for the venv script and produces a 'slightly nicer' experience by:
I would also ask that you try this to see if the user experience is how you want - with only command prompts being present when someone is actively flashing a device, would be good to confirm.
This is another OS thing - |
|
MANY THX, looks very nice |
| import apInitPassthru as appassthru | ||
| import edgetxInitPassthru as radio | ||
|
|
||
| ''' |
There was a problem hiding this comment.
I guess I would like to have the venev section be moved to the last before the if main
| #-------------------------------------------------- | ||
|
|
||
| if __name__ == "__main__": | ||
| if os.path.isdir("venv"): |
There was a problem hiding this comment.
I think a if not os.path.dir('venv') is sufficient
(btw I prefer using '' over "" when possible)
| if os.path.isdir("venv"): | ||
| print("Virtual environment found, running flasher script...") | ||
| else: | ||
| if platform.system() == 'Windows': |
There was a problem hiding this comment.
can we please separate that by os.name == 'posix' and then do the subselection using platform.system()
reason is that in other parts of the code it splices by os.name, want to keep it coherent
| -------------------------------------------------- | ||
| ''' | ||
|
|
||
| def setup_virtualenv_win(): |
There was a problem hiding this comment.
it seems to me that the three can be largely slimmed down to essentially only one
if. e.g. one would set a variable
python_exe = os.path.join('venv','Scripts','python') or python_exe = os.path.join('venv','bin','python')
and use that.
Siedeffect is that I btw also very much like using os.path.join better when possible as it really helps to avoid the / vs \ thing.
the functions could have then an argument, or you do it in that function itself
pl don't use """ or ''' for inline comments (I often use that to uncomment blocks and it just makes things screwy)
| - Run ```mLRS_Flasher.py```. | ||
|
|
||
| > [!NOTE] | ||
| > - The first launch will take approximately 1 minute as the virtual environment is being created. |
There was a problem hiding this comment.
maybe
- On first launch the start can take approximately 1 minute as a virtual environment is being created and some libraries are installed (don't worry, doesn't affect your system). An internet connection is needed.
|
ok, thinking more over it I realize I don't understand how that would work ... have you explicitely tried it on a system with no py install at all? (on my system it is not sufficient tp remove all py from the PATH, it somehow nevertheless finds it, might be in the regestry or so) |
All of this still requires a python system install, the main job is to install all of the modules. Are you trying to understand what kind of error is generated when there is no system install? |
|
ah, sorry, I was misleading |
Yes, you're right - this can't work being part of mLRS_Flasher as this would call the system python without any of the modules, exiting immediately. One idea is to just have a separate script for launching with all of the OSs but not sure if you want to go this path. Let me know. |
2 participants
Adds the virtualenv back with a pyinstaller exe. Benefits: