Given that there are many applications where beeping is done constantly, I'd be hesitant to allow processes to hang around. What we really should be doing is beeping through proper APIs instead of hacking a textual command together 😉

@WasabiFan the processes only hang around if you cancel the beep.

@WasabiFan the processes only hang around if you cancel the beep.

If we're going to enable this functionality, people are going to use it.

Could we try the solution from here? http://stackoverflow.com/questions/12354586/python-what-are-the-nearest-linux-and-osx-equivalents-of-winsound-beep

@WasabiFan it is not a problem of the beep command, but how we handle it. I have an idea how to avoid the zombie issue. Let me try ...

I'm not specifically focused on this issue of processes left behind; I would much rather replace the hacks with proper calls than put lipstick on a pig.

Beep is the right tool for the job, we also use the same technique for playing wavs and the speech synth. There are no python solution for this available (pyaudio is terrible, trust me). I've fixed the zombie process issue - if you to cmd.terminate(), you need to also call cmd.wait() to get the return value and then the zombie is gone.

Ok, that's better; what do you think about returning a wrapper object which simplifies usage? That would make this easier to document in a useful way.

What do you mean by Wrapper object? All those methods return Popen objects. Those have a decent interface for being an async operation.
Also I would rather separate changing the interface and fixing unexpected behavior.

What do you mean by Wrapper object?

Maybe this is just a result of my background in more OOP-focused languages:

class SoundPlayResult:
    def __init__(self, popen_obj):
        self.popen_obj = popen_obj
    
    def cancel():
        self.popen_obj.terminate()
        self.popen_obj.wait()

Then we could add additional methods/properties for future additions and whatever other useful attributes the Popen objects have. If we later replaced the Popen calls with something more direct, we could change the internals of this class and the interface would remain unchanged.

Also I would rather separate changing the interface and fixing unexpected behavior.

Sure, we should get a review from @ddemidov. I had you as a captive audience so I figured I'd try 😉

I see Popen with shell=True is also used in set_volume. It is different in the sense that it waits for the command completion, but may be we should switch to shlex splitter here as well for consistency?

Otherwise, the changes look good. I am on a fence regarding Popen wrappers suggested by @WasabiFan. Wrapping the returned Popen objects so that wait() is called automatically is nice, but Popen objects probably have other functionality that we don't want to loose. Of course, we could solve this by making the wrapper a subclass of Popen, but it seems cleaner to just return the result as Popen instance.

Also changed set_volume().