search

bishoph / sopare

Real time sound pattern recognition in Python for Raspberry/Banana Pi.
Other
321 stars 86 forks source link

Add config option to specify input device index #89

Open herdtie opened 4 years ago

herdtie commented 4 years ago

I struggle with setting a default device using ALSA, other people might do so, too. Also, users might not be able to adjust the default device because other applications require a different default device.

Therefore offer an option to specify input device index in config. This is a rather small change but helped me a lot.

Problem (future work): device indices are rather variable, they change if somebody plugs or unplugs a sound device, for example.

bishoph commented 4 years ago

Before anything goes into the master I usually test new code for some weeks in the testing branch. Testing branch is ahead of master at the moment. It's also unclear for me what happens if somebody specifies a string like "first" or a number that does not exist? Will look into it in detail within the next days/weeks as I'm quite busy at the moment. Thanks for the patch!

herdtie commented 4 years ago

Good thinking, I did not anticipate these common user errors. This happens when running test/test_audio.py or sopare.py -l: Enter text like "bla" in config --> get a ValueError ("invalid literal for int()...") Enter number of non-existent device in config --> IOError: [Errno -9996] Invalid device info python sopare.py -u reported successfull completion of unit tests in both cases. It is easy enough to catch these cases and fall back to the default device. Would you prefer that? Or maybe catch these cases and print out a more helpful error message like "Invalid device specified, please correct config/default.ini"? I could implement any of these.

bishoph commented 4 years ago

A clear message to the user should be the best as the desired intention was not archived. As the config is in such a case is incorrect I'm not a friend of the fallback as it's not intended and therefore could lead to lot's of questions if the user do not read the message. IMHO the message in combination with a controlled exit/stop seems to be good to catch the users attention and help to solve on the issue...

herdtie commented 4 years ago

Agreed, that would be my preference as well

---- Von: Martin Kauss notifications@github.com -- Gesendet: 07.11.20 - 09:33 ----

A clear message to the user should be the best as the desired intention was not archived. As the config is in such a case is incorrect I'm not a friend of the fallback as it's not intended and therefore could lead to lot's of questions if the user do not read the message. IMHO the message in combination with a controlled exit/stop seems to be good to catch the users attention and help to solve on the issue...

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/bishoph/sopare/pull/89#issuecomment-723418382

herdtie commented 4 years ago

Like this?

bishoph commented 4 years ago

Looks good. Going to test whenever I find some time and merge it afterwards. Again, thanks!

bishoph commented 3 years ago

Added your changes to testing branch, version 1.5.6. Changed the config option to default (-1) instead of 11 as this would not work for most of the users for obvious reasons. If you are using custom changes please use a custom config file with the "-i" option as the default.ini should work without modifications for all users out of the box.

Again, thanks for your contribution!

herdtie commented 3 years ago

Sorry, the 11 was a mistake, just used that for testing. Thanks merging

---- Von: Martin Kauss notifications@github.com -- Gesendet: 28.11.20 - 13:44 ----

Added your changes to testing branch, version 1.5.6. Changed the config option to default (-1) instead of 11 as this would not work for most of the users for obvious reasons. If you are using custom changes please use a custom config file with the "-i" option as the default.ini should work without modifications for all users out of the box.

Again, thanks for your contribution!

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/bishoph/sopare/pull/89#issuecomment-735226914