Skip to content

[WIP] Traktor Kontrol S4mk3: motors revisited#14872

Draft
jtMUMT wants to merge 48 commits intomixxxdj:mainfrom
jtMUMT:s4m3_motors_revisited
Draft

[WIP] Traktor Kontrol S4mk3: motors revisited#14872
jtMUMT wants to merge 48 commits intomixxxdj:mainfrom
jtMUMT:s4m3_motors_revisited

Conversation

@jtMUMT
Copy link
Copy Markdown

@jtMUMT jtMUMT commented May 30, 2025

Revising the motorized platter control on the Traktor S4Mk3 controller.

Current Status: motors are usable for mixing and work almost identically to using Traktor. Code needs major cleanup.

Impact on common code: removed the 20ms cap on QTimer requests so that the motors can be controlled at the correct sampling rate to match the sensor input stream (2ms / 500Hz).

Extraneous / "bonus" changes: added midpoint offset for tempo faders. Not implemented in the most graceful way but it works. Implemented upstream already

Known bugs: switching decks on the right side seems to change the pad mode / screw up the LED colours on that side. I suspect it has to do with how I'm assigning left/right status to each deck.

Apologies: for all the hacking and rearranging/renaming of existing declarations. This is very much a work in progress and the code needs to be revised before it's ready to merge (assuming it passes review of course). I have ambitions to improve the overall comprehensiveness of the mapping file by fixing hardcoded values and harmonizing some of the nomenclature. So far this has resulted in some improvements and also some unintended ambiguities (eg., I didn't realize that "slip" mode was already a thing before I used this to refer to the slipmat emulation). Right now this code is here so that others can try it out while I/we keep working on it :)

@jtMUMT
Copy link
Copy Markdown
Author

jtMUMT commented May 30, 2025

Correction: there is no bug with the pad LEDs. That was simply human error (I set the default pad mode by mistake)

@JoergAtGithub
Copy link
Copy Markdown
Member

Welcome at Mixxx!
As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

@acolombier
Copy link
Copy Markdown
Member

Hi @jtMUMT , thanks for looking into this. It looks like you based your work off an old version of the mapping. Could you please move you changes off the latest version in main?

@jtMUMT
Copy link
Copy Markdown
Author

jtMUMT commented May 30, 2025

Yup, I'll adapt it to a more recent build, no problem.

By the way, for anyone who wants to give it a go, here are the controller values (set in preferences) that work for me. I'll make these default in a future commit :)

Jogwheel Tension 0.5
Jogwheel Torque 60000
P gain 80000
I gain 500
D gain 50000
Slip Force 12000
Motor Friction Compensation 0

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented May 30, 2025

It looks like you based your work off an old version of the mapping. Could you please move you changes off the latest version in main?

I think there's no new stuff or fixes in main, and I consider this a fix actually, so please target 2.5.

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented May 30, 2025

