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

Add controller support for Pioneer DDJ-400 #2143

Closed
wants to merge 26 commits into from

Conversation

WarkerAnhaltRanger
Copy link

@WarkerAnhaltRanger WarkerAnhaltRanger commented Jun 5, 2019

This is a initial Support for the Pioneer DDJ-400
Testers are welcome!
Working:
* Mixer Section (Faders, EQ, Filter, Gain, Cue)
* Browsing and loading + Waveform zoom (shift)
* Jogwheels, Scratching, Bending
* cycle Temporange
* Beat Sync
* Beat Loop Mode
* Sampler Mode
* Beatjump mode

Testing:
* Keyboard Mode (check pitch value)
* Keyshift Mode (check pitch value
* Hot Cue Mode (including loops)
* Loop Section: Loop in / Out + Adjust, Call, Double, Half
* Effect Section (Beat FX left + Right - select the Effect Slot of FX3 (not Effect BPM))
* Output (lights)

Partially:
* PAD FX (only slots A-H, Q-P)
* Effect Section (without Beat FX left + Right - no equivalent function found)

Not working/implemented:
* Channel & Crossfader Start

…+ shift), decrease (pad 7 +shift)

                * fixed a bug in beatloop with 1/4 and 1/2 Beat loops
                * fixed a bug in BEAT SYNC + Shift (cycle tempo range)
                * added support for saving and playing loops on Hot Cue Pads
                * added support for loop in + out adjust (while looping press and hold in or out + jog wheel to adjust the loop poit position)
                * fixed a bug in Tempo Range switch (Beat Sync + shift)
@WarkerAnhaltRanger
Copy link
Author

@WarkerAnhaltRanger
Copy link
Author

WarkerAnhaltRanger commented Jun 5, 2019

  • implement blinking pad when playing sample, pad fx, key shift or keyboard
  • implement blinking Loop in and out when loop is activated

@WarkerAnhaltRanger WarkerAnhaltRanger changed the title [WIP] Add controller support for Pioneer DDJ-400 Add controller support for Pioneer DDJ-400 Jun 6, 2019
@WarkerAnhaltRanger
Copy link
Author

could someone review this?

@uklotzde
Copy link
Contributor

uklotzde commented Jun 7, 2019

Thank you for providing this mapping! We need the permission to distribute your contributions by signing the Mixxx Contributor Agreement

I have some initial technical remarks:

  • Please add a detailed description of the mapping to the wiki, example: https://www.mixxx.org/wiki/doku.php/roland_dj-505
  • The use of whitespace in the JS file is very inconsistent. Please consider reformatting it to improve readability.
  • I would recommend to run JSHint on the JS file. This should result in some recommendations like using ===/!== instead of ==/!=.

And please be patient if you don't get feedback within a few hours. We are all volunteers that spend their rare spare time for this project.

@WarkerAnhaltRanger
Copy link
Author

Thank you for your Feedback.
Signed the agreement.
Going to document mapping in the wiki soon. Basically I implemented the same functions as in the User manual for Rekordbox.
JSHint did not state any problems besides that hotcuePad in keyboardModeEnabledOutput was already defined. Fixed that.
I'll check the format.

@nschloe
Copy link
Contributor

nschloe commented Jun 19, 2019

I've tried this PR with my DDJ-400 and found it mostly working. At first glance, things not working:

  • Track selector scrolls fine, but doesn't load track on click.
  • Pressing any of HOT CUE, BEAT LOOP, BEST JUMP, SAMPLER results in the error message
    Uncaught exception at line 1 in passed code: TypeError: Result of expression '(PioneerDDJ400.modeChange)' [undefined] is not a function.

@uklotzde
Copy link
Contributor

It's ok to add missing features later, but at least the undefined access errors should be fixed.

@uklotzde
Copy link
Contributor

@nschloe Thanks for testing!

@WarkerAnhaltRanger
Copy link
Author

I've tried this PR with my DDJ-400 and found it mostly working. At first glance, things not working:

Thank you for testing!

* Track selector scrolls fine, but doesn't load track on click.

The press on the rotary selector cycles the focus between the Library Folders and the Tracks. (same as Rekordbox). You load the Track using the "LOAD" button

* Pressing  any of `HOT CUE`, `BEAT LOOP`, `BEST JUMP`, `SAMPLER` results in the error message
  `Uncaught exception at line 1 in passed code: TypeError: Result of expression '(PioneerDDJ400.modeChange)' [undefined] is not a function.`

I accidently forgot to remove the reference for the removed modeChange function in the XML File.
I updated the XML.

@nschloe
Copy link
Contributor

nschloe commented Jun 22, 2019

The press on the rotary selector cycles the focus between the Library Folders and the Tracks. (same as Rekordbox). You load the Track using the "LOAD" button

Perfect, that works.

I updated the XML.

Confirmed fixed.

@WarkerAnhaltRanger
Copy link
Author

I updated the Wikipage.
The current implementation is covered.
Also I made a Mistake with the .gitignore file (and I don't know how to revert this). The .gitignore File should not be merged.

@nschloe
Copy link
Contributor

nschloe commented Aug 7, 2019

The .gitignore File should not be merged.

Simply git revert the respective commit.

@nschloe
Copy link
Contributor

nschloe commented Aug 7, 2019

Btw, I've been using this PR succussfully with my DDJ-400 for a while now. How can I help getting this merged?

@jsBrian
Copy link

jsBrian commented Aug 10, 2019

I've been using this branch with my DDJ-400 on Ubuntu 18.04. Works for me as well.

@WarkerAnhaltRanger
Copy link
Author

Is there something missing or wrong with the code? How can I get this merged into mixxx?

@unlimited67g
Copy link

Hello all,

first thank you for your work.

I'm quite a noob.
Trying to make my DD 400 work with linux mint 19.2 cinnamon and Mixxx 2.2 .

I'm putting the .xml and .js files in the controllers folder but get this issue :

`
There was a problem parsing the XML file /usr/share/mixxx/controllers/Pioneer-DDJ-400.midi.xml.

unexpected character at line 44, column 89`

Any help welcome !

thanks again ;)

