client: add get_address() and fd()

[thanks4opensource]

This pull request adds new SequencerClientBase.get_address() and SequencerClientBase.fd() methods, and tests/test_sequencer_address.py for the former. Also a modified examples/connect.py demonstrating the new .get_address() capabilities.

SequencerClientBase.fd() is a public wrapper for ._fd. Access to the ALSA file descriptor is needed for programs doing select() (e.g. with stdin), and ones that use GUI toolkits with gui.add_fd(); gui.run() semantics. I have successfully used SequencerClientBase._fd is both cases, in addition to snd_seq_poll_descriptors() in C/C++ programs. Do you see any problem with doing this? The new .fd() method makes it "official", as well as future-proofing in case of ALSA library changes.

SequencerClientBase.get_address() returns an Address given various forms of client and port inputs such as "client-name:port-name" strings, separate "client" and "port" strings, int client and port IDs, tuples of these, Address objects, and mixes/combinations of them. It returns an Address only if the specified client/port exists, raising an exception if not. It accepts prefixes/abbreviations/substrings of client and port names, and matches current Address() -- and the underlying ALSA C library's snd_seq_parse_address() -- semantics of returning Address(client_id, 0) if given a client without a port, but only if the client exists and has a port 0 (almost universal, but not enforced by the library).

An irony is that I have submitted alsa-project/alsa-lib#528 which includes much (but not all) of this functionality directly in the ALSA C library. See that pull request for the reasoning that motivated these proposed enhancements. I originally experimented with including the "name-or-numeric-ID" capability of the enhanced snd_seq_parse_address() directly into Address by adding an optional SequencerClientBase parameter to an Address.__new__() overload. But I am unsure when (if ever) the alsa-lib pull request will be merged, I didn't want python-alsa-midi to depend on an updated release of the ALSA library, and it seemed cleaner and more Pythonic to implement it as a SequencerClientBase method anyway. Given SequencerClientBase.list_ports() this was fairly easy to do, although overload input validation and checking for port/client existence required significant additional code. I also considered merely returning None for non-existing client/ports but again felt exceptions better matched the rest of python-alsa-midi's semantics.

Included is a unittest program, tests/test_sequencer_address.py which extensively checks many (currently 4179) permutations of input types and expected successful returns or exception failures. Both the new and all existing tests run without error in this branch. A modified examples/connect.py demonstrates connecting clients/port by name instead of by numeric IDs.

Please consider merging this pull request. Feel free to modify/re-architect it as you see fit, hopefully retaining all of the current capabilities. Thank you.

[Jajcus]

Thank you for your merge requests. A few comments...

• These should be probably two Merge Requests, or at least two separate commits.
• Tests fail, as your tests do not integrate with existing test infrastructure (tox & pytest) at all. That is why the CI checks fail for the MR, and it should be verified with tox before pushing.

And now about the actual changes:

For the fd() method... I don't like it being a method. Maybe, if it was .fileno(), as in IOBase (with the same semantic), it could be argued for, but fd()? Not even get_fd()? But get_* functions here are usually direct interfaces to the underlying ALSA API, so it would be also confusing.

To me the most reasonable would be to make this a read-only property. client.fd, typed 'int | None'.

For the get_address(). I think the name is confusing, sounds like it is getting address of the client. Maybe resolve_address()? Sounds more like what the function is doing.

[thanks4opensource]

Thanks for reviewing the pull request. I've made changes as per your comments. Continuing the discussion ...

I have no strong preferences regarding the fd() or "address()" methods. I changed get_address() to resolve_address(). I changed fd() as follows:

+    @property
     def fd(self):
         "Returns the ALSA sequencer file descriptor number."
         return self._fd

Is this acceptable? It mandates use as client.fd (without "()") and raises an exception on client.fd = value. The only thing I don't like about it is that help(client.fd) doesn't display the docstring, nor does anything else I could find. I couldn't figure out the required syntax for typed 'int|None', etc, so if there's a better way using that please tell me the exact code (or feel free to change/add it yourself).

I apologize for the tests issue. I had little prior exposure to pytest, and had never heard of tor. I tried variants of python3 setup.py test without success and concluded there was no built-in way to run the tests in the directory. I resorted to doing:

$ cd tests
$ for test in *.py ; do
> echo -n $test '  '
> python3 $test && echo $?
> done

which worked for my new test_sequencer_address.py but of course did nothing with the others as I now see. I would have known that if I'd looked at their code.

I'm a huge believer in unittests but have no love for test frameworks like pytest/tor/etc. IMO they add many layers of complexity and rigorous demands while providing little benefit in return. Let me write whatever form of test I want as long as it runs silently returning 0 if successful to whatever process has forked/execl'd it, and outputs error text and returns non-0 if not. Your opinion may differ, and since this is your repo I modified the code to run at least passably well under tor/pytest.

With one exception: I strongly object to tor's style checker. As a tiny, poor example:

    sender = client.get_address(args.sender)
    dest   = client.get_address(args.dest  )

Is inarguably more readable than:

    sender = client.resolve_address(args.sender)
    dest = client.resolve_address(args.dest)

The human visual system (really that of all animals) has evolved to rapidly recognize patterns. You can glance at the first form and immediately see that some method is being called twice, with different members from the same object, and the result is being assigned to two different variables. You can look more closely if you're interested in the details, but in the second form you have to do that to understand what's going on.

I'll stop lecturing here, but the bottom line is it's been my policy to try to match the style of code I'm modifying and use my own for entirely new code. It's a gray area when I'm adding a large block of new code to an existing source file, and I erred on the side of "mine" in what I added to client.py, which I've corrected in this new commit. My apologies.

But for now I've left test_sequencer_address.py largely alone which of course raises hundreds of style errors from tor. So I have to ask: Is there any way to turn off the style checking, possibly just for this specific source file? Would you be willing to do so? Or is the decision out of your hands -- does whoever runs PyPi/pip insist on this level of compliance (even for a unittest that doesn't get installed by a normal user) before they'll accept the package? BTW, I'm getting dire warnings that setup.py is deprecated (along with some for tor and pytest) which is among the many reasons I have little respect for these fad-of-the-year software development environments/requirements.

And more generally, I have to ask what your level of interest is in receiving this kind of unsolicited new feature merge request? This is an old project for you and I'm sure you've moved on to other things and have little time to devote to it. You've been very responsive here, especially compared to https://github.com/alsa-project/alsa-lib where the backlog of pull requests (yes, I'm aware of the issue with mine) convinces me that it's effectively a dead/abandoned project. (And aren't people saying that ALSA itself has been deprecated, first by JACK and now by pipewire?)

For myself, I had a window of time to work on this, and it's rapidly closing. I had for years wanted to consolidate my usage of ALSA, ppaez/alsaseq, rtmidi, etc. in C/C++/Python -- and fix the problems I was having with them -- in the dozen or so personal applications and utilities I've written and maintain. I also wanted to solve the years of frustration I've had with not being able to address ALSA clients and ports by their names.

So again, the bottom line: If you'll give me a clear indication that you'll merge this pull request -- and the next, final one (a full-featured, improved Python replacement for aconnect/aconnect.c including another, purely orthogonal addition to SequencerClientBase which it requires) -- I'll spoil the maintainability of test_sequencer_address.py by "fixing" its whitespace/indentation, although again I'd much rather find a way to get tor off my back and leave it as is. If not, I can just let the code sit in my own forked repo. I've put in too much time and work on it to keep it privately to myself when I think others could benefit from it. That's always the decision I have to make when I start something like this. I could have finished it all in 10% of the time if it was only for my own personal use: Sloppy code, no unittests (which always take up 3x the time as the actual code they test), find and fix any bugs as they come up in my day-to-day use. But it has to be more rigorous if it's being distributed to the outside world, especially as part of a project like yours which, as I've said before, I consider to have a very high standard of software quality.

Please tell me how I should proceed, and again thanks for this repo and the time you've spent on it.