mido_backend: Fix _find_port() when passing get_devices() name

Jajcus / python-alsa-midi

Python interface for ALSA MIDI sequencer

MIT License

13 stars 2 forks source link

mido_backend: Fix _find_port() when passing get_devices() name #3

Closed SebaUbuntu closed 2 years ago

SebaUbuntu commented 2 years ago

alsa_midi.Address(name) when "name" is something like "FLUID Synth (327406):Synth input port (327406:0) 128:0" will brutally fail causing the following exception:
alsa_midi.exceptions.ALSAError: Invalid argument
Also catch alsa_midi.exceptions.ALSAError to let the next code execute (correctly matching the port)

Change-Id: Ieadd1f74bf888fa8b07e84d09d013afbb2f39880

Jajcus commented 2 years ago

This is indeed a bug and your fix seems to be the right one. I was wandering if Address() constructor should raise two different exceptions, but it seems that is ok – when ALSA is called to do the conversion then keeping the ALSA error code seems the right thing to do, but when ALSA is not called then ValueError may still be raised.

Thank you.

SebaUbuntu commented 2 years ago

This is indeed a bug and your fix seems to be the right one. I was wandering if Address() constructor should raise two different exceptions, but it seems that is ok – when ALSA is called to do the conversion then keeping the ALSA error code seems the right thing to do, but when ALSA is not called then ValueError may still be raised.

Thank you.

It may be better to reraise the exception if ALSAError message is not "Invalid argument"? For sure an ALSAError exception may be raised for some other reason, lmk

Jajcus commented 2 years ago

It may be better to reraise the exception if ALSAError message is not "Invalid argument"? For sure an ALSAError exception may be raised for some other reason, lmk

It is still just the address conversion that fails, I don't think there is even a possibility to get a different error there (this is implemented in the ALSA library, does not even go through the kernel driver) and even if there is, I don't think it needs to be handled specially at this point.