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-400: Address outstanding issues #3479

Merged
merged 88 commits into from
Jan 31, 2021
Merged

Conversation

jusko
Copy link
Contributor

@jusko jusko commented Dec 22, 2020

Todo

  • Address review points

Done
Addressing the review comments in #2403:

  • Implements the same beatjump features as rekordbox
  • Remove -4BEAT loop hack
  • Refactor jog wheel beat adjust
  • Implement outstanding PAD-FX (Not much that can really be added here at the moment)
  • Remove cue point scroll
  • Replace function declarations with function expressions and also give the script some TLC to standardize it stylistically

Issues picked up in Q/A Testing

  • BEAT SYNC does not work in the same way as clicking on the software control.
  • Allow disabled loops to be halved/doubled by < and >
  • Unmap master knob from software gain
  • Fix Channel FX switch

Xmas presents (if there's time) 🎅

  • Smooth out BEAT FX mappings
  • Add a few easter eggs

Pending Further Discussion

  • Drop or reimplement all secondary pad mode features in the discussion started here

    In short, the controller has 4 primary pad modes (HOT CUE, BEAT LOOP, BEAT JUMP and SAMPLER) and 4 secondary (i.e, shifted) pad modes (KEYBOARD, PAD FX1, PAD FX2 and KEY SHIFT).

    The primary pad modes are fully implemented and work well.

    The secondary pad modes are not production quality and are the results of proof of concept prototypes. They either need to be rewritten or dropped.

  • Make FX workflow a bit more intuitive

    A single knob on the controller is switched between wet/dry mix and metaknob mappings.

    Currently it is:

    • Set to the metaknob when an effect is enabled
    • Set to the mix when an effect is disabled or focus is explicitly removed from an effect.

    It is more intuitive to:

    • Set to the metaknob when an effect is disabled
    • Set tot the mix when an effect is enabled

Documentation* 📝
PR for the manual is over here

@jusko jusko mentioned this pull request Dec 22, 2020
@Be-ing Be-ing changed the base branch from main to 2.3 December 22, 2020 20:58
@Be-ing
Copy link
Contributor

Be-ing commented Dec 22, 2020

Thanks for continuing this! Please fix the pre-commit issues. You can download the patch as an artifact from the GH Actions job. To prevent more issues going forward, set up pre-commit locally.

@Holzhaus
Copy link
Member

If I'm not mistaken this PR is based on the 2.2 branch and can already be part of the 2.2.5 release.

@Holzhaus Holzhaus changed the base branch from 2.3 to 2.2 December 22, 2020 21:32
@Be-ing
Copy link
Contributor

Be-ing commented Dec 22, 2020

There will be no 2.2.5 release. We don't have working 2.2 build servers anymore.

@jusko jusko marked this pull request as draft December 23, 2020 06:08
@jusko
Copy link
Contributor Author

jusko commented Dec 23, 2020

Updated the PR description to summarise the remaining discussion.

Also a bit confused about the base branch: could you confirm if I should rebase onto 2.3 and change the target here back to it?

@Holzhaus
Copy link
Member

No, just leave it on 2.2. There are no conflicts and no unrelated commits in this PR, so there's no reason to rebase.

We can safely merge it into the 2.2 branch and it well end up in 2.3 anyway, regardless of whether we make a 2.2.5 release or not.

@jusko
Copy link
Contributor Author

jusko commented Dec 23, 2020

I'm going through each control, one at a time, as I write up the manual. Catching some further issues which I'll add to the list in the description...

@jusko jusko marked this pull request as ready for review December 26, 2020 09:10
@jusko
Copy link
Contributor Author

jusko commented Dec 26, 2020

Quite a few little fixes made during QA (while writing the manual).

The FX workflow is also improved, and the biggest change: the three experimental pad modes dropped.

Also moved the code around so that it is logically structured in the same way as the controller (which makes it a bit easier to maintian, for me at least).

Will keep testing it as I get mixxx setup on a Pi 😉

@timewasternl
Copy link

Quite a few little fixes made during QA (while writing the manual).

The FX workflow is also improved, and the biggest change: the three experimental pad modes dropped.

Also moved the code around so that it is logically structured in the same way as the controller (which makes it a bit easier to maintian, for me at least).

Will keep testing it as I get mixxx setup on a Pi 😉

Off-topic
Awesome you're playing with it on a Pi too. Check out the setup I did with it a while ago: https://djtechtools.com/2020/11/05/building-a-standalone-ddj-400-with-a-raspberry-pi-and-mixxx/
Finishing this mapping will surely contribute to the ease of setting it up like this.

jusko and others added 11 commits January 30, 2021 16:54
- Removes getters & setters with side effects
- Uses callbacks to keep the FX light in sync when changes
  are made through the UI
Co-authored-by: Be <be.0@gmx.com>
Relies solely on <outputs> for lighting up the required pads in
beatjump mode.

This removes the need to track the controllers padmode in the script.

This is desirable, because the controller tracks the padmode in its
firmware and seems to be susceptible to benign but unexpected
behavior when that process is interfered with (e.g., manually
resetting the padmode lights sometimes prevents the next pad mode
from being enabled until a sequence of other buttons are pressed).
@jusko jusko changed the base branch from 2.2 to 2.3 January 30, 2021 14:57
@jusko
Copy link
Contributor Author

jusko commented Jan 30, 2021

Let's rebase this branch on 2.3 and target 2.3. Though I am not sure if GitHub will execute the CI checks and builds from 2.3 then. We'll see.

Done 👍

@uklotzde
Copy link
Contributor

All green 👍

// * Mixxx mapping script file for the Pioneer DDJ-400.
// * Authors: Warker, nschloe, dj3730, jusko
// * Reviewers: Be-ing, Holzhaus
// * Manual: https://manual.mixxx.org/2.3/en/hardware/controllers/pioneer_ddj_400.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove the /en/ part of the URL? I guess a redirect (301) will add this implicitly.

@Be-ing Be-ing merged commit 1951f44 into mixxxdj:2.3 Jan 31, 2021
@Be-ing
Copy link
Contributor

Be-ing commented Jan 31, 2021

Thanks for bearing with this review @jusko! I am glad this is finally done. I am glad we came up with something useful and usable for the limited and odd effects controls on this controller.

@nschloe
Copy link
Contributor

nschloe commented Feb 1, 2021

Thanks also by me @jusko! Finally, this long-standing project is coming to a close. My DDJ-400 works better than ever! 🚀

@uklotzde
Copy link
Contributor

uklotzde commented Feb 1, 2021

@ywwg Considering the popularity of this controller we could publish a short message on our blog and forward to social media. We need to mention that it is only available in the beta. Another chance to get more beta testers on board ;)

