Warning [Controller]: ControllerEngine::slotValueChanged() Shouldn't happen -- sender == NULL

Bug #1136890 reported by rob
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mixxx
Expired
Medium
Unassigned

Bug Description

I figured out what's causing this issue. See my post lower down on the page for an explanation. It does seem there is a bug or two in Mixxx connected to this issue.

I've confirmed the issue exists in 1.11 build r3750 and r3767+

###########################

I'm getting the following error when I call engine.Trigger in a controller script:

Warning [Controller]: ControllerEngine::slotValueChanged() Shouldn't happen -- sender == NULL

...and there's some other strange behaviour associated with the error. I'm also getting messages saying:

Warning: [Controller]: Could not Disconnect connection "myfunction"

I'm assuming from the "shouldn't happen" bit that this error message shouldn't be displayed, so I suspect that I might have found a bug in mixxx rather than (or maybe in addition to) a bug in my controller script.

Revision history for this message
rob (another-rob) wrote :

Ok, I think I've found the problem (or part of it at least). When I try to set certain hooks, instead of returning true or false, engine.connectControl seems to be returning the following text:

ControllerEngineConnectionScriptValue(name = "")

...and the connection fails.

Revision history for this message
rob (another-rob) wrote :

ControllerEngineConnectionScriptValue(name = "")

is returned by engine.connectControl only when setting hooks, and it seems to always return that text string whether the hook set was successful or not.

When clearing hooks using engine.connectControl(..., true), it seems that true is returned whether the connection is cleared successfully or not - even if a 'Warning [Controller]: could not disconnect connection "myfunction"' error is displayed.

Revision history for this message
rob (another-rob) wrote :

OK, figured out a workaround, and figured out why this was happening. Given the messed up return values for connectcontrol, and the "shouldn't happen error", it does seem that there's a bug in the mixxx code, but I've figured out why my connects and triggers weren't working. It happened when I was switching modes, trying to set a hook using engine.connectcontrol where the hook I was setting (eg: hotcue_10_enabled) was assigned to a button/LED in the previous mode, and it hadn't been cleared yet. I had a for loop that functioned like this:

for (each button/LED){
if LED was previously hooked to a mixxx control, then clear the control.
set the new hook.
Trigger the new hook to set the LED state.
go to the next LED
}

I have two hotcue modes. One displays hotcues 1-24, the other displays hotcues 9-32. All hooks point to one function, which figures out which LED to light based on which hotcue triggered the function. The LED setup for loop would fail when switching between these modes - eg: switching from 1-24 to 9-32, button/LED 1 would clear ok (previously hotcue_1_enabled), but connecting it to hotcue_9_enabled would fail, apparently because hotcue_9_enabled is already connected to button/LED 9, and that hasn't been cleared yet (but connectcontrol would return "ControllerEngineConnectionScriptValue(name = "")" instead of false). Then when I trigger hotcue_9, I would get the "shouldn't happen" error.

When the for loop gets to the last set of buttons, it starts working again, because hotcues 25-32 weren't set in the previous mode.

I fixed my script by splitting that into two loops - the first clears all the hooks, then the second for sets new hooks. No more errors. But it seems that there is a bug in mixxx (probably in controllerengine.cpp - the "shouldn't happen" error comes from line 827) given the error messages and the messed up return values for connectcontrol.

In case it's useful, I've attached the pre-fix version of my controller script, with a bunch of debugging code in place. The for loop I described above is in NK2.updateLEDs(), which starts around line 193

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

Wow, thanks for all the information and testing, Rob! I've been waiting for RJ to respond here since he knows more about the internal control system code that's producing the "shouldn't happen" message. I can say that in previous discussions with him that there is indeed an issue with its ability to handle rapid updates which is why you were seeing problems, but it's due to a flaw in the design of the system, something which can only be remedied by a complete redesign (which we do have planned, dubbed "Control 2.0.")

There's actually nothing wrong with connecting Mixxx controls to multiple script functions; in fact we explicitly provided for that possibility. I think your problem was simply due to the speed of the connections/disconnections, since the internal controls are actually processed in a different thread. It would be interesting to see if your pre-fix code works correctly if you add a busy-wait delay on each loop iteration. Would you mind trying that and report back here? (That wouldn't be a good workaround though since busy-waits in controller scripting are a no-no as they can cause Mixxx to stutter and affect response of other controllers.)

Can I ask why setHook() loops on connectControl? That would connect it multiple times to the same function which might cause problems. Did you ever see it iterate more than once?

So for now, your workaround is the best thing you can do. If you're interested, you can keep an eye on the Control 2.0 spec page to monitor progress: https://blueprints.launchpad.net/mixxx/+spec/control-2.0

Changed in mixxx:
importance: Undecided → Medium
status: New → Confirmed
Revision history for this message
rob (another-rob) wrote :

No worries. Was a frustrating error right as I was finishing up a new mapping, so wound up just hammering at it until I figured out what was going on. While debugging I tried setting a timer in the loop to connect after a delay, and I got the same errors. I didn't test that too extensively, since it's obviously not a workable solution.

The sethook loops were just debugging stuff - was driving myself crazy trying to figure out what was fucking up, so put all the setHook code in a separate function so I could add debug stuff around it. Wouldn't have put loops in otherwise - the things weren't connecting, so I figured I'd hammer at them until they did. That's where I figured out that the return value of connectControl is messed up. Depending on how I set the while condition, they sometimes repeated - if it was while (done===false) they wouldn't repeat, because after one iteration the return value was "ControllerEngineConnectionScriptValue(name = "")" (and not true or false), so it didn't evaluate as false even if the connection didn't happen. If I set the while as while (done!==true) it repeats forever, because "controller..." isn't === true.

Having done a bit of looking at the mixxx source code in the course of this, I am getting tempted to try getting my feet wet with c++ and trying to contribute to the actual program itself instead of just doing mappings... It doesn't seem like it'd be too much of a jump, but I'm already using writing controller scripts as a big procrastination excuse for the actual work I have to take care of, so maybe that's not a great idea right now... haha.

Hey, I wanted to ask you a few questions about your scs.3d script at some point - great mapping, btw... I was wondering how much work it would be to make it address Channel1 through 4 instead of just 1 and 2... I've looked at it a bit, but haven't really dug in that much. I've got 2 scs.3ds and two other modular scratch controllers - I've mapped the others to handle four channels, but ideally I'd like to hack your script to do four as well... I'll drop you a note sometime with some questions once I have a chance to look at your script in a bit more detail.

Revision history for this message
rob (another-rob) wrote :

Incidentally, the Korg Nanokontrol2 mapping I was working on that produced this error is done and working fine if you guys want to add it to 1.11 - it's here: http://www.mixxx.org/forums/viewtopic.php?f=7&t=4759

RJ Skerry-Ryan (rryan)
no longer affects: mixxx/2.0
Changed in mixxx:
milestone: none → 1.11.1
Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

Rob: Sorry for the delay, I didn't see your response amid the tons of bug E-mails I get.

The SCS.3d script is fully "variablized" as far as deck control goes. You'd only need to change the deck change function to increment-and-check against [Master],maxDecks instead of just increment or decrement, then do something meaningful with the A and B LEDs (I'd probably have them both lit for deck C and neither for deck D.) I do this in the SCS.1d script so that it automatically supports as many decks that Mixxx itself does, but that controller has an ASCII character display for the deck, so it automatically supports 35 decks, more if you start displaying non-alphanumeric characters. :) Anyway, once Mixxx officially has 4-deck support (i.e. in the skins,) then I will adjust the SCS.3d preset to handle it too.

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

Is this one still happening with 2.1 alpha?

RJ Skerry-Ryan (rryan)
Changed in mixxx:
status: Confirmed → Incomplete
milestone: 1.11.1 → none
Revision history for this message
Launchpad Janitor (janitor) wrote :

[Expired for Mixxx because there has been no activity for 60 days.]

Changed in mixxx:
status: Incomplete → Expired
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/6931

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.