MattIPv4 / PyDMXControl

A Python 3 module to control DMX using OpenDMX or uDMX - Featuring fixture profiles, built-in effects and a web control panel.
https://pypi.org/project/PyDMXControl/
GNU General Public License v3.0
122 stars 23 forks source link

Discussion: Architectural Considerations for Live Band Lighting Controller #52

Open crscheid opened 1 year ago

crscheid commented 1 year ago

Publishing this as more of a discussion item for @MattIPv4 to consider and perhaps comment on rather than an actual enhancement request appropriate for this library. A lot of what I've run into in my work might end up being breaking changes if actually tackled as an enhancement to this library. I'm considering branching off of this project to do some of these enhancements but before doing so wanted to outline my thinking.

ℹ️ Background: I'm attempting to use PyDMXControl as the underlying library of a Midi controllable show controller for live bands that speaks DMX and will control multiple fixtures based on preprogrammed patterns. In doing so, I've come across some edge cases that possibly rub up against some core design assumptions of PyDMXControl.

Clarify and Align speed, offset, and delay parameters

I noticed that speed is in ms while offset and delay are in percentage integers (i.e. 50 = 50%). Took me a while to work out the math to figure out what was going on here. It might be helpful to those using this in precise timing applications to keep these consistent as ms.

Proposed Sequence Effect Class

I had the need to be able to kick off a string of animations targeted towards a group of fixtures but then also loopback to a specific step. Consider the example of a scene change that bursts the lights to full white then begins cycling a chase-like pattern through 4-5 steps.

To implement this, I created a new Sequence Effect class that allows one to pass in a list of Fixture operations that you'd like to perform at certain times in sequence with ability to loopback to a specific index and define the step timing. Steps are passed in a dict of tuples where the key is the ms value at which to perform the animation and the tuple contains the Fixture operation, and arguments. You can also offset by step instead of percentage, and define a loopback point in that list of steps.

The following will run all lights Pink, Blue, Red, Warm, Green, then cycle Red, Warm, Green in a loop. Because offset_index is False, all lights are in sync. If offset_index was True, then the first light in the all_lights list would start on pink, the second would start on Blue, the third on Red, etc.

all_alights = dmx.get_fixtures_by_name_include('Band')
Sequence.group_apply(all_alights, offset_by_index=False, loopback_index=2, steps={
    0: [["color",Colors.Pink,0]],
    1000: [["color",Colors.Blue,500]],
    2000: [["color",Colors.Red,500]],
    3000: [["color",Colors.Warm,500]],
    4000: [["color",Colors.Green,500]]
})

Proposed Ability to Cancel Fixture Animations

Considering in my code I now have this Sequence class that can apply animations across fixtures, if I want to enable a new Sequence, I need the ability to not only remove the Effect from the callbacks (e.g. my specific Sequence class), but also the animation calls that were attached to the fixtures. I encountered this when applying this use case:

Let's say I have a Sequence that slowly fades out all lights over a period of 8000ms followed by bringing those lights up to half dim in 5000ms. (good for end of song and then bringing house lights backup before next song 😎)

Before that first fade out is completed, let's say my Light Controller responds to a Midi input 4000ms into that Sequence that requests a new sequence be executed instead. To handle this I can remove the callback that refers to my Sequence which would prevent the second half dim 5000ms call from being executed.

However, the previously executed call to the Fixtures that is still running "color" or "dim" over the initial 8000ms is running and cannot be stopped.

Removing all callbacks at the controller level is a no-go because the __send function that transmits values to DMX is part of that group.

Still considering the best way to handle this. I've had success clearing all callbacks and then adding back the__send callback, but it seems hackish. Still pondering and experimenting. 🤔

dmx.clear_all_effects()
dmx.ticker.clear_callbacks()
dmx.ticker.add_callback(dmx._TransmittingController__send, 0)

At any rate, you can probably see that my use is running into some possible design assumptions. I'd be happy to consider thoughts and opinions on this. I would love to contribute to this project and its functionality rather than creating a Frankensteined branch.

Thanks for listening.

MattIPv4 commented 1 year ago

Clarify and Align speed, offset, and delay parameters

I would be open to a breaking change PR to make these more intuitive and consistent 👍

Proposed Sequence Effect Class

I'm not sure I am a fan of implementing this. To me, it feels like it is stretching what I'd consider to be an "effect" into what is really just control functionality. The library already has the ability to run a sequence of events using TimedEvents, and one could add some custom logic around this to allow it to loop if you wanted.

Open to suggestions on improving the interface of TimedEvents though 😄

Proposed Ability to Cancel Fixture Animations

Could you provide a code sample using one of the built-in effects for where this is going wrong? The base Effect class has a cleanup method that removes the callback from the internal ticker to stop animations?

crscheid commented 1 year ago

Thanks for responding.

The library already has the ability to run a sequence of events using TimedEvents

Cool, I did not see this in the library. Let me take a look at this and see I can use this aspect to meet my needs.

Could you provide a code sample using one of the built-in effects for where this is going wrong?

I took another look at the built-in effects and those effects do not utilize calls to fixtures that animate. Both Chase and and Dim track the interim values of the event and make the call to the fixture like:

self.fixture.color(color, 0)

or

self.fixture.set_channel('dimmer', int(255 * progress))

In both of these cases with the built-in effects, the call to Fixture is done instantly and the effect manages the values over time. Contrast that with my previously mentioned Sequence class where I am calling fixtures like this:

self.fixture.color(color,time_ms)

or

self.fixture.dim(dim_level, time_ms)

In my case I'm passing in the milliseconds argument to the FixtureHelpers methods which then ultimately passes that value onto the dim method which then creates its own callback with the ticker. It is this callback that continues despite clearing the effect.

Its possible that perhaps there was never an intent to allow direct access to the FixtureHelper methods that do this in the case of effects. I just found it to be very convenient instead of tracking all of the interim values within my effect.

So in effect, if you use these calls in the FixtureHelper, it appears you loose control over those animations.

MattIPv4 commented 1 year ago

So in effect, if you use these calls in the FixtureHelper, it appears you loose control over those animations.

Ah! That makes sense, and is definitely an oversight -- I would be happy to accept a PR that implements something internal on FixtureHelper/Fixture to keep track of the current animation, and expose a method to clear it. That method should probably also be called automatically any time a new animation is started, so things don't clash.

crscheid commented 1 year ago

I would be happy to accept a PR that implements something internal on FixtureHelper/Fixture to keep track of the current animation, and expose a method to clear it.

Ok cool, let me focus on that first then. Doing so will unblock progress on my end. I've got two other feature requests (#49 #50) as well so I'll try to package those in with this.

crscheid commented 1 year ago

I would be happy to accept a PR that implements something internal on FixtureHelper/Fixture to keep track of the current animation, and expose a method to clear it.

@MattIPv4: I've got a branch in my fork that is ready for a PR, but I still have the outstanding PR that supports resolving #49 outstanding. Did you want to approve that PR first before me submitting another PR for this - or would you rather I package these all up at once?

My solution is to track local callbacks assigned within the FixtureHelper class and to clear them out along with effects when clear_effects() is called on the Fixture. Seems to work well and won't affect any prior utilization of this library. Everything is self contained within Fixture()'s interface. 🤘🏼