@jusko
Copy link
Contributor Author

jusko commented Feb 1, 2021

Thanks also by me @jusko! Finally, this long-standing project is coming to a close. My DDJ-400 works better than ever! rocket

All thanks to the work you, @dj3730 and Warker (the originator) put in @nschloe. Organic open source evolution ftw! 🙂

Thanks for bearing with this review @jusko! I am glad this is finally done. I am glad we came up with something useful and usable for the limited and odd effects controls on this controller.

Really appreciate a thorough review @Be-ing. 👍 Thank you and @Holzhaus for all of your time and help.

@ywwg
Copy link
Member

ywwg commented Feb 1, 2021

publish a short message on our blog

noted!

@ywwg
Copy link
Member

ywwg commented Feb 1, 2021

@jusko Can you write a short paragraph describing the updates? I see the bullet points above but I don't know all of the context

@Be-ing
Copy link
Contributor

Be-ing commented Feb 1, 2021

There was no DDJ-400 support before this so there aren't really any updates to describe.

@ywwg
Copy link
Member

ywwg commented Feb 1, 2021

ah ok it sounded like it was updates to existing support. I'd still appreciate a short summary that I can cut and paste rather than try to deduce the major achievements / limitations

@Holzhaus
Copy link
Member

Holzhaus commented Feb 1, 2021

Instead of highlight features of this particular controller, you could maybe just make a blogpost saying that Mixxx 2.3 will add ootb support for popular controllers such as the Pioneer DDJ-200, Pioneer DDJ-400, Native Instruments Traktor S3, ...?

@uklotzde
Copy link
Contributor

uklotzde commented Feb 1, 2021

Including links to the manual pages for quick reference.

@ywwg
Copy link
Member

ywwg commented Feb 2, 2021

sounds good!

@georgitsenov
Copy link

Hay fellas!

I am using mixxx 2.3.0 on Ubuntu 20.04.2 with Pioneer DDJ-400 controller and I am very happy with both the software and the hardware. The mapping is awesome too - thanks to everyone involved! I am trying to find a way to enable vinyl break, like it can be configured in Serato in order to perform this:
https://youtu.be/3dl2Lpnqy0w

I checked on various repositories, mixxx forums and launchpad and this pull request is the closest thing, that I found:
#2591

Do you have any info on how this can be scripted as advised from Mixxx designer here:
https://mixxx.discourse.group/t/turn-off-turntable-effect/13282

Or perhaps where I can submit such feature request?
Thanks!

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.

10 participants