engine.trigger not working for control connected in XML

Bug #1607277 reported by Sébastien BLAISOT
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mixxx
New
Undecided
Unassigned

Bug Description

I connected [EffectRack1_EffectUnit1],group_[Channel1]_enable to a led in XML.

then, after some change in the led states, in javascript, trying

engine.trigger("[EffectRack1_EffectUnit1]","group_[Channel1]_enable");

to update the leds doesn't have any effect, the leds are not updated.

is it possible to update leds from javascript engine.trigger when the led are connected in XML ?

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

The trigger code operates on script-specific COs, but I don't think there's a reason the signal ControlObjectScript::trigger couldn't be connected to whatever COs the XML get mapped to. (I can't find it right now.)

Revision history for this message
Daniel Schürmann (daschuer) wrote :

I probably do not understand the issue. Here is what I have got:

> I connected [EffectRack1_EffectUnit1],group_[Channel1]_enable to a led in XML.

This should work as usual. The LED should always reflect the "[ Ch1 ]" button in the LateNight skin. Is this true on your side?

> then, after some change in the led states, in javascript, trying
> engine.trigger("[EffectRack1_EffectUnit1]","group_[Channel1]_enable");
> to update the leds doesn't have any effect, the leds are not updated.

https://github.com/mixxxdj/mixxx/blob/7c58008d3ff35026763fdf7198905b068435e07e/src/controllers/controllerengine.cpp#L696

This should transmit the current state of the CO to the LED. It should be not required unless there is an another instance which is controlling the LED.
But anyway, after this, the LED and the GUI button should be in synch. Is this true on your side?

> is it possible to update leds from javascript engine.trigger when the led are connected in XML ?

If you uses engine.trigger("[EffectRack1_EffectUnit1]","group_[Channel1]_enable", value), both the mapped controller LED and the GUI LED should be updated. If this is not the case, it is a bug. Does it happen?

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

> This should work as usual. The LED should always reflect the "[ Ch1 ]" button in the LateNight
> skin. Is this true on your side?

Yes, this is true, unless some other function changes the led.

> This should transmit the current state of the CO to the LED.

No, Engine.trigger calls whatever javascript function is connected to a CO through Javascript, but it seems that it doesn't update GUI or doesn't send midi messages to update the leds mapped through XML

> If you uses engine.trigger("[EffectRack1_EffectUnit1]","group_[Channel1]_enable", value),
> both the mapped controller LED and the GUI LED should be updated. If this is not the case,
> it is a bug. Does it happen?

No, calling engine.trigger doesn't update the controller LED when that led is mapped through XML.

You can see how I had to workaround that here https://github.com/sblaisot/mixxx/blob/ba99922caa76415bf76b6048078db0ab6b232a58/res/controllers/Reloop-Beatmix-2-4-scripts.js#L479

I'll try to make a test mapping to demonstrate that.

Revision history for this message
Daniel Schürmann (daschuer) wrote :

I am glad you have a workaround. I think I got the point now no need for a demo mapping.

This means we should make sure that all mapped midi devices receive an update on trigger().
Something like a setValue(getValue()); without an value equal check right? This fits to the current description for trigger().

It the issue only about the controller LED?
The GUI should be never out of sync, I think it is true, right?

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

yes, GUI is always in sync because there is only one CO mapped to a visual feedback.

but I suspect if you achieve to make a gui visual feedback change without CO change (what happens in the javascript for controller led when you use a led to give another meaning in certain circumstances), then it will be out of sync and engine.trigger will not restore the sync.

engine.getValue(engine.setValue()) should also be a workaroud because this makes the CO change (even with the same value) and then updates the feedback (either GUI or XML controller led mapping). I didn't try it though.

Currently, engine.trigger() does not update XML-mapped outputs. These are only updated on CO changes.

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

FWIW, when I originally wrote engine.trigger(), it did exactly that: a setValue(getValue())

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

I just tested it and setValue(getValue()) doesn't refresh the leds on the controller.

I even don't see any "Debug [Controller]: sending MIDI bytes: xxx, xxx, xxx" in the output, so Mixxx doesn't refresh the leds when a CO is set to the same value it already has .

Neither engine.trigger() nor engine.setValue(engine.getValue()) work to resync controller leds mapped in XML. The only way to achieve a resync is to *change* (i.e. set to ANOTHER value) the CO or as I did, to implement led logic in javascript based on CO value.

IMHO, engine.trigger() or setting the CO to the same value it already have should fire a resync to handle this case, but this could have side effects if we have CO set frequently without changing value.

what do you think?

Revision history for this message
Daniel Schürmann (daschuer) wrote :

It looks like the trigger() feature was broken along with some factorings / new features.

There are two reasons:
1.) Today we have to independent and highly ControlProxy objects for MidiOutput and Scripting.
A trigger() on the script on only triggers the connected script function. It knows nothing about a probably existing Output Midi mapping
2.) The value holding ControlDoublePrivate has a IgnoreNops feature, which is enabled by default. This prevents the setValue(getValue())

I think we cannot choose a solution that re-enables setValue(getValue()) bypassing IgnoreNops, because the Mixxx code relies in some places to the fact that the same value is never received twice.

To repair the trigger() feature for hardware ouputs only, we need a signal path, that connects the MidiOutput and scripting objects.

What do you think?

Revision history for this message
Sébastien BLAISOT (sblaisot) wrote :

Thanks for the explanation Daniel. It's clear for me now.

I suggest explaining that on the wiki to avoid confusion to script developers and give some hint on how to workaround this bug until it can be fixed.

Revision history for this message
Swiftb0y (swiftb0y) wrote :

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/8620

lock status: Metadata changes locked and limited to project staff
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.