@uklotzde
Copy link
Contributor

uklotzde commented Sep 7, 2019

@WarkerAnhaltRanger Please interactively rebase your branch and remove all unrelated changes that affect files other than the added .xml and the .js file.

@uklotzde
Copy link
Contributor

uklotzde commented Sep 7, 2019

@unlimited67g Please verify that you downloaded the actual contents of the files (Raw) and not some HTML view from Github. I don't spot any errors on line 44 in the .xml file.

@uklotzde uklotzde added this to the 2.3.0 milestone Sep 7, 2019
@Be-ing
Copy link
Contributor

Be-ing commented Sep 7, 2019

Sorry for the delay. I'll review this today or tomorrow.

@Be-ing
Copy link
Contributor

Be-ing commented Sep 7, 2019

Unless you have mapped new features for Mixxx 2.3, please change the target branch of the pull request to 2.2 and rebase on the 2.2 branch. That way this can be included in Mixxx 2.2.3.

Copy link
Contributor

@Be-ing Be-ing left a comment

Choose a reason for hiding this comment

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

Some preliminary comments; I haven't read through the whole wiki or script yet. There are lots of little style issues. Please review the JS coding conventions

PioneerDDJ400.numFxSlots = 3;

Object.defineProperty(PioneerDDJ400, 'selectedFxSlot', {
get: function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This works with the ancient QtScript interpreter? I remember trying this years ago and couldn't get it to work.

});

PioneerDDJ400.padFxBelowPressed = ignoreRelease(function(channel, control, value, status, group) {
const groupAbove = group.replace(/\[EffectRack1_EffectUnit(\d+)_Effect(\d+)]/, function(all, unit, effect) {
Copy link
Contributor

Choose a reason for hiding this comment

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

const is not actually supported by the interpreter; that it happens to work is only a coincidence. Please change all consts to var to avoid confusion.


PioneerDDJ400.padFxBelowPressed = ignoreRelease(function(channel, control, value, status, group) {
const groupAbove = group.replace(/\[EffectRack1_EffectUnit(\d+)_Effect(\d+)]/, function(all, unit, effect) {
const effectAbove = parseInt(effect, 10) - 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the 10 parameter for parseInt is needed.

};

PioneerDDJ400.cueLoopCallLeft = function(_channel, _control, value, _status, group){
if(value == 0) return; // ignore release
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not use one line if statements.

// loop halve
engine.setValue(group, 'loop_scale', 0.5);
}
else{
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this to the previous line: } else {

}
}
//engine.setValue(group, 'loop_in_goto', 1);
engine.setValue(group, 'playposition', newPosition);
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand the purpose of this. This controller has hotcue buttons, so what does this add?


// loop_in / out adjust
const loopEnabled = engine.getValue(group, 'loop_enabled');
if(loopEnabled > 0){
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around parentheses: if (loopEnabled > 0) {

return;
}

// save playing loop on hotcue pad
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this hack should be removed. What is the point of saving a loop if it is lost when Mixxx shuts down? We are currently working on adding this feature to Mixxx in #2194. If you would like to help implement and/or test that feature, that would be great!


Object.defineProperty(PioneerDDJ400, 'selectedFxSlot', {
get: function() {
return engine.getValue('[EffectRack1_EffectUnit3]', 'focused_effect');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this use EffectUnit 3? This requires the user to show all 4 effect units on screen even though the controller can only manipulate one of them. Wouldn't it make more sense to map the controller to EffectUnit 1?

<control>
<description>RELOOP/EXIT +SHIFT (DECK1) - press - Active loop on/off</description>
<group>[Channel1]</group>
<key>reloop_toggle</key> <!-- TODO Check if correct -->
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest mapping this to reloop_andstop

@unlimited67g
Copy link

@unlimited67g Please verify that you downloaded the actual contents of the files (Raw) and not some HTML view from Github. I don't spot any errors on line 44 in the .xml file.

Hello,

thank you i did that and no more of this error !

But when i plug the controller and the open mixxx and then enable the midi controller in the preferences, i can't control anything with the controller...
Am i missing something (again..) ?

Thanks

@Be-ing Be-ing removed this from the 2.3.0 milestone Sep 14, 2019
@WarkerAnhaltRanger
Copy link
Author

I'm sorry. I'm afraid I can't finish this pull request since my RL took over.
Maybe someone else could finish this?

@Be-ing
Copy link
Contributor

Be-ing commented Oct 3, 2019

That's okay, thank you for communicating that. @nschloe or @jsBrian would you like to continue with this? You can start by making a branch on your fork from this branch and making a new pull request.

@nschloe
Copy link
Contributor

nschloe commented Oct 3, 2019

Thanks @WarkerAnhaltRanger! I've now created a new PR (#2312) where I'll try to work out the remaining things.

@Be-ing Be-ing closed this Oct 3, 2019
@nschloe nschloe mentioned this pull request Dec 7, 2019
@nschloe nschloe mentioned this pull request Dec 16, 2019
@Be-ing Be-ing mentioned this pull request Feb 7, 2020
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