Numark Mixtrack Pro 3 mapping by djsteph · Pull Request #905 · mixxxdj/mixxx

djsteph · 2016-03-17T01:10:08Z

Hello
I have completed the Numark Mixtrack Pro 3 mapping files (version 1.1). Changes noted in the JS file. Forum thread is here: http://mixxx.org/forums/viewtopic.php?f=7&t=7286&start=140#p28103 and documentation of mapping is here: http://mixxx.org/wiki/doku.php/numark_mixtrack_pro_3

Thank you

Steph

Numark Mixtrack Pro 3 mapping version 1.1

Be-ing · 2016-03-17T06:15:12Z

Great to see this finally submitted :) It may take me a bit to get a chance to thoroughly review this because there are still a few other mappings I haven't finished reviewing. At first glace the code and wiki both look pretty good.

sblaisot · 2016-03-17T06:28:33Z

PLease update windows uninstaller: https://mixxx.org/wiki/doku.php/contributing_mappings#windows_installer_update

sblaisot · 2016-03-18T21:34:15Z

  Delete "$INSTDIR\controllers\Xone K2.midi.xml"
  Delete "$INSTDIR\controllers\Xone-K2-scripts.js"
+  Delete "$INSTDIR\controllers\Numark-Mixtrack-3.midi.xml"
+  Delete "$INSTDIR\controllers\Numark-Mixtrack-3-scripts.js"


Please keep all files in alphabetical order. This will be easier for future maintenance.
Could you please move these two lines in the correct place ?

Be-ing · 2016-04-06T04:15:03Z

Okay, I read through the wiki. I did quite a bit of editing to make the language and formatting more concise and easier to navigate. I haven't really dug into the code yet, but I see that, wow, there's a lot of it! But it looks neat and well written. :)

It's good that you've included some options, but the amount of options is kinda overwhelming. Some of them could be removed. IMO the iCutEnabled and fastSeekEnabled options are unnecessary. Because these features have to be deliberately activated by the user by holding Shift, they do not interfere with other functionality. If a user doesn't want to use them, all they have to do is not use them. Keeping the options available clutters the script and wiki.
Also the TapExpandLibrary option could be removed if pushing the browse encoder is mapped to [Master]. maximize_library. I think that would be a better use of it considering that there are load buttons right next to it.
I think it would be clearer to rename PADLoopButtonPressed to PADLoopButtonHold and PADSampleButtonPressed to PADSampleButtonHold.

What color are the pads in Manual Loop Mode?

Is the microphone input mixed with the main output in hardware? Is the input available as a sound card input to the computer?

Are there any volume controls available in your OS mixer program?

djsteph · 2016-04-06T17:40:01Z

Hello Be,

Thank you for reviewing the wiki.

I added the options "iCutEnabled" and "fastSeekEnabled" because of the
Shift Lock feature: If Shift Lock is enabled and you move the platter
accidently, ICut or fast seek could be triggered unintentionnaly. The
alternative would be to remove the options "iCutEnabled" and
"fastSeekEnabled" and have one to enable the Shift lock feature.

I agree with the following: "the TapExpandLibrary option could be removed
if pushing the browse encoder is mapped to [Master]. maximize_library." and
"it would be clearer to rename PADLoopButtonPressed to PADLoopButtonHold
and PADSampleButtonPressed to PADSampleButtonHold."

What color are the pads in Manual Loop Mode? When enabled, they are blue.
If no loop start is present in manual loop, the pads leds are "off"

For the microphone section, I copied the text from the Numark user manual,
but did not test it with Mixxx as I don't have a mic to test with (sorry, I
no longer do live performance).

Thanks

Steph

On Wed, Apr 6, 2016 at 12:15 AM, Be notifications@github.com wrote:

Okay, I read through the wiki. I did quite a bit of editing to make the
language and formatting more concise and easier to navigate.

It's good that you've included some options, but the amount of options is
kinda overwhelming. Some of them could be removed. IMO the iCutEnabled and
fastSeekEnabled options are unnecessary. Because these features have to be
deliberately activated by the user by holding Shift, they do not interfere
with other functionality. If a user doesn't want to use them, all they have
to do is not use them. Keeping the options available clutters the script
and wiki.
Also the TapExpandLibrary option could be removed if pushing the browse
encoder is mapped to [Master]. maximize_library. I think that would be a
better use of it considering that there are load buttons right next to it.
I think it would be clearer to rename PADLoopButtonPressed to
PADLoopButtonHold and PADSampleButtonPressed to PADSampleButtonHold.

