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

Using subprocess instead of os.system #250

Merged
merged 1 commit into from
Jul 11, 2018

Conversation

prateekiiest
Copy link
Member

Continuation of #245
Fixes #239

@prateekiiest prateekiiest changed the title Using subprocess instead of os.subsystem Using subprocess instead of os.system Jul 11, 2018
Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prateekiiest , all tests are failing

@@ -13,19 +13,19 @@ def install():

@app.route('/config/<stt>/<tts>/<hotword>/<wake>')
def config(stt, tts, hotword, wake):
os.system('sudo bash /home/pi/SUSI.AI/susi_linux/access_point/server/config.sh {} {} {} {}'.format(stt,tts,hotword,wake)) #nosec #pylint-disable type: ignore
subprocess.call(['sudo', 'bash' , '/home/pi/SUSI.AI/susi_linux/access_point/server/config.sh {} {} {} {}'.format(stt,tts,hotword,wake)]) #nosec #pylint-disable type: ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace with

subprocess.call(['sudo', 'bash' , '/home/pi/SUSI.AI/susi_linux/access_point/server/config.sh', stt, tts, hotword, wake])

Or

subprocess.call(['sudo', 'bash' , 'server/config.sh', stt, tts, hotword, wake])

to not stick to location of susi_linux.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok @hongquan making the changes wherever this exists

'flite -v -voice file://{0} -f {1} -o extras/output.wav'.format(flite_speech_file, filename))
os.system('play extras/output.wav')
subprocess.call(
['flite', '-v', '-voice', 'file://{0} -f {1} -o extras/output.wav'.format(flite_speech_file, filename)])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still haven't split all command parameters here.

@hongquan
Copy link
Member

Why do you make so many small commits which solve the same error?

image

There are a lot of errors similar to 0f92d73, why don't you fix at once?

@prateekiiest
Copy link
Member Author

prateekiiest commented Jul 11, 2018

Sorry @hongquan I will squash all the commits together once I fix the errors.

The current codacy issues https://app.codacy.com/app/fossasia/susi_linux/file/19482449645/issues/source?bid=5114668&fileBranchId=7972456#l34

So I believe we should avoid these particular issues, since we are not invoking shell here.

@prateekiiest prateekiiest force-pushed the use-subprocess branch 2 times, most recently from f975bf1 to ed682a4 Compare July 11, 2018 13:52
correct syntax

fix codestyle issues

fix trailing whitespace

Update server.py

fix trailing whitespace

fix codacy issues

parse the commands correctly for flite

Fix all Codacy issues

Fix all Codacy issues
@hongquan
Copy link
Member

Later on we will replace all "sudo bash" call under access_point server.
It looks weird that the server can not be scripted to do the job of Bash (!?).

@hongquan hongquan merged commit 424b9cb into fossasia:master Jul 11, 2018
@prateekiiest prateekiiest deleted the use-subprocess branch July 11, 2018 14:33
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.

3 participants