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

data parameter of the set_callback method isn't clear #55

Closed
jmeile opened this issue Dec 31, 2019 · 4 comments
Closed

data parameter of the set_callback method isn't clear #55

jmeile opened this issue Dec 31, 2019 · 4 comments

Comments

@jmeile
Copy link

jmeile commented Dec 31, 2019

I think the documentation of the data parameter of the set_callback method should be improved. For me it wasn't clear what it does. I had to read the C++ documentation in order to know what this is about.

After reading the python documentation of set_callback:
https://spotlightkid.github.io/python-rtmidi/rtmidi.html#rtmidi.MidiIn.set_callback

which says:

and the second argument is value of the data argument passed to this function when the callback is registered.

It wasn't clear to me what this data argument was about. After reading the documentation of the C++ source code:
https://www.music.mcgill.ca/~gary/rtmidi

There it is written:

It is possible to provide a pointer to user data that can be accessed in the callback function

For me, that's much more clearer. So, I guess that data argument could be everything. Even an object instancing an own defined class, ie:
midiin.set_callback(MidiInputHandler(port_name), my_object)

Then I guess you can do things like:

class MidiInputHandler(object):
    def __init__(self, port):
        self.port = port
        self._wallclock = time.time()

    def __call__(self, event, data):
        message, deltatime = event
        self._wallclock += deltatime
        print("[%s] @%0.6f %r" % (self.port, self._wallclock, message))
        data.my_method(self.port, self._wallclock, message)

Is this the purpose of the data parameter? If so, you could even extend the midiin_callback.py example and instead of printing the message in the callback, you could define a class that does this or perhaps put some comments to that example telling that you could do that as well.

Thanks and a happy new year to you.

Best regards
Josef

@SpotlightKid
Copy link
Owner

SpotlightKid commented Dec 31, 2019

Is this the purpose of the data parameter?

Yes, that's correct. Though it is less needed in Python than in C/C++. As you have done in your example, in Python you can make the callback a class method, and that class can act as a namespace or container for any data you need access to in the callback (e.g. in your example self._wallclock).

User data that is provided when registering a callback and passed to the callback whenever it's called isn't a concept that's particularly specific to rtmidi, but used by many frameworks that use callbacks (e.g. Gtk or Qt), so the documentation assumes the developer is familiar with that. Of course this could be described in more detail and with some examples. If you have any suggestions for the actual text, I'm open for PRs.

As for the example scripts, like I said, using the data parameter isn't really needed in Python, unless you are limiting yourself to classless code, so I didn't see the need for one doing so. There are examples, which demonstrate the class-based approach, though (e.g. examples/advanced/recvrpn.py or examples/midi2command/midi2command.py). I'm open for PRs in that regard too (keep the examples simple).

@jmeile
Copy link
Author

jmeile commented Dec 31, 2019

What about this text:

and the second argument is value of the data argument passed to this function when the callback is registered, whose type can be either a basic data type, or an object and it can be used inside the callback.

I think that for newbies that would be a good way of understanding this. Anyway, it is just a suggestion.

Best regards
Josef

@SpotlightKid
Copy link
Owner

Thanks for the suggestion, sounds good. I'll update the documentation next week. A new minor release of python-rtmidi is planned for Jan, 15th.

@SpotlightKid
Copy link
Owner

Updated docstring of MidiIn.set_callback in f0b65a0.

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

No branches or pull requests

2 participants