General improvements

[lights0123]

I saw your comment on the main Ndless repository, and I thought I'd leave some feedback.

• Add a shebang line to the top of your script: #!/usr/bin/env python3. That way, you can just ./NdlessSDK_AutoInstall.py. chmod +x it before committing, it'll save the execute permissions when other people clone it.
• There's no reason for your script to run as root, as you prefix every command with sudo anyways. I would remove that check and remove sudo runuser everywhere except your apt install commands.
• Document that this only works on Debian and its derivatives (i.e. Ubuntu). You might want to check if apt exists and if not, tell the user that.

• NdlessSDK-automated-installer/NdlessSDK_AutoInstall.py

  Lines 88 to 92 in 0b297df

  	print('\n# verify build')
  	os.system('sudo apt update')
  	cm = 'echo $?'
  	res = str(exect(cm))

  replace with subprocess.run('sudo apt update').returncode. But you shouldn't need that in the first place—apt update doesn't "verify build"? Whether the build succeeded or not has no effect on apt update succeeding.

• NdlessSDK-automated-installer/NdlessSDK_AutoInstall.py

  Line 111 in 0b297df

  countdown('\n# build ndless and sdk\n! This process can take minutes to hours depending on your connection and power\nStart in..', 5)

  Why a countdown? gcc takes long enough to build anyways, no one is going to be staring at the terminal to see a message that's only on the screen for 5 seconds.

[Vogtinator]

Adding to the points above:

• This is actually a shell script written in python for some reason - using just #!/bin/sh would be easier
• If ´makesucceeds, runningnspire-gcc` afterwards is pointless, it's definitely working
• Spaces in $PATH should actually not be an issue
• It should not write to ~/.bashrc, but rather ~/.profile as that's read by all POSIX shells
• Build the SDK and Ndless itself should really not take hours. 5 minutes tops

[trueToastedCode]

Thank your improvements. I used python because it was the fastet way to create such a script. Since the python version does exist now, i won't rewrite it with sh...

-> https://pastebin.com/FANUX4C1

I tried to add you demands and it seems to work know... but there are a few things that are unclear for me:

• sudo apt update was neccessary for me while testing, but i commented it out know since it again requires root (i tested it not yet)
• Spaces where an issue for me while testing, but i am know leaving the user the choice

[Vogtinator]

cm = 'echo $?'
res = str(exect(cm))

That won't work as expected, $? is a shell variable and exect spawns a new shell every time. You should use the return value from os.system instead.

[trueToastedCode]

So if os.system('echo $?') == 0 would be working?

[Vogtinator]

Yep

Well, it would take the return value from echo, which is most likely 0. So it won't do what you want it to do.

You have to use the return value from the previous os.system.

[lights0123]

Or just use subprocess.run('sudo apt update').returncode and get rid of system, which is not recommended anymore anyways.