Also, please install pre-commit (it found some issues, see https://github.com/mixxxdj/mixxx/actions/runs/15338571816/job/43219530783?pr=14872)
https://github.com/mixxxdj/mixxx/wiki/Using-Git#set-up-automatic-code-checking
and run it on the changed files. After rebase onto 2.5, I think this can be done with

FILES=$(git log --name-only --pretty="" mixxx/2.5..HEAD)
pre-commit run --files $FILES

or, if you have xargs
git log --name-only --pretty="" mixxx/2.5..HEAD | xargs pre-commit run --files

@acolombier
Copy link
Copy Markdown
Member

acolombier commented May 31, 2025

Note that another way to run pre-commit after pushing is the following:

pre-commit run --show-diff-on-failure --color=always --from-ref upstream/2.5 --to-ref HEAD

A few early nit notes before I start a deeper review:

  • It would be great if you could leave the change to a minimal and restore changes unrelated in this context (e.g moving colour constant around
  • Please stick to the existing naming convention. PascaleCase for constant, camelCase for setting key and other variable. Also, please use left and right instead of L and R

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented May 31, 2025

Okay, I just gave this a shot.
Engaging TT mode did nothing first. Idk, I think was torque which I increased first, then wheel started moving.
But they made noise. Like, ticking and humming sounds, couldn't hear a pattern with a track playing at steady tempo.

With these vaues I got it working somehow incl. scratching, but settings still don't feel optimal: after scratch release I have to push the wheels so they start moving again.

  • Jogwheel Torque: 40.000
  • Slip friction force: 4.400
  • Motor friction compensation: 2.300

(. is the thousands separator)

I guess I will finally have to install Traktor and check how it feels like there.
(I hope the wheels on my S4 are still okay, bought it second-hand)

@ronso0 ronso0 marked this pull request as draft May 31, 2025 23:09
Zeph T added 3 commits August 22, 2025 10:19
fixed a mistake during manual merge, addressed many pre-commit warnings and errors

Corrected error in the TempoFader calibration settings: missing unit conversion from mm to ticks. Added proper warning message for incorrect deck assignment

Requested: reverted Tempo Fader changes

typo: delete line of tabs

reverted comment in scriptinterfacelegacy.cpp

removed unused var oldWheelTimer
added dead zone during scratching to avoid 'chattering' motor problem

re-enabled scratch mode on TT mode activation: fixes issue where scrubbing was not possible using crown adjustment until disc was touched

fixed error when selecting TT mode; scratch mode will not be re-enabled if playback is already underway

Fixed: inability to scrub with crown while paused in motor mode (faulty conditional)

Fixed: selects proper rotation speed (more faulty conditionals). Tweak to default proportional error gain
@jtMUMT jtMUMT force-pushed the s4m3_motors_revisited branch from b14d0d6 to 22a8628 Compare August 22, 2025 03:38
Zeph T added 3 commits August 22, 2025 12:06
…o make my new code more in line with the existing base even if there are some historical deviations from the style guide that need fixing (those can be fixed in a future PR)
@github-actions github-actions Bot added library engine ui build code quality preferences waveform qml controller backend developer experience Issues, bugs and PRs related to the development process, development environment & developer docs labels Aug 28, 2025
@jtMUMT
Copy link
Copy Markdown
Author

jtMUMT commented Aug 28, 2025

  • I've squashed the recent commits to my branch so that they're more concise and clean.
  • Motor controller preferences are simplified.
  • Merged @acolombier 's S4Mk3 improvements branch into my PR to bring it up to date with 2.6*

*I tried rebasing but it would have necessitated 62 commits' worth of manual merges and I felt this would be too likely to introduce further errors.

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Oct 24, 2025

@jtMUMT sorry for the delay!
Unfortunately merge conflicts have evolved. I think these can easily be resolved by rebasing onto main.
(IMO better than merging the main branch).

Looks like you can squash (eg. the "experiments" commits) and also drop some commits (eg. 6d7a6ec).
I know this is probably a lot fo work but we'll get a clean commit history.

In case you don't have capacities to do this I can try it myself -- just to help push this awesome feature.
(been advertising the motors feature a lot lately, so I finally want to get it into my mapping, too : )

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Oct 25, 2025

In case you don't have capacities to do this I can try it myself

I'm at it and it's very ... Mostly because you moved unrelated defintions around.
Let's try to avoid that in the future, it just causes trouble, especially with huge feature PRs like this.
(had to learn that the hard way myself, too)

I ended up simply copying the mapping files from this PR and created a new branch based on 2.6
I split it into the HID report constants (small) and the motor changes.
I also removed redundant ..modes (newly added where PascalCase, but the old camelCase modes where still defined and used)
And made eslint happy I hope..

Please check the commits and split the second one if like.
ronso0#95

-- didn't do another manual test, yet --

@jtMUMT
Copy link
Copy Markdown
Author

jtMUMT commented Oct 25, 2025

I'm at it and it's very ... Mostly because you moved unrelated defintions around. Let's try to avoid that in the future, it just causes trouble, especially with huge feature PRs like this. (had to learn that the hard way myself, too)

Please check the commits and split the second one if like. ronso0#95

-- didn't do another manual test, yet --

Big thank you for cleaning up my mess. I'm still inexperienced with atomizing features and keeping a clean repo. Whatever it takes to get the feature integrated, even if we have to start a new branch/PR, is OK with me. I'll be able to get involved next week to catch up with your work and to conduct tests. Again, 1000x thanks for helping and advising <3

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Oct 29, 2025

I updated and tested ronso0#95
See indivdual commits there.

In general it now works well 🎉 , though there are still some issues:

  • the motor chattering is back. sometimes right away when I enable TT mode, but almost always after scratch release (in TT mode). script enters the playbackError > SlipmatErrorThresh branch but the deck shouldn't be slipping. My guess is that the slipping/scratch detection above needs to be fixed
  • the CUE "goto and stop" behavior with motors is not ideal. "goto and stop" should always make it stop there, but currently the motor keeps spinning and playpos comes to halt some time after the CUE. This is not good as now (stopped) we can't do Cue preview anymore (stuttering) because next Cue press would set it to the shifted position.
    I think since we're in some sort of hybrid space anyway (TT emulation + seek/jump with Cue, hotcues and stuff) we should try to stick to the expected Cue behavior and detach motors from scratching/jogging when pressing Cue. All Cue modes stop and goto on press. I'm not sure how todo it though. Btw I think we don't need it to be perfect (it's still marked Beta) but it should also not break use cases.

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Oct 30, 2025

the motor chattering is back. sometimes right away when I enable TT mode, but almost always after scratch release

I think I fixed that. edit: nope :\ looks like it's overshooting, and resetting isSlippingproperly

The cue/hotcue goto_and_stop/preview issue is a bit annoying.
I disabled that, but this also disables Vinyl stop which is actualy nice : )

Btw I think there should be some way to reset() the motor manager when switching to/from motor mode.
This would allow reset in case of issues like the chattering after slipmat mode.

@jtMUMT
Copy link
Copy Markdown
Author

jtMUMT commented Oct 31, 2025

Regarding the motor chattering, I encountered this same problem and fixed it before, but understandably it's easy to re-introduce it. There are certain boolean conditionals that require "==" instead of "===" or else they don't properly compare and always result in "False". I fear that the pre-commit might flag these and prompt us to use === but it's wrong. It was by trying to conform to this pre-commit that I ended up having this problem in a previous commit.

Line 3796:
} else if (this.deck.wheelTouch.touched === false && this.deck.isSlipping && Math.abs(playbackError) < SlipmatErrorThresh) {

This should use == instead.

There are a few instances of this, which I originally addressed in f5b1014 (Jogwheel TT mode fixes and improvements)

edit: tested with the conditional change and solves the problem :)

e2: for your convenience, the other affected lines are 363 (selecting RPM) and 3853 (detect spin-up to target) but please do check against the referenced commit

@jtMUMT
Copy link
Copy Markdown
Author

jtMUMT commented Oct 31, 2025

Yeah when I try to make the changes, the pre-commit is blocking me:

 363:30  error  Expected '===' and instead saw '=='  eqeqeq

3796:57  error  Expected '===' and instead saw '=='  eqeqeq

3853:42  error  Expected '===' and instead saw '=='  eqeqeq

I'm sure this is down to my own unfamiliarity with JS and these conditionals could be improved, but I'd love to know what exactly is happening. Seems like instance variable always return true even if they're booleans set to false

@jtMUMT
Copy link
Copy Markdown
Author

jtMUMT commented Oct 31, 2025

In any case, I made a branch with fixes for both issues:
https://github.com/jtMUMT/mixxx/tree/fix/s4-motors-chattering-and-cue

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Nov 1, 2025

Thanks!
I cherry-picked your commits to https://github.com/ronso0/mixxx/tree/s4-motors-rebased / ronso0#95.

No I'll test : )

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Nov 1, 2025

This works good.

  • added the same for hotcues.
  • moved the hard-stop trigger before triggering cue_gotoandstop.
  • had to increase MotorWindDownMilliseconds to 900 because even after the run-out, the wheel makes a tiny move backwards. This is independent from wheel tightness and torque as far as I can tell.

However, hard-stop didn't account for quick repetetive Cue/hotcue preview (stutter effect), so I tried to tweak the timers a bit.
This kinda works: with hotcue preview I end up at the hotcue 50/50, with Cue preview it's more like 15-20%.
-> how to check: in Jog mode, set Cue at hotcue and do preview with both
-> Cue should always be lit on release
I also managed to get the wheel spin backwards and accelerate to frightening speeds 😬
It does catch up / restore desired speed, but it proves I'm a bad js dev ; )
I guess you'll spot the culprit quickly, it's the last commit.


Also, minor issue:
the hard-stop guard only applies to controller interaction -- when triggering cue_default or ending hotcue preview with the keyboard or mouse, the wheel run-out/scratch issue is there.
Not sure if this a real issue though, like why use the mouse/keyboard while the controller is enabled?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants