Skip to content
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

updates for the "Korg Kaoss DJ" controller script #12683

Merged
merged 25 commits into from
May 24, 2024
Merged

updates for the "Korg Kaoss DJ" controller script #12683

merged 25 commits into from
May 24, 2024

Conversation

daschuer
Copy link
Member

New assignments:

browser Knob browse library or playlist up & down (same as before)
shift + browser Knob toggle focus between Playlist and File-Browser
tap open folder
2 x tap close folder
(left) shift + tap tap bpm of left track
(right) shift + tap tap bpm of right track
load A / B load track to deck 1 / 2 (same as before)
shift + load A / B open / close folder in file-browser
shift + touchpad X control super knob of QuickEffectRack for deck 1
shift + touchpad Y control super knob of QuickEffectRack for deck 2

changed assignments

  • the loop-on button now works as follows:
    • if a loop is active, it deactivates the loop (same as before)
    • if no loop is active, a new beatloop is created at the current playback position
      (before a re-loop of the deactivated loop was triggered)

doc-updates:

mixxxdj/manual#430

@daschuer daschuer changed the base branch from main to 2.4 January 29, 2024 09:11
@daschuer
Copy link
Member Author

@pi43r Please Test and share your results. Do you have interest to take over?

@pi43r
Copy link
Contributor

pi43r commented Jan 30, 2024

Thanks for opening this again!
I can try it on my controller but it might take some time. There also seems to be new interest in the forum.

I would need to get familiar with scripting for mixxx but I could definitely try updating the docs if I find anything.

@Swiftb0y
Copy link
Member

@daschuer for me as a reviewer: are you trying to just get this merged or do you want to receive suggestions on potential improvements before merging?

@daschuer
Copy link
Member Author

I have no clue about the code here. This PR is just to save the commits from oblivion and that someone else can easily take over.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

mergeable otherwise. thank you.

res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
Comment on lines +299 to +306
const now = new Date();
const timeSinceLastTap = now - KAOSSDJ.lastTapDate;
KAOSSDJ.lastTapDate = now;
if ((timeSinceLastTap < 600) && (timeSinceLastTap > 0)) {
engine.setValue("[Library]", "MoveLeft", true);
} else {
engine.setValue("[Library]", "MoveRight", true);
}
Copy link
Member

Choose a reason for hiding this comment

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

maybe GoToItem + Back would be better?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have no "Back" AFAIK. I leave this for one who can test this.

Copy link
Member

Choose a reason for hiding this comment

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

Not really, but we have [Library], MoveFocusBackward which is commonly mapped I think.

@Swiftb0y
Copy link
Member

I have no clue about the code here. This PR is just to save the commits from oblivion and that someone else can easily take over.

Good to know. I doubt anyone else would be interested, so lets fix the easy things and get this merged.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

thank you. almost lgtm. Just fix the typo and the fxGroup string above (otherwise it will not get interpreted correctly and KAOSSDJ.fxTouch will be broken).

res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
@daschuer
Copy link
Member Author

daschuer commented Feb 4, 2024

Thank you for review. I have amended the isFX issue. I hope that is OK here.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 4, 2024

yup sure. only remaining issue is the unresolved fxGroup issue from my previous review.

Co-authored-by: Swiftb0y <[email protected]>
@daschuer
Copy link
Member Author

daschuer commented Feb 4, 2024

Done.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

good job to eslint for catching this one.

res/controllers/Korg-KAOSS-DJ.midi.xml Show resolved Hide resolved
res/controllers/Korg-KAOSS-DJ.midi.xml Show resolved Hide resolved
res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
daschuer and others added 2 commits February 4, 2024 14:36
Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

LGTM. At least this shouldn't have worsened the current version. Lets just merge and see if users notice anything.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 4, 2024

@daschuer are you also volunteering to adopt the corresponding manual PR? mixxxdj/manual#430

@daschuer
Copy link
Member Author

daschuer commented Feb 4, 2024

No sorry, that needs to be really done by one owning such controller. I think also some testing would be not hurt here.

@Swiftb0y
Copy link
Member

Swiftb0y commented Feb 4, 2024

@raphaelquast do you still have access to the controller and can test the current version of the mapping? Do you have time to finish the manual page?

@raphaelquast
Copy link
Contributor

@Swiftb0y
I do still own the controller but i'm very limited in time at the moment... (and it seems like it's going to be like this for a while)
If a functionality test from someone owning the controller is the only thing that's blocking this from merging then I can try to find some time to have a quick look... but unfortunately I'm not able to actively contribute in the near future.

@pi43r
Copy link
Contributor

pi43r commented Feb 12, 2024

I can confirm operation of the new script in Mixxx 4.0.16 on Fedora39.
I couldn't test on MacOs because I'm not able to install the midi driver on my M2 macbook.
Will edit this for Windows when I get to it.

My findings, most of them pretty minor:

  • Moving between levels using Knob [4] + shift needs retriggering of shift after every change
  • Display is off. (Should indicate FX number or param / key or scale. I don't recall this ever to be the case in the past)
  • Program Knob [7] does nothing? (supposed to select effect or change key, might be talking about the internal kaoss pad)
  • Touchpad [10] does nothing. It should change the super FX knobs. (works with the old script)
  • Shift + Play = Key lock not working (doesn't work in old script either)
  • Shift + Sync = Cancels Sync does not work. Pressing sync cancels it. (Maybe it's just an issue with the docs?)

So the main issue is the center touchpad functionality.

@Swiftb0y
Copy link
Member

Hi @pi43r. Thank you for taking the time to test this mapping.

Mixxx 4.0.16

Where did you get that version number from, that doesn't look right.

Moving between levels using Knob [4] + shift needs retriggering of shift after every change

Can you elaborate on what you mean by "retriggering of shift"? What behavior do you expect as working normally? how does it behave instead currently?

Display is off. (Should indicate FX number or param / key or scale. I don't recall this ever to be the case in the past)

Yeah, I don't think the original mapper bothered to try to understand how that works. Someone with access to the hardware and some basic reverse-engineering skills would have to figure that out.

Program Knob [7] does nothing? (supposed to select effect or change key, might be talking about the internal kaoss pad)

Thank you for your testing. I'll try to see what I can fix remotely. In the meantime, I see you have already opened mixxxdj/manual#613. Would you be interested in incorporating the changes made here into that PR as well?

@Swiftb0y
Copy link
Member

Shift + Play = Key lock not working (doesn't work in old script either)
Shift + Sync = Cancels Sync does not work. Pressing sync cancels it. (Maybe it's just an issue with the docs?)

Those currently seem to be unimplemented for the sake of simplicity. Do you think they're necessary or can we just cut that feature from the docs?

res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
res/controllers/Korg-KAOSS-DJ-scripts.js Outdated Show resolved Hide resolved
@pi43r
Copy link
Contributor

pi43r commented Feb 12, 2024

Mixxx 4.0.16
Where did you get that version number from, that doesn't look right.

Copied it from human memory :) It should be 2.4.0 (0.16.beta.20240209gitd112e4c.fc39)
https://koji.rpmfusion.org/koji/buildinfo?buildID=28064

Moving between levels using Knob [4] + shift needs retriggering of shift after every change

Can you elaborate on what you mean by "retriggering of shift"? What behavior do you expect as working normally? how does it behave instead currently?

Holding shift + knob 4 changes from Browser to Search to the Folder window. This change only happens once. I have to release shift and press it again in order to highlight the next "level". I would expect it to cycle through them as long as it shift is pressed.

Those currently seem to be unimplemented for the sake of simplicity. Do you think they're necessary or can we just cut that feature from the docs?

Works well without it. Key lock would be nice though

Would you be interested in incorporating the changes made here into that PR as well?

Sure, I can edit the manual

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@Swiftb0y
Copy link
Member

I partly wrote this PR @daschuer so I'll let you decide whether to merge.

@daschuer
Copy link
Member Author

Thank you. I see no reason to hold that back.

@daschuer daschuer merged commit 40fa8dd into 2.4 May 24, 2024
25 checks passed
@Swiftb0y Swiftb0y deleted the korg-kaoss-dj branch May 24, 2024 08:13
@raphaelquast
Copy link
Contributor

@ALL really nice to see that the changes for to the Korg Kaoss DJ script have made it to the finish line! Thanks for following up on my initial PR (and sorry for not being able to finish it myself in reasonable time)!

@daschuer
Copy link
Member Author

@pi43r I have noticed we have not ask you for the formal confirmation to distribute your work with Mixxx.
Can you please sign https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ and comment here when done?
Thank you.

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

Successfully merging this pull request may close these issues.

6 participants