Skip to content

S4MK3: add support for Stem, rework hotcues and add beatjumps#703

Merged
ronso0 merged 2 commits intomixxxdj:2.6from
acolombier:feat/s4-mk3-screens
Feb 23, 2026
Merged

S4MK3: add support for Stem, rework hotcues and add beatjumps#703
ronso0 merged 2 commits intomixxxdj:2.6from
acolombier:feat/s4-mk3-screens

Conversation

@acolombier
Copy link
Copy Markdown
Member

@acolombier acolombier commented Dec 6, 2024

@acolombier
Copy link
Copy Markdown
Member Author

@ywwg I would really appreciate a review if you can! :)

Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
2. **Grid**: The track's detected beats will be move forward or backward on the waveform.
3. **BPM**: The track's detected BPM will be increased or decreased.
4. **Keyboard**: The keyboard's keys displayed on pads get moved up or down to display higher or lower keynotes.
5. **Hotcue color**: When selecting a hotcue, the move encoder can be used to change the selected color.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the hotcue button is pressed?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it's explained below..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would be better to explain the modes first then refer to them in each component section.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting to swap the list and table


============================================================================================== =========================================== ================================================================================================================= ===================================================================================== ================================================================================================================================================================================================================
============================================================================================== =========================================== ================================================================================================================= ===================================================================================== ======================================================================================================================================================================================================================================
Setting Variable value Default Range Description
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My experience with this table:

  • the content is very wide and I have to scroll horizontally to check the description for example
  • I don't need the Variable value column. If I'd be interested, I'd look at the settings in the js file
    -> this would reduce the table width
  • in the Default column we may remove the the prefix (LEDColors etc.) and just show the values
    -> this would reduce the table width

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since 2.4 is EOL, I'm tempted to remove a lot actually, since with the SettingsAPI, you shouldn't have to edit the file, except if you are hacking around. Wdyt?

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Mar 10, 2025

Wanted to take look but Netlify fails with

1:38:04 AM: Failed during stage 'Install dependencies': dependency_installation script returned non-zero exit code: 1
1:38:04 AM: mise python@3.*      install
1:38:04 AM: mise WARN  no precompiled python found for 3.*, force mise to use a precompiled version with `mise settings set python.compile false`
1:38:04 AM: mise python@3.*      python-build
1:38:04 AM: python-build: definition not found: 3.*

@acolombier acolombier force-pushed the feat/s4-mk3-screens branch from 0a13166 to 4551cbc Compare March 10, 2025 13:40
@acolombier
Copy link
Copy Markdown
Member Author

Fixed @ronso0 - my branch was quite out of date it turns out

@acolombier acolombier changed the title S4MK3: add support for screens S4MK3: add support for Stem, rework hotcues and add betajump Aug 18, 2025
@acolombier acolombier changed the title S4MK3: add support for Stem, rework hotcues and add betajump S4MK3: add support for Stem, rework hotcues and add beatjumps Aug 18, 2025
@acolombier acolombier changed the base branch from main to 2.6 August 18, 2025 17:10
@acolombier acolombier requested review from ronso0 and ywwg August 18, 2025 17:11
@acolombier
Copy link
Copy Markdown
Member Author

I have rebased and reword that PR to exclude the screen bits (4c9c600), which I will add in a follow up PR against 2.7. Does that sound all good?

Copy link
Copy Markdown
Member

@ywwg ywwg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment only

@ywwg
Copy link
Copy Markdown
Member

ywwg commented Aug 21, 2025

