Skip to content

Add QmlControlProxy::trigger()#15420

Merged
acolombier merged 4 commits into
mixxxdj:mainfrom
JoergAtGithub:qml_trigger
Nov 23, 2025
Merged

Add QmlControlProxy::trigger()#15420
acolombier merged 4 commits into
mixxxdj:mainfrom
JoergAtGithub:qml_trigger

Conversation

@JoergAtGithub
Copy link
Copy Markdown
Member

This PR adds a trigger method to QmlControlProxy. This is using a single shot timer with zero interval, to ensure that both updates (1 then 0) will correctly be reported to the CO slots, as two seperate events in the event loop.

…hot QTimer for value resetting after event loop processing.

Replaced the existing hardcoded trigger function in Sampler.qml by calling the new method.
@Swiftb0y
Copy link
Copy Markdown
Member

IMO it would be better if trigger just emitted the controls valueChanged signal.

@JoergAtGithub JoergAtGithub force-pushed the qml_trigger branch 2 times, most recently from 311b316 to bd6306f Compare September 27, 2025 10:11
@JoergAtGithub
Copy link
Copy Markdown
Member Author

IMO it would be better if trigger just emitted the controls valueChanged signal.

@acolombier wrote in #14849 (comment) that both states are important for property binding. Therefore I implemented it this way.

@acolombier
Copy link
Copy Markdown
Member

I believe you might be able to also emit the two signals back to back so they both get addressed in the eventloop, in case you don't want to use the timer.

Thanks for looking into this! <3
I'll check it when I'm back at my keyboard

@Swiftb0y
Copy link
Copy Markdown
Member

IMO it would be better if trigger just emitted the controls valueChanged signal.

@acolombier wrote in #14849 (comment) that both states are important for property binding. Therefore I implemented it this way.

Right, thanks for the pointer. I still didn't find a way where this was actually required. I can only find this used for the "eject" control.

ControlObject::set(ConfigKey(group, "eject"), 1.0);
ControlObject::set(ConfigKey(group, "eject"), 0.0);

IMO we should first fix the eject CO behavior and then see if the double value in the event loop is required anywhere else. I think the primary issue stems from the fact that "eject" is set up as ControlPushButton, even though it really only behaves as a basic signal in practice.

@acolombier
Copy link
Copy Markdown
Member

AFAIS, this snippet would indeed induce two consecutive emits (one for 1.0, one for 0.0) which should indeed be sufficient for the property binding to detect this trigger!

So in theory, you should be able to remove the QTimer and have two consecutive setValue

@JoergAtGithub
Copy link
Copy Markdown
Member Author

IMO we should first fix the eject CO behavior and then see if the double value in the event loop is required anywhere else. I think the primary issue stems from the fact that "eject" is set up as ControlPushButton, even though it really only behaves as a basic signal in practice.

We've also

script.triggerControl = function(group, control, delay) {
if (typeof delay !== "number") {
delay = 200;
}
engine.setValue(group, control, 1);
engine.beginTimer(delay, () => engine.setValue(group, control, 0), true);
};

Which is used for many other controls:
https://github.com/search?q=repo%3Amixxxdj%2Fmixxx+path%3A%2F%5Eres%5C%2Fcontrollers%5C%2F%2F+triggerControl&type=code

Why do you think this is specific for "eject" ?

Copy link
Copy Markdown
Member

@acolombier acolombier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for looking into this. I tested locally to see if we could get this to work without the timer and it seems to works great.

Comment thread src/qml/qmlcontrolproxy.cpp Outdated
Comment thread src/test/qmlcontrolproxytest.cpp Outdated
Comment thread src/test/qmlcontrolproxytest.cpp Outdated
JoergAtGithub and others added 2 commits November 23, 2025 09:52
Co-authored-by: Antoine Colombier <7086688+acolombier@users.noreply.github.com>
Copy link
Copy Markdown
Member

@acolombier acolombier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks for addressing this!

@acolombier acolombier merged commit 670e716 into mixxxdj:main Nov 23, 2025
15 checks passed
@JoergAtGithub JoergAtGithub deleted the qml_trigger branch November 23, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants