kephale / pulser

A library of psygnal generators
Apache License 2.0
0 stars 1 forks source link

Advice on Signal and SignalInstance #1

Open kephale opened 1 year ago

kephale commented 1 year ago

https://github.com/kephale/pulser/blob/e3ba65908f74254c411b6431a4a0568d2914a7f8/src/pulser/midi.py#L64

@tlambert03 I wanted to try to use a dict of Signals, but the implicit instantiation that happens when a containing class is created does not work when the Signal is in a dictionary (makes sense).

Previously, I did start by having a single Signal for the midi device which works as a class attribute, but it wasn't very elegant to have to parse event information within each connected function.

The quoted approach currently works, but I suspect it is not desirable to grab a SignalInstance so directly.

This is the code that I'm trying to support:

@magic_factory(
    call_button="XTouch",
)
def xtouch_widget(viewer: "napari.viewer.Viewer", listening=False):
    global xtouch

    if listening:
        game = ping_pong(viewer)

        p1_signal = xtouch.knob_signals[1]
        p2_signal = xtouch.knob_signals[8]

        # Slider movement control
        @p1_signal.connect
        def p1_controller(event):
            value = event

            norm_value = value / 127.0
            pmin = game.bar_radius
            pmax = game.height - game.bar_radius

            game.player1_position = pmin + norm_value * (pmax - pmin)

        @p2_signal.connect
        def p2_controller(event):
            value = event

            norm_value = value / 127.0
            pmin = game.bar_radius
            pmax = game.height - game.bar_radius

            game.player2_position = pmin + norm_value * (pmax - pmin)
tlambert03 commented 1 year ago

but I suspect it is not desirable to grab a SignalInstance so directly.

that's fine actually! Signal is just a thin descriptor that generates SignalInstances bound to specific objects. The logic for that SignalInstance creation happens here Signal.__get__ (as it appears you've already discovered yourself). So, you'd be much better off just directly calling SignalInstance(int, instance=self, name=f"knob_{idx}") rather than using Signal(...).__get__(self).

SignalInstance is a public class and will definitely remain so. The Signal descriptor abstraction was mostly there to create API parity with what people are already used to in Qt signals/slots.


Another thing that might be useful here is to create a SignalGroup ... but let me take a closer look at the code and see if it would be better or just "different" :)