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

Pioneer DDJ 200 support #2377

Closed
wants to merge 10 commits into from
Closed

Conversation

dan-giddins
Copy link

Adding support for the Pioneer DDJ 200

Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

Thanks for contibuting!

The code is rather short and I don't think there are any obvious issues.
However, I didn't find a wiki page for this mapping, which makes it hard to see if there are any problem with the way buttons are mapped.

If you didn't create a wiki page yet, please go ahead and document your mapping here: https://mixxx.org/wiki/doku.php/pioneer_ddj-200?do=edit
For the Pioneer DDJ-400, there's a wiki page here: https://mixxx.org/wiki/doku.php/pioneer_ddj-400
You could use that as a template and delete/change all buttons that do not exist or are mapped differently on the DDJ-200.

res/controllers/Pioneer DDJ-200.midi.xml Show resolved Hide resolved
res/controllers/Pioneer-DDJ-200-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-200-scripts.js Outdated Show resolved Hide resolved
@Holzhaus Holzhaus added this to the 2.2.4 milestone Dec 14, 2019
@Holzhaus
Copy link
Member

@dan-giddins Pinging you in case you didn't see the review comments. ;-)

@dan-giddins
Copy link
Author

@dan-giddins Pinging you in case you didn't see the review comments. ;-)
I did see, but I've been very busy recently! I will get around to getting this done at some point!

@Holzhaus
Copy link
Member

Another thing: Controller mappings are usually added to the current stable branch (2.2), not the development branch (master). Hence, you need to rebase your changes onto the 2.2 branch. I added some instructions to to the wiki in case you don't know how.

@dan-giddins dan-giddins changed the base branch from master to 2.2 March 29, 2020 16:59
@dan-giddins
Copy link
Author

dan-giddins commented Mar 29, 2020

Another thing: Controller mappings are usually added to the current stable branch (2.2), not the development branch (master). Hence, you need to rebase your changes onto the 2.2 branch. I added some instructions to to the wiki in case you don't know how.

@Holzhaus This should be done now? I haven't done a rebase before, so just let me know if anything is wrong

@Holzhaus
Copy link
Member

Another thing: Controller mappings are usually added to the current stable branch (2.2), not the development branch (master). Hence, you need to rebase your changes onto the 2.2 branch. I added some instructions to to the wiki in case you don't know how.

@Holzhaus This should be done now? I haven't done a rebase before, so just let me know if anything is wrong

The rebase worked fine. If it didn't, you'd see all commits from the master branch that are not present in 2.2 in addition to you changes listed here. Thanks.

@dan-giddins
Copy link
Author

@Holzhaus I've tried making an account on the forums to create the wiki page, but I'm not getting any email with my password in, and therefor I can't log in!

Account name = dan-giddins

@dan-giddins
Copy link
Author

@Holzhaus I have managed to get scratching and jogging working, but do you know how I could add this functionality to the jog wheel
image
For reference: https://www.recordcase.de/media/pdf/cd/8f/4a/Pioneer-DDJ-200-User-Manual.pdf

@Holzhaus
Copy link
Member

Holzhaus commented Apr 4, 2020

@Holzhaus I've tried making an account on the forums to create the wiki page, but I'm not getting any email with my password in, and therefor I can't log in!

Account name = dan-giddins

Sorry, we sometimes have problems with forum emails not getting sent. I think the Wiki account and the forum account are not linked, so you can try to create a wiki account here: https://mixxx.org/wiki/doku.php/start?do=register

@rryan Can you activate the forum account manually?

@Holzhaus
Copy link
Member

Holzhaus commented Apr 4, 2020

@Holzhaus I have managed to get scratching and jogging working, but do you know how I could add this functionality to the jog wheel
image
For reference: https://www.recordcase.de/media/pdf/cd/8f/4a/Pioneer-DDJ-200-User-Manual.pdf

Have a look at: https://github.com/mixxxdj/mixxx/blob/master/res/controllers/Roland_DJ-505-scripts.js#L429-L435

@dan-giddins
Copy link
Author

@Holzhaus sorry I mean a wiki account.

@dan-giddins
Copy link
Author

@Holzhaus great, just waiting on the wiki account now :)
Still haven't got any password email so I can't log in

@uklotzde uklotzde modified the milestones: 2.2.4, 2.2.5, 2.2.x Jun 12, 2020
@Holzhaus
Copy link
Member

Holzhaus commented Jun 30, 2020

@dan-giddins

@Holzhaus great, just waiting on the wiki account now :)
Still haven't got any password email so I can't log in

I'm sorry for the massive delay. Unfortunately, we were unable to fix the issues with our Wiki mail. We finally moved to GitHub wiki instead, and since you already have a GitHub account you should now be able to create a Wiki page for this controller: https://github.com/mixxxdj/mixxx/wiki

@Holzhaus
Copy link
Member

I installed the config file according to the installation link like this:
pre-commit sample-config >.pre-commit-config.yaml
Or where can I find this file?

Ok, you overwrote our config file with the sample configuration. Please undo that.

Just install pre-commit either via pip or your package manager, then run pre-commit install: https://pre-commit.com/#3-install-the-git-hook-scripts
You need to skip the "Add a pre-commit configuration" step because we already have a configuration and you're not setting up pre-commit support for a new repository.

@fkbreitl
Copy link
Contributor

I have updated the code accordingly at
dan-giddins/mixxx-ddj-200-mapping@master...fkbreitl:master
However, now Code Factor complains about 'engine' and 'script' is not defined.
It didn't do this before.
Why? Is this a problem? Of course its know by Mixxx.

@Holzhaus
Copy link
Member

Ignore codefactor. If precommit agrees, all is fine.

@fkbreitl
Copy link
Contributor

Ok, I think my PR dan-giddins/mixxx-ddj-200-mapping#11 is still valid and has my updates.
Then do we have to wait for Dan to pull it or can you pull it directly?

We also have a new 4 deck mapping. Can we have two mappings (a 2D and 4D version) for the same controller in Mixxx.

@Holzhaus
Copy link
Member

Ok, I think my PR dan-giddins/mixxx-ddj-200-mapping#11 is still valid and has my updates.
Then do we have to wait for Dan to pull it or can you pull it directly?

You can also open a new pull request directly from your repo to this one, then I can close this PR.

We also need documentation for the user manual (starting with Mixxx 2.3, documentation for built-in mappings will be part of the normal manual, not the wiki). I migrated the Wiki page to the RST format using pandoc and added some graphics. Please fork this branch and open a PR for the manual-2.3.x branch on the manual repo: https://github.com/Holzhaus/manual/tree/pioneer-ddj-200

This needs to be taken care of before we can merge.

@fkbreitl
Copy link
Contributor

fkbreitl commented Oct 18, 2020

Ok, I have created the PR for the manual. You did a lot of work. Looks very good. Thank you!
Holzhaus/manual#2

How can I created a PR from my repo to this one?

@Holzhaus
Copy link
Member

Holzhaus commented Oct 18, 2020

How can I created a PR from my repo to this one?

Oh, I just realized you created a new repo for the mapping.

You need to fork the mixxx repository (click the little fork button at the top right corner, clone it, run git checkout -b ddj-200 origin/2.2, add your mapping files to res/controllers and open a PR). See https://github.com/mixxxdj/mixxx/wiki/Contributing%20mappings#using-git-with-your-mapping for details.

EDIT: in this case it would be nice to keep @dan-giddins in the commit history. So I suggest to fork/clone the Mixxx repo, then run:

$ git remote add dan-giddins https://github.com/dan-giddins/mixxx.git
$ git fetch dan-giddins
$ git checkout ddj-200-support

Then overwrite the files with your fixed versions, make a new commit, push and open a PR against the 2.2 branch.

@fkbreitl
Copy link
Contributor

fkbreitl commented Oct 18, 2020

Just a side question: Do we really have to use camel case for all JS variables?
Because for variables of Mixxx controls I would prefer to use the same name for clarity, i.e. "sync_enabled" instead of "syncEnabled".

@Holzhaus
Copy link
Member

Just a side question: Do we really have to use camel case for all JS variables?
Because for variables of Mixxx controls I would prefer to use the same name for clarity, i.e. "sync_enabled" instead of "syncEnabled".

Yes, please use camelCase for all variables. That also makes it easier to distinguish variables from the actual control.

@fkbreitl
Copy link
Contributor

And what is the git push command I have to use?
git push origin 2.2?
Or is git push just fine?

@Holzhaus
Copy link
Member

Use the name of your current branch. If you followed by edited instructions above, use git push origin ddj-200-support.

@fkbreitl
Copy link
Contributor

Great, that's what I guessed. The PR is here:
#3185
But I don't know how to revert the file renaming, which you mentioned.

@Holzhaus
Copy link
Member

Holzhaus commented Oct 18, 2020

But I don't know how to revert the file renaming, which you mentioned.

Run git log -- path/to/file to find the latest commit that changed the file. Copy the commit ID and verify that it's indeed that commit:

  git show <commit>

If you found the commit, run this to undo the changes:

git reset --hard <commit>~1 -- path/to/file

This sets the file to the state it had 1 commit before that commit. Now run git commit -m foo to commit the changes.

Then do a rebase to remove the changes from that commit:

git rebase -i <commit>~1

Now copy the line with your latest commit (message "foo") right below the line that contains the commit you found via git log and change pick to fixup.

EDIT: I think commit 9a3742d is the perpetrator. You can simply remove that commit completely:

git rebase -i 9a3742d5e8c3cbb3d56bb8872e39b99275a6d437~1

Then remove the line with that commit id (or replace pick with drop).

@Holzhaus
Copy link
Member

This has been superseeded by #3185, closing this.

@Holzhaus Holzhaus closed this Oct 18, 2020
@Holzhaus Holzhaus mentioned this pull request Oct 18, 2020
@fkbreitl
Copy link
Contributor

fkbreitl commented Oct 18, 2020

@Holzhaus I get this error:

$ git reset --hard 9a3742d~1 -- Pioneer\ DDJ-SB.midi.xml
fatal: Cannot do hard reset with paths.

@Holzhaus
Copy link
Member

oh, then do it without --hard. I think it already stages the changes in that case, but the working tree remains unchanged.

@dan-giddins
Copy link
Author

@fkbreitl As you seem much more active with this work, I'm going to leave it up to you to merge this feature into MIXXX. Message me if you want any help, and I am happy to do PRs for you :)

@fkbreitl
Copy link
Contributor

Thanks @dan-giddins, we are close. Have a look at #3185.

@fkbreitl
Copy link
Contributor

@dan-giddins The DDJ-200 mapping is in v2.2 now! Also the 4-deck version is on its way! See #3193.

@Marees213
Copy link

After updating my cross fader when moved fast to left or right selects automaticly a new song on decks accordingly.... can i still pull a request since its closed?

@fkbreitl
Copy link
Contributor

Which version are you using?

@Marees213
Copy link

Marees213 commented Mar 26, 2021 via email

@Holzhaus
Copy link
Member

If you have a fix for this, please open a new pull request.

@Marees213
Copy link

Marees213 commented Mar 26, 2021 via email

@dan-giddins
Copy link
Author

@Marees213 are you sure you don't have Auto-DJ on? This is a deliberate function of Auto DJ I believe.

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.

5 participants