(can't find a way to just dismiss a change request)

@acolombier
Copy link
Copy Markdown
Member Author

(can't find a way to just dismiss a change request)

For the future, it's hidden at the bottom, near the merge button :)

image

[..] address this a separate issue?

Do you want to create an issue and perhaps capture how you would like it to render? Or do you have time/interest in looking into this?

@ywwg
Copy link
Copy Markdown
Member

ywwg commented Aug 22, 2025

Mostly I just want a narrative howto for how to work with effects like you have for the motor and jogwheel section.

@ywwg
Copy link
Copy Markdown
Member

ywwg commented Aug 22, 2025

#788

Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
@acolombier acolombier requested a review from ronso0 September 10, 2025 23:03
@SamWhited
Copy link
Copy Markdown
Contributor

The screens use a USB Bulk transfer. Mixxx doesn’t support rendering content for external screens yet

Still says this in the intro section, but I think this PR is for post screen support.

@acolombier
Copy link
Copy Markdown
Member Author

Good catch! This should indeed have been moved in the other manual PR.

I have taken the freedom to squash the fixup already, so if everything is looking fine now, we can merge it as the mapping changes have been merged already!

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 14, 2025

Are the changes other than the rst file relevant for this PR, or did they slip in by accident during rebase?

The netlify preview is outdated and the HTML runner skipped -- because of these changes?

@acolombier
Copy link
Copy Markdown
Member Author

Yeah, 93eee16 is not related to this PR directly. I can move it to another PR if you would prefer!

@acolombier
Copy link
Copy Markdown
Member Author

The netlify preview is outdated and the HTML runner skipped -- because of these changes?

Spent sometimes looking into this - the HTML job is expected to be skipped, and by the look of it, the Netlify deploy is orchestrated remotely, likely with a webhook, but I don't have access to this repo's settings, or the Netlify project.

@mixxxdj/admins can you have a look or grant me the permission to do so?

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 14, 2025

Yeah, 93eee16 is not related to this PR directly. I can move it to another PR if you would prefer!

Yup, that'd be better.
Else this LGTM and ready for merge.

Thanks for looking into the netlify issue!

@ywwg
Copy link
Copy Markdown
Member

ywwg commented Sep 15, 2025

I should have time to do one last test today and I promise I will only look at the specific functionality being added :)

@ywwg
Copy link
Copy Markdown
Member

ywwg commented Sep 22, 2025

Sorry for the delay. I hope to get to this soon

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 22, 2025

This mapping is out in 2.6beta and the docu doesn't match anymore.
Let's just merge and fix potential issues as well as improve formatting/fx handling in a followup?

+--------------+-----------------------------------------------------------------------------+
| Keyboard | This mode is enabled while :hwlabel:`SHIFT` + :hwlabel:`STEM` is held down |
+--------------+-----------------------------------------------------------------------------+
| Hotcue color | This mode is enabled while :hwlabel:`HOTCUES` is held down |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this functionality should be merged in, right? This is not working for me.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I am using 2.6)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Press both HOTCUE and a hotcue, then turn left encoder.
Doesn't work?

Hotcue color controls have been added in 2.4 or even earlier so this should work.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, nothing happens. I'll triple-check that my S4 is using the right mapping.

Copy link
Copy Markdown
Member

@ronso0 ronso0 Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. I can test the mapping (mixxxdj/mixxx#15216) tomorrow.

Btw it would be cool if we could use the last used hotcue, instead of having to press two buttons.

Copy link
Copy Markdown
Member

@ywwg ywwg Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just setting the selected pad value regardless of mode would at least fix the immediate discoverability issue and wouldn't introduce any side effects

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that would work as it will have a side effect with the left encoder itself - if you go with this approach, when you would use it, it would change the color of the last triggered hotcue. Equally, it would require to trigger the hotcue which would impact the playback (currently you can edit the color why being live)

Copy link
Copy Markdown
Member

@ywwg ywwg Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could clear the selected hotcue button value when the hotcue selector button is released.

at the very least, let's clarify the docs to say that the user has to hold the hotcue button down for a period of time before pressing the hotcue pad.

Copy link
Copy Markdown
Member

@ywwg ywwg Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to tie this edit mode to long press? it looks like short press doesn't do anything other than change the screen. Changing the page only triggers on short release.

Pick whatever solution you like, but for now, let's at least make the docs more clear so that others don't trip over the same issue.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we could use short press too tbh.

Do you have any rewording suggestions to make the current instructions clearer? I can try to update when I'm back otherwise.

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 24, 2025

I don't find info on how to focus an effect.
(it's Shift + long-press effect)

@acolombier
Copy link
Copy Markdown
Member Author

acolombier commented Sep 24, 2025

I don't find info on how to focus an effect. (it's Shift + long-press effect)

Ah good catch! I'm going to be away from keyboard for the next few days, so feel free to push a change if you would want to unblock the 2.6 out of sync doc, otherwise I will have a look when I get back, somewhere in the first half of October

Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 27, 2025

I have this commit 162b5e3 / acolombier#1 to simplify the fx section a bit, ie. remove redundancy.

Btw these tables are a PITA to edit, wasn't there a more user-friendly way?

@SamWhited
Copy link
Copy Markdown
Contributor

Btw these tables are a PITA to edit, wasn't there a more user-friendly way?

I always use CSV Tables, ugly, but way easier to edit.

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 27, 2025

Yeah, I just discovered that, too, but with csv there's no row-span/col-span.

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Feb 22, 2026

@acolombier Do you plan to proceed here? Looks like there are no conflicts yet, so please check the review proposals and my PR. I think we can merge this quickly and clean up / optimize further in follow-ups.

@acolombier
Copy link
Copy Markdown
Member Author

Should be good now @ronso0 ! Waiting for your 👍 before squashing the fixup.

Copy link
Copy Markdown
Member

@ronso0 ronso0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a small adjustment to improve readability

Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
Comment thread source/hardware/controllers/native_instruments_traktor_kontrol_s4_mk3.rst Outdated
@acolombier
Copy link
Copy Markdown
Member Author

Addressed and squashed.

@acolombier acolombier requested a review from ronso0 February 23, 2026 18:35
@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Feb 23, 2026

Yeij, thank you!

@ronso0 ronso0 merged commit 857dd96 into mixxxdj:2.6 Feb 23, 2026
8 of 9 checks passed
@ronso0 ronso0 mentioned this pull request Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants