Conversation
|
Thank you for submitting this mapping. Please also update windows uninstaller : https://www.mixxx.org/wiki/doku.php/contributing_mappings#windows_installer_update |
|
Thanks again for submitting this. As you may have noticed, we have a bit of a backlog of pull requests we're trying to wrap up for Mixxx 2.1, so it may be a little while before I get around to reviewing this. |
| /////////////////////////////////////////////////////////////// | ||
|
|
||
| PioneerDDJSX.init = function(id) { | ||
| PioneerDDJSX.syncRate = 0; |
There was a problem hiding this comment.
I believe you can use this here, and in many other places instead of repeating PioneerDDJSX.
There was a problem hiding this comment.
I'm not sure if this works in the init and shutdown functions. I think this might be forcibly set to the global object for those functions, but I'd have to test.
There was a problem hiding this comment.
@radusuciu : I changed the code according to your remarks! Sorry, but I didn't knew it better concerning hasOwnProperty, but now I know it!
I tried this, but in the most cases it won't work. So I don't use it, with PioneerDDJSX I'm safe.
|
|
||
| PioneerDDJSX.blinkAutodjState = 0; | ||
|
|
||
| PioneerDDJSX.vuMeterTwinkle = function() { |
There was a problem hiding this comment.
Quite a bit of repetition in this function, could be simplified imo.
| } | ||
|
|
||
| for (index in PioneerDDJSX.selectedLoopIntervals[deck]) { | ||
| if (typeof index === "string") { |
There was a problem hiding this comment.
Don't we usually want to use hasOwnProperty here?
| } | ||
| }; | ||
|
|
||
| PioneerDDJSX.parameterLeft = function(channel, control, value, status, group) { |
There was a problem hiding this comment.
Lots of duplication between parameterLeft and parameterRight functions. May be worth merging them into a single method and passing, or calculating the appropriate bit values to use.
…comment @radusuciu), replaced if statement (typeof index === „string“) for property validation with hasOwnProperty (comment @radusuciu), separated and combined code of PioneerDDJSX.parameterLeft() and PioneerDDJSX.parameterRight() into new function PioneerDDJSX.changeParameters() (comment @radusuciu), the keyword „this“ does not work in most cases, so it isn’t used. Pioneer DDJ-SX.midi.xml: corrected info header.
| } | ||
| }; | ||
|
|
||
| PioneerDDJSX.softTakeoverEmulation = function(unit, index, currentValue) { |
There was a problem hiding this comment.
engine.softTakeover was fixed with engine.setParameter in #1065, so you should be able to remove this now.
There was a problem hiding this comment.
Sorry, but I don't really understand that...can you explain it a bit? I tried engine.setParameter but it jumps to actual fader/knob value instead of take it over if I reach the in mixxx existing value (softTakeover).
There was a problem hiding this comment.
I thought I need engine.softTakeover in both cases, if I use engine.setParameter or engine.setValue. Do you mean that I should prefer engine.setParameter?
There was a problem hiding this comment.
I accidentally made the comment on the wrong line of code. I meant to say you can remove your PioneerDDJSX.softTakeoverEmulation function.
Yes, you do need engine.softTakeover for both engine.setValue and engine.setParameter. In Mixxx 2.0, there was a bug that made engine.softTakeover not work when engine.setParameter was used, but is fixed in the master branch and will be in Mixxx 2.1. Note you need to call engine.softTakeoverIgnoreNextValue when switching what the potentiometer controls, as documented on the wiki.
| offset = 0, | ||
| samplerIndex = 0; | ||
|
|
||
| //Roll Mode: |
There was a problem hiding this comment.
Inconsistent spacing in this section, use 4 spaces to indent.
There was a problem hiding this comment.
Sorry, that came from the recently made changed. I will correct that!
|
As I just read over this, should I replace |
If what you have is working for your purposes, you don't need to rewrite it. |
There was a problem hiding this comment.
Overall this looks quite good. Thanks for making it easy to review. :)
There will be things to change with the looping and effects controls when #1187 and #1211 are merged. You don't need to rewrite this using the Components library just to use the EffectUnit object from the library. If you choose to rewrite the rest using Components, it would make this enormous JS file more manageable.
|
|
||
| // show Sampler Load configuration file dialog: | ||
| if (PioneerDDJSX.samplerShowLoadDialog) { | ||
| script.toggleControl("[Sampler]", "LoadSamplerBank"); |
There was a problem hiding this comment.
Samplers persist across restarts now that #1213 has been merged, so remove this option.
| // switch on Effect Units as default: | ||
| for (var index in PioneerDDJSX.fxUnitGroups) { | ||
| if (PioneerDDJSX.fxUnitGroups.hasOwnProperty(index)) { | ||
| engine.setValue(index, "enabled", true); |
There was a problem hiding this comment.
EffectUnits are on by default, so this is unnecessary.
There was a problem hiding this comment.
To be save, it was implemented here, but you're right. I remove this.
|
|
||
| PioneerDDJSX.shutdown = function() { | ||
| PioneerDDJSX.setNonDeckSoftTakeover(false); | ||
| PioneerDDJSX.bindNonDeckControlConnections(false); |
There was a problem hiding this comment.
Mixxx takes care of this, so you can remove the above two lines.
There was a problem hiding this comment.
I already guessed something like that, it remained here to be save. But I remove these lines now.
| var fullValue = (PioneerDDJSX.highResMSB[group].filterHigh << 7) + value; | ||
| engine.setValue( | ||
| group, | ||
| "filterHigh", |
There was a problem hiding this comment.
The filterLow/Mid/High ControlObjects for EQs were deprecated in Mixxx 2.0 when the EQs were rolled into the effects system.
There was a problem hiding this comment.
I replace these ControlObjects by the new EqualizerRack1 objects.
| PioneerDDJSX.chFaderStart[channel] !== null | ||
| ) { | ||
| engine.setValue(group, "play", 0); | ||
| engine.setValue(group, "playposition", PioneerDDJSX.chFaderStart[channel]); |
There was a problem hiding this comment.
Neat! If I understand this correctly, you can hold shift and move the fader up briefly to use the deck like a short sample repeatedly?
There was a problem hiding this comment.
Yes, you're right ;)
|
|
||
| PioneerDDJSX.crossfaderCurveKnobLSB = function(channel, control, value, status, group) { | ||
| var fullValue = (PioneerDDJSX.highResMSB[group].crossfaderCurveKnob << 7) + value; | ||
| script.crossfaderCurve(fullValue, 0x00, 0x3FFF); |
There was a problem hiding this comment.
On some controllers I think the crossfader adjustment knob changes the MIDI signals the crossfader sends. I guess that isn't the case for this controller?
There was a problem hiding this comment.
Oh, I didn't consider that until now. I will check this!
There was a problem hiding this comment.
After checking this out, I can say that the crossfader adjustment knob doesn't change the MIDI signals, so the crossfader curve adjustment is completely up the Mixxx.
|
|
||
| PioneerDDJSX.needleSearchTouched = false; | ||
|
|
||
| PioneerDDJSX.needleSearchTouch = function(channel, control, value, status, group) { |
There was a problem hiding this comment.
When does this get executed? When touching the needle search? If so, I'm a bit confused about the purpose of the signal. Or is there a button to hold down while using the touch strip to prevent accidents?
There was a problem hiding this comment.
Yes, this gets executed if you touch the needle strip, so the control of the needle strip is separated in that needleSearchTouch and needleSearchStripPosition. It's the same implementation/structure as for the jogwheel controls.
There was a problem hiding this comment.
Hmm, okay. It may be helpful to add an option to require shift (or maybe the jog wheel?) to be pressed for needle searching to prevent accidents.
There was a problem hiding this comment.
I will implement an option for requiring shift to do needle search!
There was a problem hiding this comment.
Yeah, I could imagine someone accidentally hitting the touch strip while using the effects. That wouldn't be fun.
|
|
||
| PioneerDDJSX.playFromCuePoint = false; | ||
|
|
||
| PioneerDDJSX.playIllumination = function(value, group, control) { |
There was a problem hiding this comment.
I'm confused what this is doing. Have you tested it with different cue modes set in Mixxx's preferences?
There was a problem hiding this comment.
I will check this in different cue modes. The illumination control (LEDs inside the jogwheels) was not so easy to handle / to show me what I want to see.
There was a problem hiding this comment.
After checking the different cue modes set in the Mixxx's preferences, I can say that there's no problem concerning the illumination control/ jog wheel LEDs. The illumination control is independent of the cue mode. A better way would be to be able to control the jog wheel LEDs itself, but as already said that would take a lot of rework (without using "Serato Mode").
|
|
||
| /////////////////////////////////////////////////////////////// | ||
| // JOGWHEELS // | ||
| /////////////////////////////////////////////////////////////// |
There was a problem hiding this comment.
I don't see anything here for the spinning LED display in the jog wheel. Have you gotten that working? You can look at the Reloop Beatmix 2/4 mapping for an example.
There was a problem hiding this comment.
It is not possible to let the LEDs inside the jog wheel (illumination control) rotate backwards, there's no midi function for that (as far as I have seen). You can't control the LEDs separately on this controller and that's the main problem to show the correct turning/cue-position on this controller.
There was a problem hiding this comment.
I see in Pioneer's demo video that the LEDs can rotate backwards with Serato. Another video shows it working with Traktor. So it is possible, but Pioneer has not documented it. You will probably have to intercept the MIDI messages Serato sends to the controller with MIDI OX (Windows) or MIDI Monitor (Mac) to figure out what to send. You might have to turn off the setting to use the controller with DJ software other than Serato.
There was a problem hiding this comment.
It IS possible, but then I have to rework a lot of code, since not only the LEDs act differently in "Serato Mode". All PAD functionalities have to be controlled/interpreted differently and the MIDI messages are not documented. The mode "DJ Software other than Serato" makes it a lot more easier to handle in a first step. I want to rework the mapping concerning the jog LEDs, "Serato Mode" and PAD functionalities in the future, would that be ok? I don't know how fast I can "explore" all the necessary MIDI messages and how much time it costs to rework the pad functionalities.
There was a problem hiding this comment.
Hmmm, okay. How are the pad signals different? Does it only send one signal for each pad no matter which pad mode is selected?
There was a problem hiding this comment.
You might not need to redo the mapping. It is possible that sending the same MIDI signals for the jog LEDs that Serato sends still works in the non-Serato mode.
There was a problem hiding this comment.
You can't switch the PAD MODE in "Serato Mode" and also the pads in the default CUE PAD MODE don't work. Another thing I saw is that the vu meters act differently in "Serato Mode". I have to monitor the MIDI signals to see what happens there instead.
I will test sending MIDI signals for the jog LEDs in non-Serato mode.
I also found some code controlling the jog LEDs and sending serato-messages to the controller, I have to figure that out. It would be nice, if the LEDs act as I want them to.
There was a problem hiding this comment.
As a result of sending MIDI signals for the jog LEDs in non-serato mode, I can say that these LEDs won't light up :(. They only light up in serato-mode...so I have to figure a few MIDI signals out and do a little rework to the script and XML (to get PADs working in serato-mode).
|
On the wiki you mention that the crossfader assignment switches are not mapped. Do they send MIDI signals? If so, they should be mapped. Mixxx's crossfader assignment switches are not shown in any skin presently for the main decks (they are for the samplers). That will be fixed with #940. |
Yes, they send MIDI signals and I can map them, but I don't know how as I can't see a ControlObject on the wiki for that. How can I assign the crossfader? |
The ControlObject is called "orientation". |
| 'loadedDeck': 0x00, //deck 1-4: 0x00 - 0x03 | ||
| 'playPauseDeck': 0x0C, //deck 1-4: 0x0C - 0x0F | ||
| 'cueDeck': 0x10, //deck 1-4: 0x10 - 0x13 | ||
| 'djAppConnect': 0x09 |
There was a problem hiding this comment.
What does sending this MIDI message do? It does not look like you send it anywhere in the script. I am guessing that it asks the controller to send the positions of all the knobs and faders so their initial positions can be set in the software on startup. If so, you'll have to delay enabling soft takeover until after the first MIDI messages are received (the Components library does that automatically).
There was a problem hiding this comment.
This MIDI message is just listed here as it exists / is available. I tried it but could not see any reaction, so I don't send it in the script. It also doesn't ask the controller to send positions of knobs and faders to the software. Nothing happens sending it. I think I remove it from here.
There was a problem hiding this comment.
Hmm, maybe it does that in Serato mode? Controllers designed for Serato typically have a message that Serato sends them on startup to sync the software to the positions of the knobs and faders. When you're looking at the messages for the jog wheel LEDs, look for that message when Serato starts.
Thank you and SORRY that I didn't see this on the wiki!! I implement the crossfader assignment! |
No problem, it is not well named. I edited the description on the wiki so it should be easier to find if someone does a text search for "crossfader assign" in their browser. |
I will help you with that as soon as I've done all other "ToDos" for the controller mapping. |
|
By the way: As far as I have seen, the mapping will work for the DDJ-SX2, too (except the colored PAD lights). |
Nice. Hopefully someone will adapt this for the DDJ-SX2 after this is finished. |
…nted Sysex handshake and controls’ status (fader position etc.) to update Mixxx controls on init, Added user option to use SHIFT for controlling needle search, Optimizations concerning softTakeover, Implemented Slicer-functionalities and moved beatloops to group2, Optimizations concerning sampler velocity mode. Pioneer DDJ-SX.midi.xml: Sorted XML tags into a logical order
|
I'm back with a lot of changes....I reworked the wheel LEDs to react to play position, implemented a Sysex handshake and now all control positions are set according the actual positions on the controller during the init function AND implemented slicer functionalities. I have to update the wiki now. |
|
|
||
| // send init message to controller: | ||
| midi.sendSysexMsg(PioneerDDJSX.initSendMsg, PioneerDDJSX.initSendMsg.length); | ||
| PioneerDDJSX.initTimer = engine.beginTimer(1000, "PioneerDDJSX.initTimerTick()"); |
There was a problem hiding this comment.
Why not just use 5000 as timer length and remove the increment/if >5 from initTimerTick() ?
There was a problem hiding this comment.
Sometimes you don't see the easier way... I will change that.
There was a problem hiding this comment.
Why is this necessary? Does the controller ever fail to send back the sysex message to complete the handshake?
There was a problem hiding this comment.
What I realized is that when I start Mixxx, the handshake (send syses message to controller and controller sends syses message back) works safe/reliable, but when I update the *.js file when Mixxx has already started, the handshake works only sometimes and it seems that the controller doesn't send back anything, but I don't know why. That's why I integrated this timer thing. What is the difference between the init during Mixxx startup and the init when js-file is being updated when Mixxx has already started??
There was a problem hiding this comment.
That's strange. As far as I know the only difference is that updating the JS file executes the shutdown function before calling init again.
There was a problem hiding this comment.
Ok, I will do another test later. Serato doesn't send anything to the controller on shutdown. So I will comment out everything what happens in the shutdown function and do a few tests starting/closing Mixxx and updating *.js-File when Mixxx has already started. But I don't think that will change anything as I only send usual messages (like during playback for example) to the controller on shutdown. I only reset the LEDs.
There was a problem hiding this comment.
I found out now or I think that it's a timing issue between the shutdown and the init, that updating js-file causes the controller not to respond to the syses message. There are a lot of midi messages in the shutdown to reset all LEDs. Serato doesn't reset anything at all on shutdown. So....should I simplify the script by deleting the timer and the LED resets inside the shutdown function (to what I tend to at the moment) or should I leave it?
There was a problem hiding this comment.
As I deleted the handshake, the shutdown function is no problem now. The LEDs will be reset during shutdown and on updating the js-file the MIDI message to get the controls status works.
| 0x05, 0x09, 0x0F, 0x0F, 0x01, 0x08, 0x00, 0x09, 0xF7 | ||
| ]; | ||
| PioneerDDJSX.initRecvMsg = [ | ||
| 0xF0, 0x00, 0x20, 0x7F, 0x01, 0x02, 0x01, 0x02, 0x12, |
There was a problem hiding this comment.
Are you sure this message is independent of firmware revision or controller serial number ?
There was a problem hiding this comment.
It's independent of the serial number, because I tested two different controllers of the same type. Firmware version....I'm not really sure, I just saw that every hex value after the first 7 changes every time serato starts, so I think it has something to do with timestamp and checksum.
There was a problem hiding this comment.
Humm, these are Serato Sysex Messages (00 20 7F is Serato identifier), so it should only work with DDJ-SX in Serato Mode.
I think DDJ-SX can be switched to other modes, so probably in these other modes, I will not work with these serato sysex messages.
Maybe at least document on the wiki that this mapping is only designed for DDJ-SX in Serato mode.
There was a problem hiding this comment.
Yes, this was my intention...the controller must be in serato mode now (because of the wheel LEDs and PADs! I already wrote that I have to update the wiki. I will do that tomorrow.
There was a problem hiding this comment.
I'm glad you got it working in Serato mode so users don't have to go through the setup process.
There was a problem hiding this comment.
There came something to my mind concerning the handshake and the midi message to get the controller pot/knob states: I tested the init without the Serato sysex message and found out that it doesn't make any difference if I send this and receive this message or not. So the handshake during the init is just a verification that the correct controller is connected and not really necessary. Should I keep this handshake thing or should I delete it to simplify the script?
There was a problem hiding this comment.
The changes mostly look good. I'm glad you got the jog wheel LEDs and initializing the potentiometer positions to work. I'll take a closer look at the slicer code after you update the wiki.
The updates for the EffectUnit object in the Components library have been merged. This will make it easy to map the effects sections to take full advantage of the new effects UI once you merge the latest updates from master into this branch. Documentation is on the wiki. If you have any questions about using the library, feel free to ask. Refer to the Pot documentation for how to map both the MSB and LSB messages for the effects knobs. The example code in the EffectUnit documentation defines midi attributes for the knobs, but that is not strictly necessary because they don't send MIDI output and the input signals are handled by the XML. After implementing a way to register MIDI input handlers from JS, the midi attributes for Pot Components will be needed, but that won't happen for Mixxx 2.1.
| PioneerDDJSX.useNewLibraryControls = false; //default: false | ||
|
|
||
| // If true, SHIFT has to be pressed to activate needle search control | ||
| PioneerDDJSX.needleSearchShiftEnable = false; //default: false |
There was a problem hiding this comment.
The default is right there, so there's no need to repeat it in a comment.
There was a problem hiding this comment.
Ok I delete the default comments.
There was a problem hiding this comment.
Or you could move them to the line above in case someone forgot what the default was after changing it and wants to change it back.
There was a problem hiding this comment.
That's a good idea!
| trackLoaded = engine.getValue(group, "track_loaded"); | ||
| if (!trackLoaded) { | ||
| return; | ||
| if (value >= 0) { |
There was a problem hiding this comment.
playposition can go below 0 when seeking before the beginning of the track.
There was a problem hiding this comment.
I read that but it doesn't make any sense to me to let the wheel LEDs rotate when seeking before the beginning of the track. Not good?
There was a problem hiding this comment.
Yes, it does make sense. If you look at the waveform on screen it shows a position before the track, so the jog wheel should match that.
There was a problem hiding this comment.
Ok I will change and test it!
|
|
||
| // send init message to controller: | ||
| midi.sendSysexMsg(PioneerDDJSX.initSendMsg, PioneerDDJSX.initSendMsg.length); | ||
| PioneerDDJSX.initTimer = engine.beginTimer(1000, "PioneerDDJSX.initTimerTick()"); |
There was a problem hiding this comment.
Why is this necessary? Does the controller ever fail to send back the sysex message to complete the handshake?
| engine.connectControl("[Channel4]", "VuMeter", "PioneerDDJSX.VuMeterLeds", !bind); | ||
| }; | ||
|
|
||
| PioneerDDJSX.setDeckSoftTakeover = function(channel, enable) { |
There was a problem hiding this comment.
It looks like this is only executed if the initialization handshake fails. Is that correct?
There was a problem hiding this comment.
Yes, this is correct. But I think I'm going to remove the handshake and the setDeckSoftTakeover, too (because the pot/knob states are initialized by the controller and have to be set by the controller).
| PioneerDDJSX.initAfterHandshake = function(status) { | ||
| // initiate control status request: | ||
| if (status) { | ||
| midi.sendShortMsg(0x9B, 0x08, 0x7F); |
There was a problem hiding this comment.
Please add a comment somewhere explaining the initialization process. If I understand correctly, it goes like:
- Software sends initial sysex message
- Controller sends a different sysex message back
- Software sends this normal MIDI message
- Controller sends positions of all knobs and faders
Other Pioneer controllers probably follow the same process, so it would be helpful to have this documented.
There was a problem hiding this comment.
Is it necessary to send the sysex message? Could you just send 0x9B, 0x08, 0x7F?
There was a problem hiding this comment.
Did you ever figure out what the "DJ App Connect" MIDI message does (if anything)? I edited the DDJ-SB2 mapping to send that in the init function in #1243 but I didn't get a chance to test what it actually does.
There was a problem hiding this comment.
I found out that sending the sysex message isn't necessary. Important is just to send 0x9B, 0x08, 0x7F. I also tried sending this "DJ App Connect" MIDI message, but it doesn't seem to do anything. I have no idea for what it is for. I logged every message Serato sends but this message does never appear.
There was a problem hiding this comment.
Okay, thanks for testing. I'll update the DDJ-SB2 mapping in #1243 to send that same message. Hopefully it works for that controller too, but I no longer have access to a DDJ-SB2 to test.
There was a problem hiding this comment.
A friend of mine has this controller. When there's time, I could test it with him.
| @@ -1776,20 +2008,21 @@ PioneerDDJSX.hotCueLeds = function(value, group, control) { | |||
|
|
|||
| PioneerDDJSX.VuMeterLeds = function(value, group, control) { | |||
There was a problem hiding this comment.
I don't see any function connected to [Master], VuMeter. Are the master meters controlled by the hardware in response to the sound card output?
There was a problem hiding this comment.
Yes, the "Master" vu meters are controlled by the hardware.
There was a problem hiding this comment.
Okay, please add a comment in the code making note of that.
| value = 0x76; | ||
| } | ||
|
|
||
| if (engine.getValue(group, "PeakIndicator")) { |
There was a problem hiding this comment.
Does this make it so the red LEDs only light up when the deck is clipping? That is how it should be.
There was a problem hiding this comment.
Yes, exactly. This is an effect I realized when switching from "DJ software other than Serato"-mode to "Serato"-mode.
There was a problem hiding this comment.
Okay, good. I added a new guideline to the Contributing Mappings wiki page to clarify this because when I tested the Pioneer DDJ-SB2 mapping recently I discovered the scaling of the controller's level meters did not match Mixxx, which defeated the purpose of having level meters on the controller.
| PioneerDDJSX.toggledBrake = [false, false, false, false]; | ||
| PioneerDDJSX.scratchMode = [true, true, true, true]; | ||
| PioneerDDJSX.wheelLedsBlinkStatus = [0, 0, 0, 0]; | ||
| PioneerDDJSX.padMode = [0, 0, 0, 0]; //0 = HOTCUE, 1 = ROLL, 2 = SLICER, 3 = SAMPLER |
There was a problem hiding this comment.
Make an object to name each of these modes like an enum in C or C++ so there are not magic numbers in the code:
PioneerDDJSX.padModes = {
hotcue: 0,
roll: 1,
slicer: 2,
sampler: 3,
}
PioneerDDJSX.activePadMode = [PioneerDDJSX.padModes.hotcue, PioneerDDJSX.padModes.hotcue, PioneerDDJSX.padModes.hotcue, PioneerDDJSX.padModes.hotcue];
There was a problem hiding this comment.
You're right. I will change that.
| startPos = 0, | ||
| beatsToJump = 0; | ||
|
|
||
| if (PioneerDDJSX.slicerType[deck] === 1) { |
There was a problem hiding this comment.
Use an object to name the possible values for slicerType so there are not magic numbers in the code.
| } | ||
| if (startPos >= Math.abs(beatsToJump) && PioneerDDJSX.slicerPreviousBeatsPassed[deck] !== PioneerDDJSX.slicerBeatsPassed[deck]) { | ||
| PioneerDDJSX.slicerPreviousBeatsPassed[deck] = PioneerDDJSX.slicerBeatsPassed[deck]; | ||
| if (Math.abs(beatsToJump) > 0) { |
There was a problem hiding this comment.
Why can't this jump backwards?
There was a problem hiding this comment.
Whoops, I glanced over the Math.abs. But this check is not necessary because setting the beatjump Control to 0 does not do anything.
There was a problem hiding this comment.
I was not sure about what happens if I call beatjump with 0. Then I can remove the if-clause here.
There was a problem hiding this comment.
Actually, keep it. #1187 will change the beatjump Control so it updates beatjump_size, and setting it to 0 would make the beatjump_size spinbox show 1/32.
|
It is easier to review and keep track of changes if you make a smaller Git commit for each change. If you have already made unrelated changes without making a new commit, a GUI Git client is helpful to select only specific changes for each commit. We have a list of some GUI Git clients on the wiki. Personally I prefer Git Cola. |
| if (!trackLoaded) { | ||
| return; | ||
| if (value >= 0) { | ||
| wheelPos = 0x01 + ((speed * (value * duration)) % 0x48); |
There was a problem hiding this comment.
speed and 0x48 look like magic numbers. Is there any math behind these numbers, or did you just guess and check until it looked like it worked right?
For the clarity of the code, it would be helpful to have an intermediate variable for value * direction called something like playPositionInSeconds.
There was a problem hiding this comment.
Refer to the Reloop Beatmix 2/4 mapping code for an example of how to make the code clearer.
There was a problem hiding this comment.
The Reloop Beatmix 2/4 mapping calculates the position of the jog LED so it rotates at 33 1/3 RPM like a turntable, which is also how the jog scratching is set up.
There was a problem hiding this comment.
speed and 0x48 are no magic numbers. speed represents the turntable rotation (33 1/3 RPM), it is calculated on paper before. I will change the code so you will see the calculation according scratch RPM parameter. 0x48 is the maximum hex value/midi message value representing all LEDs are on (one complete wheel turn). I will add intermediate variables to make the calculation visible and the code clearer.
Ok, as my branch is 363 commits behind the master branch and the midi-components-0.0.js file is in an very old state, what is the right way to update this file in my controller branch so it has the same state as the master? After that I could make the change I think. |
…o. I didn’t see this.
|
Almost done! Now that the effects controls can be toggled between units 1/3 and 2/4 it would be good to have a way to assign decks to units 3 & 4. The most obvious way would be shift + effect unit routing buttons at the top of the mixer columns. However, it would be distracting to see those LEDs flash every time the shift button is pressed for an unrelated reason. How about repurposing the Panel Select button to act as a shift button for the effect unit assignment buttons? The version of the Deere skin currently in the master branch can't show effect units 3 & 4 (that will be changed with #940), but the new Tango skin does. With the CueButton object in Components, I made shift + cue jump to the beginning of the track and stop (using the |
Merge the master branch first: |
Ok, I did that already. What's next? How do I get the components file from the master to the mapping branch? Or can I change it in my forked master branch? |
Merging the master branch into your mapping branch does this. Then edit the file on your mapping branch and commit it. |
I don't know....I think that this solution would not be very intuitive and the Panel Select button already has another functionality. I would rather take the obvious solution using shift. Would that be ok, too? I don't see any better way at the moment.
Hm...I like the brake effect and I want to use it but I don't know where else to put it at the moment. I understand you and I think it should be an effect, too (like in VirtualDJ I think). So if I replace the SHIFT+CUE functionality now I cannot use the brake effect any more. But it would be better if SHIFT+CUE leads to the beginning of the track. |
…usChooseMode in the definition of the effectFocusButton to use this.on and this.off values (in order to guarantee that the controller button led will light up).
Ah sorry, there's a mistake in the wiki. I correct that. SHIFT+CUE toggles spinback effect at the moment, not brake. But it's the same problem....I think I change SHIFT+CUE to jump to the beginning of the track. |
…the beginning of the track and stop.
I think you'd find it quite distracting to see the lights on the effect unit routing buttons switch when you press shift to access a shifted functionality on the decks.
How about moving the current Panel button functionality to shift + Panel and using Panel without shift for the effect unit 3 & 4 assignment layer? |
| }; | ||
| this.output = function (value, group, control) { | ||
| this.send(value === this.number); | ||
| this.send((value === this.number) ? this.on : this.off); |
There was a problem hiding this comment.
Thanks for figuring this out!
|
Now that #1187 has been merged, update this to make use of the new Controls. How about: |
… EffectUnit 3 and 4 by pressing SHIFT + PANEL SELECT. Also button LEDs will light up according EffectUnit assignment (to see assignment of EffectUnit 3 and 4 SHIFT + PANEL SELECT must be pressed).
… CUE Mode parameter buttons to jump beats according beatjump_size and shift + parameter buttons to change the beatloop_size. Fixed Led reset function concerning fx button leds as EffectUnits 3 and 4 were added in the previous commit.
|
The updates to the Deere skin in #940 that make use of the new loop controls were merged to master, so you can now use the latest master build to update the mapping. |
…4 assignment only when SHIFT and PANEL SELECT is pressed.
|
@Be-ing : Sorry that it was so silent here in the last weeks but now I found time to implement nearly everything you suggested. I left the panel select functionality as it is and added the EffectUnit 3 and 4 assignment to SHIFT + PANEL SELECT + FX1/2. Concerning the loop functionalities I left the SHIFT + AUTO LOOP function as it is (activate current loop) and didn't replace it by beatlooproll_activate as this would confuse users (perhaps) because it doesn't match the labeling of the button. |
Be-ing
left a comment
There was a problem hiding this comment.
Thanks for the updates. By the way, if you did not notice, you can now hold loop_in/out while a loop is active and move the jog wheel to adjust the loop in/out point.
| engine.setValue(group, "beatloop_" + PioneerDDJSX.selectedSlicerQuantization[deck] + "_activate", value); | ||
| engine.setValue(group, "slip_enabled", value); | ||
| engine.setValue(group, "beatloop_size", PioneerDDJSX.selectedSlicerQuantization[deck]); | ||
| engine.setValue(group, "beatloop_activate", value); |
There was a problem hiding this comment.
This change wasn't necessary because beatloop_X_activate changes beatloop_size to X, but there's no need to change it back.
| engine.setValue(group, "reloop_toggle", true); | ||
| } else { | ||
| engine.setValue(group, "beatloop_" + 4 + "_toggle", true); | ||
| engine.setValue(group, "beatloop_activate", true); |
There was a problem hiding this comment.
beatloop_activate should be reset to 0 on button up or the button on screen will stay lit. reloop_toggle should also be reset to 0 (the button in skins lights up with loop_enabled, but just in case a skin doesn't do that, the controller script shouldn't leave reloop_toggle stuck at 1).
There was a problem hiding this comment.
It's confusing that there are multiple buttons for reloop_toggle. However, it would also be redundant to use shift and this button for beatlooproll_activate because of the ROLL pad mode, so I'm not sure it matters much either way...?
|
|
||
| PioneerDDJSX.loopActiveButton = function(channel, control, value, status, group) { | ||
| script.toggleControl(group, "reloop_exit"); | ||
| script.toggleControl(group, "reloop_toggle"); |
There was a problem hiding this comment.
As I realized with #1271, toggling Controls on each button down/up event can cause strange situations if the Control does not start in the expected state. For example, if reloop_toggle happens to be 1 for whatever reason when the button is pressed, it will be set to 0 on button down and reset to 1 on button up, which will not produce the expected behavior. It's better to set the Control to 1 on button press and 0 on button release, regardless of the state of the Control before the button was pressed.
|
|
||
| PioneerDDJSX.shiftPanelSelectButton = function(channel, control, value, status, group) { | ||
| var channelGroup; | ||
| PioneerDDJSX.shiftPanelSelectPressed = value ? true : false; |
There was a problem hiding this comment.
If I understand correctly, you need to hold shift, hold panel, and press an effect assignment button to toggle assignment for effect units 3 & 4? Don't you need 3 hands for that? This is why I suggested moving the old Panel functionality to shift + Panel, so you could hold the Panel button with one hand and press the effect unit assignment button with the other hand.
There was a problem hiding this comment.
You understood that correctly, but it's very easy to hold shift and panel select with one hand. In either way you need two hands to assign EffectUnit 3 and 4. I can put my thumb onto the shift button and my forefinger onto the panel select easily while pressing the assignment buttons with the other hand. I have chosen this way because then the original panel select function can be maintained and I think it's more intuitive to activate/deactivate panels (samplers, effects) if you don't press shift (as the button is only labeled PANEL SELECT).
There was a problem hiding this comment.
Okay, I don't have a DDJ-SX to try this with, so I'll take your word for it. I think this will be ready for merge after addressing the comments about the loop related buttons below.
I have problems figuring this out: If I hold loop_in the loop in marker moves according the play position. So if I hold it long enough without touching the jog wheel, the loop length gets 0 and the loop in marker moves with the play position. Is that the desired behaviour? @Be-ing: Can you give me a few details how this is intended to be used? |
… not to use script.toggleControl but engine.setValue and reset parameter value on button release. Also changed button led behavior to react to loop_enabled.
| if (engine.getValue(group, "loop_enabled")) { | ||
| engine.setValue(group, "reloop_toggle", value); | ||
| } else { | ||
| engine.setValue(group, "beatloop_activate", value); |
There was a problem hiding this comment.
This will not work as expected. Setting beatloop_activate to 1 sets loop_enabled to 1, so on button release this will set reloop_toggle to 0 instead of beatloop_activate, so the beatloop_activate button will stay lit on screen. Try this:
if (value) {
if (engine.getValue(group, "loop_enabled")) {
engine.setValue(group, "reloop_toggle", true);
engine.setValue(group, "reloop_toggle", false);
} else {
engine.setValue(group, "beatloop_activate", true);
engine.setValue(group, "beatloop_activate", false);
}
}
There was a problem hiding this comment.
You're right, I didn't see that. But it's strange because it's working as it should. That's why I didn't see it. I will try your solution.
There was a problem hiding this comment.
Both code parts are working, but yours is correct.
I also realized another "maybe bug" which has nothing to do with my controller mapping, as it doesn't work on the skin as well. If I load a track and do a beatloop it stays marked in the waveform at the next start of Mixxx. But at this next start I could't do the reloop_toggle. I have to make a new beat loop to be able to use reloop_toggle. I could load other tracks and reload that track with the beat loop again and the reloop_toggle is working. But on the next start of Mixxx it isn't working anymore on this track with this previously created beat loop until I make a new beat loop. Before reloop_toggle I used reloop_exit and it worked in that situation.
There was a problem hiding this comment.
Ok, I just found out that I have to load a track twice to get the reloop_toggle working on a track with a previously created beat loop but that's strange I think.
There was a problem hiding this comment.
Ah ok, thank you for the explanations!
That is working as intended. Holding the button down moves the loop in/out marker as the playhead moves. So holding the button while moving the jog wheel moves the marker, as well as holding the button while the track is playing. |
|
Yay! LGTM! Thank you for contributing the first complete Mixxx mapping for a popular high end controller. :) I'm sure many people will be grateful for this. I hope I get a chance to play with it sometime. @daschuer, could you merge this? |
|
Yeah! Thank you very much for your support @Be-ing ! After this mapping is merged I hope I can get m y hands on the SX2 sometime and do a mapping for this controller, too (nearly everything can be adapted from SX mapping). |
|
ping @daschuer: ready for merge? |
|
Thank you for all the work! |
First final version of a controller mapping for the Pioneer DDJ-SX controller.