What color are the pads in Manual Loop Mode?

Is the microphone input mixed with the main output in hardware? Is the
input available as a sound card input to the computer?

Are there any volume controls available in your OS mixer program?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#905 (comment)

Be-ing · 2016-04-06T18:46:49Z

Would making iCutEnabled and fastSeekEnabled only disable those behaviors when shift lock is on make more sense?

Please add that info about the pad LEDs to the wiki.

You don't actually need a microphone to test how the microphone input works. In Mixxx's Sound Hardware preferences, on the input tab, can you select any inputs from the Mixtrack Pro 3? If not, then the mic input is mixed with the main output in hardware and not routed to the computer.

Regarding the sound card volume, I put some info about that on the Hercules P32 wiki page about accessing the OS mixer: http://mixxx.org/wiki/doku.php/hercules_p32_dj . What OS do you use? Do you see any controls for the Mixtrack Pro 3's sound card?

djsteph · 2016-04-06T19:01:04Z

I think that disabling shift lock altogether may be a better option. This
is something that was already in the script when I started to modify it.
This would reduce the number of options in the script.
I think it would also prevent unintentionnal actions... e.g. touching a
button and not realizing that shift lock is, resulting in an unexpected
behavior.

I'll adjust the wiki when I have more spare time .... same for mic testing.

Stéph

On Wed, Apr 6, 2016 at 2:46 PM, Be notifications@github.com wrote:

Would making iCutEnabled and fastSeekEnabled only disable those behaviors
when shift lock is on would make more sense?

Please add that info about the pad LEDs to the wiki.

You don't actually need a microphone to test how the microphone input
works. In Mixxx's Sound Hardware preferences, on the input tab, can you
select any inputs from the Mixtrack Pro 3? If not, then the mic input is
mixed with the main output in hardware and not routed to the computer.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#905 (comment)

djsteph · 2016-04-07T00:36:34Z

Hello,

I checked for the mic, and I don't see any mic inputfrom the controller. So
mic is mixed with the main output in hardware and not routed to the computer

Stéph

On Wed, Apr 6, 2016 at 3:01 PM, Stephane Morin steph@smorin.com wrote:

I think that disabling shift lock altogether may be a better option. This
is something that was already in the script when I started to modify it.
This would reduce the number of options in the script.
I think it would also prevent unintentionnal actions... e.g. touching a
button and not realizing that shift lock is, resulting in an unexpected
behavior.

I'll adjust the wiki when I have more spare time .... same for mic testing.

Stéphane Morin
Maison: 450 510-0622
Cellulaire: 514 243-5876

On Wed, Apr 6, 2016 at 2:46 PM, Be notifications@github.com wrote:

Would making iCutEnabled and fastSeekEnabled only disable those behaviors
when shift lock is on would make more sense?

Please add that info about the pad LEDs to the wiki.

You don't actually need a microphone to test how the microphone input
works. In Mixxx's Sound Hardware preferences, on the input tab, can you
select any inputs from the Mixtrack Pro 3? If not, then the mic input is
mixed with the main output in hardware and not routed to the computer.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#905 (comment)

Be-ing · 2016-04-07T03:20:27Z

It seems that double tapping the Shift button isn't something that would be easy to do accidentally, so I don't think making an option to disable would be very useful. It also seems to me that the use cases when shift lock would be most useful would be for using iCut or fast track searching.

Good to know about the microphone. Please make a note of it on the wiki. (Btw, I added a new section to the Contributing Mappings wiki page about this http://mixxx.org/wiki/doku.php/contributing_mappings#microphone_inputs)

djsteph · 2016-04-07T10:02:13Z

You are correct Be! That is when shift lock is most useful.

So how do we conclude? To me, the options are available at the time of
set-up then the user forgets about it. There may be a lot, but having used
the controller, I felt that they could useful. The mapping has been
downloaded many times and got no feedback regarding the options.

Envoyé de mon iPad

Le 6 avr. 2016 à 23:20, Be notifications@github.com a écrit :

It seems that double tapping the Shift button isn't something that would be
easy to do accidentally, so I don't think making an option to disable would
be very useful. It also seems to me that use case when shift lock would be
most useful would be for using iCut or fast track searching.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#905 (comment)

Be-ing · 2016-04-07T22:27:41Z

Okay, I guess leave those options as they are. I edited the "Configuration options" and "Platter/Jog Wheel" sections of the wiki to clarify their purpose.

Leave a comment when you've made the other changes

* - Renamed user options: PADLoopButtonPressed to PADLoopButtonHold * - Renamed user options: PADSampleButtonPressed to PADSampleButtonHold * - TapExpandLibrary moved from Tap button to Browse Button push * - Linked printComments to debug value of init function: The debugging parameter is set to 'true' if the user specified the --mididebug parameter on the command line * - Cleaned NumarkMixtrack3.shutdown function

djsteph · 2016-04-11T01:48:49Z

I have pushed a new commit with requested changes. I'll update the wiki when I get a chance to repost the mapping on the forum. If I do it now, it will be confusing.

Be-ing · 2016-04-11T03:47:43Z

+    LibraryGroup ="[Master]";
+    LibraryCommand = "maximize_library";
+    }
+


Considering this code only works differently for the Dark Metal skin, I think it would make more sense to change the skin option to a boolean called DarkMetal.

Other than this little thing, looks good :)

Be-ing · 2016-04-11T03:51:53Z

I notice there are 4 empty lines at the end of the JS file. May as well delete the extra 3.

Be-ing · 2016-04-11T16:29:40Z

Btw, it would be easier to make sense of the changelog if someone looked back on this in the future if you split the changes into smaller commits. GUI git clients can be helpful for this. Personally I prefer Git Cola. I can look through all the changes I've made since the last commit and only select a small group of related changes to stage for a smaller commit, then repeat for the next related group of changes.

djsteph · 2016-04-11T20:44:30Z

Hello Be,

You'll have to give me an exemple, I don't understand what you mean by

it would be easier to make sense of the changelog if someone looked back on this in the future if you split the changes into smaller commits

I tried to provide enought details without going to line number changes.

Be-ing · 2016-04-11T21:14:23Z

Yes, you have provided enough details. But just looking at the names of the commits, "Update based on Be 's feedback" doesn't provide much info at a glance; a reader would have to read the extended commit message to see what changed. PRs #828, #902, and #912 are good examples with smaller commits. It's not a big deal, so if you find it too much trouble, don't worry about it.

- changed skin option to use boolean for DarkMetalSkin. It requires different code to expand library view. - removed trailing empty lines at end of script

djsteph · 2016-04-12T01:23:40Z

Here you go, this should be good to go now :). If all is Ok, the wiki will need updating of options changes, changes to Browse and tap button and include color of LEDs of Pad buttons when in manual mode

Be-ing · 2016-04-12T01:49:02Z

JSHint gives one small nitpicky warning:
Numark-Mixtrack-3-scripts.js: line 1749, col 35, A leading decimal point can be confused with a dot: '.75'.

So change .75 to 0.75.

Be-ing · 2016-04-12T01:57:37Z

-    LibraryGroup ="[Master]";
-    LibraryCommand = "maximize_library";
-    }
+    if (DarkMetalSkin){


The alignment is off on GitHub's web rendering of this file. It seems that's because you've mixed tabs and spaces here. The rest of the file correctly uses 4 spaces to indent. Use that here too.

Also, IMO it's clearer to read to put a space between ) and { for conditional statements. It looks like there are a few other places you've done this and also places that you didn't put a space between if/for and an opening ( too. Some simple find and replace operations should take care of those.

- line 1749, change .75 to 0.75 in var gammaOutputRange - added spacing in numerous place for easier reading, - changed tabs to spaces

djsteph · 2016-04-12T02:12:53Z

Changes done..

Be-ing · 2016-04-12T02:19:45Z

Looks good to me! 👍 Thank you! I'm really glad to have another quality, complete mapping for one of the most popular controllers on the market right now.

Whenever you get around to your TODO items, feel free to open another pull request.

djsteph · 2016-04-13T17:54:57Z

Thank you Be for taking the time to review this mapping.What is the next step for this mapping to be merge in the master branch for the next release?

rryan · 2016-04-13T18:03:08Z

+	<name>Numark Mixtrack (Pro) 3</name>
+	<author>Stéphane Morin</author>
+	<description>The Numark Mixtrack 3 and Numark Mixtrack Pro 3 are the same controller except that the Pro version has an integrated sound card.</description>
+        <wiki>http://www.mixxx.org/wiki/doku.php/numark_mixtrack_pro_3</wiki>


Is there a forum post for this mapping yet? Would you like to make a placeholder forum post and put it here?
The syntax is: <forums>url</forums>. Make sure to escape the & as &

rryan · 2016-04-13T18:04:28Z

Thank you Be for taking the time to review this mapping.What is the next step for this mapping to be merge in the master branch for the next release?

Thanks for doing the review @Be-ing and thank you for the nice mapping @djsteph -- next step is that I'll merge this but had a quick question about adding a forum link.

Added link to Mixtrack Pro 3 forum in XML file

djsteph · 2016-04-14T01:13:04Z

The forums link is added in the XML file
Thanks

rryan · 2016-04-16T06:03:43Z

👍

Numark Mixtrack Pro 3 mapping

17d1acf

Numark Mixtrack Pro 3 mapping version 1.1

Updated uninstaller as per comment for Mixtrack Pro 3 mapping

161b9ea

sblaisot reviewed Mar 18, 2016
View reviewed changes

Update to ensure controller is listed in alphabetical order

60959d2

Be-ing reviewed Apr 11, 2016
View reviewed changes

Changed skin option to use boolean if Dark Metal skin is used

3d0ff1a

- changed skin option to use boolean for DarkMetalSkin. It requires different code to expand library view. - removed trailing empty lines at end of script

Be-ing reviewed Apr 12, 2016
View reviewed changes

Minor changes - Jhint review and readability

c527afe

- line 1749, change .75 to 0.75 in var gammaOutputRange - added spacing in numerous place for easier reading, - changed tabs to spaces

rryan reviewed Apr 13, 2016
View reviewed changes

Added link to Mixtrack Pro 3 forum in XML file

32792c8

Added link to Mixtrack Pro 3 forum in XML file

rryan merged commit 28121bb into mixxxdj:master Apr 16, 2016

Be-ing mentioned this pull request May 5, 2016

fixed the controller mappings for ION-discover DJ #936

Merged

Uh oh!

Conversation

djsteph commented Mar 17, 2016

Uh oh!

Be-ing commented Mar 17, 2016

Uh oh!

sblaisot commented Mar 17, 2016

Uh oh!

sblaisot Mar 18, 2016

Choose a reason for hiding this comment

Uh oh!

Be-ing commented Apr 6, 2016

Uh oh!

djsteph commented Apr 6, 2016

Uh oh!

Be-ing commented Apr 6, 2016

Uh oh!

djsteph commented Apr 6, 2016

Uh oh!

djsteph commented Apr 7, 2016

Uh oh!

Be-ing commented Apr 7, 2016

Uh oh!

djsteph commented Apr 7, 2016

Uh oh!

Be-ing commented Apr 7, 2016

Uh oh!

djsteph commented Apr 11, 2016

Uh oh!

Be-ing Apr 11, 2016

Choose a reason for hiding this comment

Uh oh!

Be-ing Apr 11, 2016

Choose a reason for hiding this comment

Uh oh!

Be-ing commented Apr 11, 2016

Uh oh!

Be-ing commented Apr 11, 2016

Uh oh!

djsteph commented Apr 11, 2016

Uh oh!

Be-ing commented Apr 11, 2016

Uh oh!

djsteph commented Apr 12, 2016

Uh oh!

Be-ing commented Apr 12, 2016

Uh oh!

Be-ing Apr 12, 2016

Choose a reason for hiding this comment

Uh oh!

djsteph commented Apr 12, 2016

Uh oh!

Be-ing commented Apr 12, 2016

Uh oh!

djsteph commented Apr 13, 2016

Uh oh!

rryan Apr 13, 2016

Choose a reason for hiding this comment

Uh oh!

rryan commented Apr 13, 2016

Uh oh!

djsteph commented Apr 14, 2016

Uh oh!

rryan commented Apr 16, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants