Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Numark MixTrack Pro 3 errors on startup #12369

Closed
eiglow opened this issue Nov 27, 2023 · 15 comments
Closed

Numark MixTrack Pro 3 errors on startup #12369

eiglow opened this issue Nov 27, 2023 · 15 comments

Comments

@eiglow
Copy link

eiglow commented Nov 27, 2023

Bug Description

Every time I start Mixxx, I get 2 errors:

Uncaught exception: file:///usr/share/mixxx/controllers/Numark-Mixtrack-3-scripts.js:375: TypeError: Property 'flashOnceOff' of object [object Object] is not a function
Backtrace: @file:///usr/share/mixxx/controllers/Numark-Mixtrack-3-scripts.js:375
Uncaught exception: file:///usr/share/mixxx/controllers/Numark-Mixtrack-3-scripts.js:316: TypeError: Property 'flashOnceOn' of object [object Object] is not a function
Backtrace: @file:///usr/share/mixxx/controllers/Numark-Mixtrack-3-scripts.js:316

It seems to mostly work fine if I just ignore these errors however.

Version

2.4.0-beta

OS

Fedora 40 (rawhide)

@eiglow eiglow added the bug label Nov 27, 2023
@ronso0
Copy link
Member

ronso0 commented Nov 27, 2023

Fedora 39 has just been released, Fedora 40 is still alpha.
Please try with Fedora 39.

Both timer calls seem to use the correct syntax (after #11953)

this.flashTimer = engine.beginTimer(num_ms_on + num_ms_off, function() {
this.flashOnceOn(false);
});

this.flashOnceTimer = engine.beginTimer(this.num_ms_on - scriptpause, function() {
this.flashOnceOff(relight);
}, true);

I don't see who is throwing that error TypeError: Property [..] of object [..] is not a function.
Which Mixxx version exactly are you using? See About dialog for details.

Maybe this is caused by a newer QScriptEngine (either buggy or more strict checks?), idk.
I think @Swiftb0y or @uklotzde can shed a light on this.

@Swiftb0y
Copy link
Member

I'm pretty sure this is unrelated to the fedora version.

there may be an issue with the rebinding of this in the handler. can you try changing the code to this?

var led = this;
this.flashTimer = engine.beginTimer(num_ms_on + num_ms_off, function() { 
    led.flashOnceOn(false); 
}); 

Do you get any errors when you enable the mapping? Something could go wrong while its initializes itself and the error you experience is just a symptom of that.

@eiglow
Copy link
Author

eiglow commented Nov 28, 2023

Fedora 39 has just been released, Fedora 40 is still alpha. Please try with Fedora 39.

I experienced the same issue on Fedora 39.

Which Mixxx version exactly are you using? See About dialog for details.

The info page didn't tell me anything extra, the git version was unkown. However, based on the package version, it appears to be built from 3223a75.

there may be an issue with the rebinding of this in the handler. can you try changing the code to this?

Your suggested change seems to avoid the line 316 error, but the line 375 error still occurs as I would expect.

Do you get any errors when you enable the mapping? Something could go wrong while its initializes itself and the error you experience is just a symptom of that.

image

This is how the errors present. The snippets in the issue are copy-pasted from the "Show Details..." section.

If I click "Retry", the error just re-appears. This is the same for both errors.

If I click "Ignore", it dismisses the dialog as expected.

@ronso0
Copy link
Member

ronso0 commented Nov 28, 2023

Your suggested change seems to avoid the line 316 error, but the line 375 error still occurs as I would expect.

Okay, great!
You need to adopt the fix provided by @Swiftb0y for the function around line 375.
If this works out well, too, it would be great if you could open a PR targetting the 2.4 branch of the mixxx repo.
Instructions how to contribute are here https://github.com/mixxxdj/mixxx/blob/main/CONTRIBUTING.md

@Swiftb0y
Copy link
Member

You need to adopt the fix provided by @Swiftb0y for the function around line 375.

This is a viable workaround for this mapping, but a symptom of a bigger issue. I'm afraid there is a regression from QJSEngine switch that the function passed to engine.beginTimer is not getting its this properly rebound.
I'll try to look into it tonight.

@Swiftb0y
Copy link
Member

Yup, it just straight up calls the function without rebinding. This probably only worked in the first place because of some QScriptEngine quirk, or it got lost in the transition.

}
m_pScriptEngineLegacy->executeFunction(timerTarget.callback);

@Swiftb0y
Copy link
Member

Swiftb0y commented Nov 28, 2023

Umm, so I don't think this is properly fixable. In the docs, we say When the function is executed this is set to whatever this was where engine.beginTimer was called. But in order for that to work we need to be able to access that this object in the call to engine.beginTimer so we can then call it via QJSValue::callWithInstance. As I aside, I don't think there is any way to access the calling context in the JS engine, right? From what I could tell, its not possible to retrieve that this from C++. We can only fix this in javascript! I guess some mass-find-and-replace is in order. Any better ideas are welcome.

@Swiftb0y
Copy link
Member

@eiglow Just to be sure, can you also test the mapping with the changes I made here:
Swiftb0y@7da28fa
You don't need to compile a new mixxx version, I only made changes to the mapping. Please also check whether all the functionality related to long-pressing buttons still works.

@eiglow
Copy link
Author

eiglow commented Dec 1, 2023

@eiglow Just to be sure, can you also test the mapping with the changes I made here: Swiftb0y@7da28fa You don't need to compile a new mixxx version, I only made changes to the mapping. Please also check whether all the functionality related to long-pressing buttons still works.

While testing, I encountered this error when long-pressing the sync button:

Uncaught exception: file:///usr/share/mixxx/controllers/Numark-Mixtrack-3-scripts.js:550: TypeError: Property 'buttonDecide' of object [object Object] is not a function
Backtrace: @file:///usr/share/mixxx/controllers/Numark-Mixtrack-3-scripts.js:550

I was still able to reproduce this without the above changes though, so I don't think it's caused by those changes, I just didn't spot it before.

Should I create a separate issue for this error?

@Swiftb0y
Copy link
Member

Swiftb0y commented Dec 1, 2023

Should I create a separate issue for this error?

No need, we can try to fix this here too. I prepared another commit with some debug logging. Can you please try that and then post the log messages from when you long-press the sync button? Swiftb0y@00157b5
I'm a little rusty in my JS, so this might take a couple attempts.

@ronso0
Copy link
Member

ronso0 commented Dec 4, 2023

Is this a blocker?

@Swiftb0y
Copy link
Member

Swiftb0y commented Dec 5, 2023

depending on the mapping, possibly. Can't say for sure. First step would probably be to update the wiki. I'll take care of that.

@ronso0
Copy link
Member

ronso0 commented Dec 5, 2023

The changes LGTM, but I'm no js wizard.
A lot of unrelated improvements, like links to the manual. Thank you!

@Swiftb0y
Copy link
Member

@eiglow we fixed this issue for all mappings including this one. Can you do us a favor and try the newest 2.4 version and see if the error still occurs? Also, since this essentially fixed, I'd like to close it. If you need any further assistance in fixing the Syncbutton you mentioned previously (#12369 (comment)) please open a new issue. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants