Traktor S2 Mk3: Change jogwheel to use scratch2#15792
Traktor S2 Mk3: Change jogwheel to use scratch2#15792BoredGuy1 wants to merge 5 commits intomixxxdj:2.6from
Conversation
|
Thanks for this PR. It seems the Traktor Mk3 models all share the same high-res wheels, so IMO it would make sense to at least test the wheel code of the S4 Mk3 -- that uses edit at least they seem to have the same high-res timer. |
|
Hi ronso0, I am not sure why, but with the current Also, what is the difference between |
Okay, great. Though it's not The difference between |
Jep, that is wrong. Will fix it. IIRC that was a scratch/jog control long ago. |
|
OK, so I think there actually is a difference between the S4's wheels and the S2's wheels. The S4's wheels are motorized, so they are always sending HID packets with updated wheel position data. On the other hand, the S2's wheels are not, so it will stop sending packets when the wheels are stopped. This leads to a strange situation where I have explored workarounds to this, such as creating my own timer loop to decay the velocity without having to receive HID packets. But this introduces other problems, such as decaying the velocity in the middle of a scratch. I haven't been able to get In my opinion, we should stick with the EDIT: Actually, hold that thought. There's one more thing I haven't tried yet. |
|
I suddenly remembered that Anyway, this version uses I suspect it might have something to do with the engine itself, which is giving me an extra buffer's worth of audio in the wrong direction. If that's the case, I'm not sure how I would address this script-side, or if that's even possible. In any case, we should probably get some others with S2Mk3s to test this. |
|
Pre-commit is complaining about |
|
So how does it work/feel now, with #15845 merged? |
|
Works pretty well! The jogwheel is a lot more responsive compared to the stock script, especially for complicated scratching tricks. |
|
Nice. |
ywwg
left a comment
There was a problem hiding this comment.
thank you for this. I added a few notes and questions
| // Otherwise, check again after a while | ||
| } else { | ||
| engine.scratchDisable(deckNumber); | ||
| TraktorS2MK3.jogStopTimerId[deckNumber - 1] = engine.beginTimer(JOGWHEEL_STOP_POLL_TIME, () => TraktorS2MK3.jogStopper(field), true); |
There was a problem hiding this comment.
should we check to see if there is already an active timer (for whatever reason) and cancel it before creating the new one?
There was a problem hiding this comment.
Maybe? The way the jogTouchHandler works is that when the jogwheel is touched, the stop timer is cancelled, then when the jogwheel is released, the stop timer begins.
In order to physically release the jogwheel, it must be touched first, and therefore any existing timers should already be cancelled. So I didn't think it was necessary.
I might have missed some edge cases though.
| if ((TraktorS2MK3.jogDecayTimerId[deckNumber - 1] !== null) && | ||
| (TraktorS2MK3.jogDecayTimerId[deckNumber - 1] !== undefined)) { | ||
| engine.stopTimer(TraktorS2MK3.jogDecayTimerId[deckNumber - 1]); | ||
| TraktorS2MK3.jogDecayTimerId[deckNumber - 1] = null; | ||
| } |
There was a problem hiding this comment.
this is the second time this code appears, better to factor it out to a function that takes the deck number as a parameter
There was a problem hiding this comment.
Do you mean rewriting the function to take a deckNumber parameter rather than a field parameter?
I think that would work for jogStopper and jogDecayer, since those are only dependent on the deck number (only field.group), but not jogTouchHandler and jogHandler, since those are dependent on both the deck number and the value (that is, field.group and field.value).
There was a problem hiding this comment.
fair -- refactor however makes the most sense to you! but stopping the timer seems like a useful helper function.
There was a problem hiding this comment.
By the way, just as a heads-up - I'm currently abroad right now and don't have my equipment. I probably won't be making any big commits until I'm back home in June, unless someone else has a Traktor S2Mk3 and is willing to test my commits.
|
|
||
| // Affects how sensitive jogging/nudging (turning the wheel without touching the top) is. | ||
| // A constant of 0.5 makes jogging/nudging roughly equivalent to scratching. | ||
| const JOG_SENSITIVITY = 0.25; |
There was a problem hiding this comment.
do we want to expose any of these constants as controller preference values?
There was a problem hiding this comment.
Technically they can all be configured as preferences, although TICKS_PER_REV, JOGWHEEL_CLOCK_HZ, and TARGET_RPM probably shouldn't be changed.
Would you like me to move all the config options that make sense (JOG_SENSITIVITY, JOGWHEEL_STOP_POLL_TIME, JOGWHEEL_DECAY_POLL_TIME, JOGWHEEL_ALPHA, and JOGWHEEL_EPSILON) to the top? That might be more user-friendly.
I can also take the time to alter the comments if needed. They make sense to me, but they might not make sense to everyone.
There was a problem hiding this comment.
I want to expose the minimum to the user.... now that I go through them, JOG_SENSITIVITY is the only one that I think people might want to tweak.
There was a problem hiding this comment.
JOG_SENSITIVITY is definitely important.
JOGWHEEL_ALPHA and JOGWHEEL_EPSILON are maybe a little important? JOGWHEEL_ALPHA controls the smoothing factor ("slipperiness"), and JOGWHEEL_EPSILON controls the responsiveness when changing directions.
JOGWHEEL_DECAY_POLL_TIME and JOGWHEEL_STOP_POLL_TIME... yeah, probably not so important. Right now they're already set to the fastest (20ms), and you can directly control decay speed or stop sensitivity via JOGWHEEL_ALPHA and JOGWHEEL_EPSILON respectively.
There was a problem hiding this comment.
We can put these three (JOG_SENSITIVITY, JOGWHEEL_ALPHA, and JOGWHEEL_EPSILON) in the controller settings as long as you write good descriptions of them so users will understand whether they might want to change them and how.
Relevant forum post
This PR makes two minor improvements to the jogwheels of the Traktor S2 Mk3 script.
A single revolution on the physical controller used to result in roughly 2/3 of a revolution on the Mixxx virtual jogwheel. This is because the old script assumed one revolution was 1024 ticks, and passed that to engine.scratchEnable. Based on my personal tests, I have changed it to 648 ticks per revolution. This produces a much closer result, although this value might have to be calibrated from controller to controller to be 100% accurate.
The old script used to discard one of the four bytes sent from the jogwheel, because no one knew what that byte meant. I was able to decode it. The first 2 bits are an extension of the jogwheel position, while the last 6 bits are an extension of the 100 kHz counter/timer. So now, the first 10 bits (instead of the first 8 bits) represent the wheel position and are used to calculate tickDelta, while the last 22 bits (instead of the last 16 bits) represent the counter and are used to calculate timeDeltas. are a number that increments and overflows at 100 kHz. The 2 bits were the MSBs of the wheel position while the 6 bits were the LSBs of the counter. So while the precision of the wheel position measurement didn't change much, the precision of the timer improved drastically. In theory, this means jogwheel bending (turning the jogwheel without touching the top) is now 64 times more precise, although in practice it was already quite precise to begin with.
Edit: This PR now overhauls the entire scratching part of the S2MK3 script by using the high-res timer in the jogwheel to calculate velocity, which is then directly applied with
scratch2instead ofscratchTicks. The title has been updated accordingly to reflect this.