-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Native Instruments Traktor Kontrol S2 MK3 mapping #2348
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor issues, otherwise looks good to me.
@Be-ing ping |
Thank you for helping out with the code review @Holzhaus. Reviewing the code is helpful, however my bigger concern with controller mappings is whether the UX design makes sense and works with how Mixxx is designed. This is why it is essential for a wiki page to be written before review. The contents of the wiki page can be compared with the code to verify that the code does what it is supposed to do (as much as can be done without the reviewer having the hardware). Also, the wiki page can be edited for language clarity/organization/style. This takes some work and so far I have been the only one doing this. I am overwhelmed keeping up with mappings and trying to make releases happen. If you could help review mappings for UX design, that would be really helpful! |
// Reset sync button timer if active | ||
if (TraktorS2MK3.syncPressedTimer[group] !== 0) { | ||
engine.stopTimer(TraktorS2MK3.syncPressedTimer[group]); | ||
TraktorS2MK3.syncPressedTimer[group] = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're resetting the timer, but you're not changing any control object. According to your wiki entry, the mapping is supposed to toggle sync_master
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As written in https://www.mixxx.org/wiki/doku.php/mixxxcontrols#channeln under sync_enabled
Mixxx distinguishes between pressing the SYNC button for a long period (< 1 sec) and a short press. I just imitate this behavior in my script, since Mixxx is not checking the duration when setting the control from a controller script. This is also mentioned in the Wiki page, but i made it more clear there now.
The timer is started when the button is pressed. When it is released the syncHandler()
function is checking, if the timer is still running. If this is true, i reset the sync control key. Otherwise i do nothing, so SYNC lock is enabled.
But you pointed to the right spot, there was some unnecessary code in syncHandler()
, because stopping the timer when it is already expired is useless. I fixed this in mi01@0ac8408
|
||
TraktorS2MK3.syncHandler = function (field) { | ||
if (TraktorS2MK3.shiftPressed[field.group]) { | ||
engine.setValue(field.group, "beatsync_phase", field.value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but the shift behaviour is not really documented. In the wiki entry, it sounds like this depends on quantize, not the shift state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot this in the Wiki page, updated it now.
It looks like the intro to the wiki page was copied and pasted:
Can you verify that this is all correct? Does this controller have a power supply separate from the USB power? |
I read through the wiki page and made some edits for clarity. Everything seems to be mapped in a sensible way with two small exceptions. You could map the sampler gain knob to control the gain knobs of all the samplers together. This is an ugly hack but it's the best we can do for now. In the future I want to implement a sampler output bus that would have its own gain knob. The way you have mapped the FX buttons in the middle of the mixer isn't all that useful without any other effects controls available on the controller. However, I don't know what else to suggest to do with them for now. Do you have any other ideas? With #1705 one of my goals is to implement something like Traktor's Mixer FX but even more powerful so you could save custom effect chains and assign them to the QuickEffect knobs. But don't count on that feature getting implemented soon. |
Indeed i copy and pasted this part of the wiki page from the predecessors page, but its correct, there is an optional power supply (15V/1.2A). But i fixed the typo now, mentioning the MK2 instead of MK3 there. You can have a look at the manual, if you like: https://www.native-instruments.com/fileadmin/ni_media/downloads/manuals/traktor/TRAKTOR_KONTROL_S2_MK3_Manual_English_1218.pdf
Yes, this seems to be the only way to give this knob a meaningful function. Added this in a7e475b and updated the wiki page.
I dont have any other idea regarding this buttons, but assigning effect units to the decks seems not completely useless to me. However it would be great to have a improved effect system in the future and i promise to update the mapping in this case. |
Please rebase this on the 2.2 branch so we can include it in the next minor release of Mixxx (2.2.4). |
Great work. I am glad we have a mapping for this controller soon after its release. Just rebase this on the 2.2 branch and then it will be ready to merge. |
13e7ef4
to
14e4a1b
Compare
There are still many unrelated commits from the master branch in this. Do an interactive rebase with |
Co-Authored-By: Jan Holthuis <[email protected]>
14e4a1b
to
1698b25
Compare
Sorry, accidentally just copied your command from above and rebased to the head of the master branch before. Hope this works now. Thank you for your great feedback and support! |
Thanks for your contribution and patience with the review. |
I moved this controller into the Community Supported Mappings list on the wiki |
Hello,
i created a USB-HID controller mapping for the Native Instruments Traktor Kontrol S2 MK3. I think its complete now, so i would like to contribute it officially here. There is a wiki page and a thread in the forums also with additional infos. I'm looking forward to get some feedback.