-
-
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
NI Traktor Kontrol S2 Mk1: add controller mapping #3905
Conversation
As a First-time contributor we need your consent by formally signing the contributor agreement: |
I'm confused... I thought this wasn't an HID device?? Can you post the output of |
It is contains HID device according to Output of lsusb -v
|
Ah, I think I was confusing this with the S4 Mk1. I think the S2 Mk1 may have been one of the first controllers where NI switched from a Vendor Defined Class for a Bulk endpoint to HID. |
The manual states that it uses a proprietary protocol named NHL. |
Ah, that explains my confusion. The manufacturer's documentation is incorrect. |
Let's not delay 2.3.1 further for PRs that haven't started review and get to work reviewing the backlog of mapping PRs for 2.3.2. |
This PR is marked as stale because it has been open 90 days with no activity. |
Who is able and has interest to review this? |
Does this need to be tested? I have a S4 MK1 available. |
I'll give this a go and report back with results |
I've been using this setup with the S2 "Mk1" for two days now without any issues I can attribute to the mappings themselves. I'm running Mixxx 2.3.2 on a Raspberry Pi 4 |
@leifhelm I would like to start with the code review of the S2 Mk1 HID mapping. But before I start, I would like to hear, if you are still available to address findings of the code review? |
Yes :) |
I do not like that I hard-coded the thresholds for the jog wheels. I had to adjust both values to get reliable jog wheel touch to work. Maybe it is more nice to measure the value when not pressed at startup. This is also not that nice. I do not know how Traktor solves this problem either. |
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.
Here are some findings. Either fix them, or explain, why you think it's correct as it is.
MessageShort.addControl("[EffectRack1_EffectUnit1]", "group_[Channel2]_enable", 0x0A, "B", 0x08); | ||
MessageShort.addControl("[EffectRack1_EffectUnit2]", "group_[Channel2]_enable", 0x0A, "B", 0x04); | ||
|
||
MessageShort.addControl("[Master]", "maximize_library", 0x0E, "B", 0x08, false, this.toggleButton); |
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.
Which button is mapped here?
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.
The browse encoder press
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.
Please add a comment in the code, this is not self-explaining
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 might also try
if (shift) {
engine.setValue("[Library]", "sort_column_toggle", 0);
} else {
engine.setValue("[Library]", "MoveFocusForward", 1);
}
instead of maximize_library. This would allow to navigate through the library, and as shifted operation allows to sort by selected library column.
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 tried it but it does not work. It just adds a marker around the left or the right window. Nothing more. Should you be able to navigate the left window?
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 use "[Playlist]", "SelectTrackKnob"
. Maybe I should be using something like [Library]
MoveVertical
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.
Try something like
if (shift) {
engine.setValue("[Library]", "MoveHorizontal", delta);
} else {
engine.setValue("[Library]", "MoveVertical", delta);
}
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 cannot figure out how to do what I want: Move to the next/prev song without shift and browse the sidebar with shift. There is no way to set a focus, just to set it to the next window 😞.
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 guess this feature requires 2.4 work. Here this code snippets work with 2.4. Maybe you can provide a follow-up PR for 2.4 once this is merged.
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.
Yes [Library] focus_widget
is what i am looking for.
b595d9e
to
8cdd254
Compare
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.
LGTM
Please address the open points in mixxxdj/manual#412 too.
|
@leifhelm Thank you for your contribution and your patience! |
This adds a controller mapping for the Traktor Kontrol S2 Mk1. It is based on the mapping from the Traktor Kontrol S2 Mk2.
Things to do before merge:
Please give feedback especially if something does not work (as expected).