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-SR2: Initial controller mapping #13429

Open
wants to merge 19 commits into
base: 2.4
Choose a base branch
from

Conversation

WaylonR
Copy link
Contributor

@WaylonR WaylonR commented Jul 1, 2024

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

plenty of comments for now. Feel free to ask questions.

res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
shiftOffset: 8,
number: i + 1,
group: deck.currentDeck,
on: this.color,
Copy link
Member

Choose a reason for hiding this comment

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

this probably doesn't do anything if the colormapper takes preference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, that's odd. I set this.color = DDJSR2.PadColor.OFF; .... and uhh.. it turned off ALL the pads and the mode button leds. Thought 'this' was local to DDJSR2.HotcueMode function? In other words, has some relevance. on: this.color removed. Done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed "on: this.color" no change.. but a bug that occurs occasionally, white hotcues, where there are no hotcues present in the track, still happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@Swiftb0y Swiftb0y Sep 2, 2024

Choose a reason for hiding this comment

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

Removed "on: this.color" no change.. but a bug that occurs occasionally, white hotcues, where there are no hotcues present in the track, still happens.

Does this still happen in the current version?

@Swiftb0y Swiftb0y changed the base branch from main to 2.4 July 2, 2024 13:50
@WaylonR
Copy link
Contributor Author

WaylonR commented Aug 14, 2024

A lot of progress done, a bit more to do, before a pre-commit check and push, but not much. Maybe a week.

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

a couple more comment, notice that there are also another batch of open comments on the previous review
image
I also prefer to resolve review comments myself because its often the case that a review comment is not understood or a request not properly executed so I want to make sure myself that they are actually resolved.

res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
DDJSR2.scratchSettings = {
"alpha": 1.0 / 8,
"beta": 1.0 / 8 / 32,
"wheelResolution": 20000,
Copy link
Member

Choose a reason for hiding this comment

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

20000? that doesnt seem right... The wheelresolution is supposed to be how ticks there are per revolution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needs calibration. 50000 seems to be correct, where a 360 rotation of the wheel is the same as a 360 rotation in the Spinning Vinyl onscreen. Is this not the intended purpose? 1000 has it skipping through the track very quickly.

Copy link
Member

Choose a reason for hiding this comment

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

Needs calibration. 50000 seems to be correct, where a 360 rotation of the wheel is the same as a 360 rotation in the Spinning Vinyl onscreen. Is this not the intended purpose?

Yes that is usually the intended purpose. A one-to-one mapping between the onscreen vinyl sticker and the physical one (in theory if there was one). I'm just confused because usually the number if ticks per rotation is much lower (~1000 on highend controllers) so either the calculations in the mapping are wrong or pioneer is cheating again because I don't think their jogwheel is actually that high-res.

res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

couple more comments, this is taking shape

res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

Please do me the favor and convert the remaining couple "plain" handlers to components as well.

res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
@WaylonR
Copy link
Contributor Author

WaylonR commented Aug 28, 2024

Please do me the favor and convert the remaining couple "plain" handlers to components as well.

Which plain handlers? the ones that are still "plain" have complicated functions, that can't be simply put into a inKey... would be more code, wrapping a component around it, only to put the function in input: .. unless I'm missing a few simple ones.

res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
Comment on lines 1316 to 1319
// Determine the current position in the color cycle
// The modulo operation ensures the color value stays within the available range
// The multiplication by maxColorValue ensures that the position is spread over the entire range of indexed colors.
colorIndex = Math.round((revolutionsPerSecond * elapsedTime * maxColorValue) % maxColorValue);
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is what you want? This goes through the entire range of colors within a single revolution (~1.8 seconds). That seems seizure inducing to me. We also have the rule that controller mappings should avoid blinking lights unless immediate attention is required (because they would likely be distracting) so I'm not convinced this feature adds value. The controller should not be in "vegas mode" as part of regular operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is optional. Can set the mapping to colors, or track, in the PR, but for those who want it, it's available.

Copy link
Member

Choose a reason for hiding this comment

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

sure... so then lets simplify the code a bit. I'm just making sure that high-frequency color changing behavior is actually what you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

set the decks in the PR to track mode by default?

Copy link
Member

Choose a reason for hiding this comment

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

yes, I think it should be that by default as well.

res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
res/controllers/Pioneer-DDJ-SR2-scripts.js Outdated Show resolved Hide resolved
Comment on lines 1284 to 1293
if (remainingTime > 0 && remainingTime < 30 && !engine.isScratching(this.deckNumber)) {
//increase blinking according time left
const blinkInterval = Math.max(remainingTime / 3, 3);
if (this.blinkStatus < blinkInterval) {
colorIndex = DDJSR2.wheelLedCircle.blink;
} else if (this.blinkStatus > (blinkInterval - parseInt(6 / blinkInterval))) {
this.blinkStatus = 0;
colorIndex = DDJSR2.wheelLedCircle.maxVal;
}
this.blinkStatus++;
Copy link
Member

Choose a reason for hiding this comment

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

the midi mapping reference mentions an "End Warning" (midi: [0x9B, 0x04 + channelOffset]). Why don't you just use that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried it. seems like it does nothing. The jog ring goes blank.

Copy link
Member

Choose a reason for hiding this comment

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

thats weird... you could try reverse-engineering it by looking at the traffic from/to serato. Have you tried sending all possible values or just specific ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the 1hz to 5hz end warning will do fine. On all jog modes, unless it's 'off'?

Copy link
Member

Choose a reason for hiding this comment

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

takes a little extra effort/complexity, but I'll think about it.

Copy link
Member

Choose a reason for hiding this comment

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

having multiple separate components controlling the same LED doesn't make sense, they will just step over each other when they can't communicate. And if they can communicate (aka depend on each other), there is little reason to make them separate components in the first place. Before I continue digging into the code here, I need to know whether the endWarning LED message actually works. You previously said that it just turns of the jogwheel LED, which I took as the feature being essentially broken, is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The midi in the mapping document does nothing, as far as I know, So I have to use the endWarning function I made, to change the color number, before it's sent to the controller. Should I push what I've got so far, even though it doesn't work for track_color?

Copy link
Member

Choose a reason for hiding this comment

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

sure, though judging by the buggy behavior you described, its probably more flawed than you think...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, code works for POSITION mode only. Just changes the color that gets send() if it's end_of_track.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, thats what I was afraid of. Try the approach I just posted in the separate review.

res/controllers/Pioneer-DDJ-SR2-scripts.js Show resolved Hide resolved
Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

try this. this overlays the end_of_track warning in a way that doesn't rely on the frequent updates of position. That way it should work correctly in the static and the track_color mode.

Comment on lines 1250 to 1317
const WheelRing = function(options) {
if (options.color === "POSITION") {
options.outKey = "playposition";
options.output = function(value) { this.sendIfChanged(this.getPositionColor(value)); };
} else if (options.color === "TRACK") {
this.endWarning(options.color);
options.outKey = "track_color";
options.output = function(colorRGB) { this.sendIfChanged(this.colorMapper.getValueForNearestColor(colorRGB)); };
} else {
options.trigger = function() {
this.sendIfChanged(DDJSR2.wheelLedCircleColor[options.color]);
};
}

components.Component.call(this, options);
};

WheelRing.prototype = new components.Component({
blinkStatus: 0,
colorMapper: DDJSR2.wheelColorMap,
sendIfChanged: function(value) {
value = this.endWarning(value);
if (this.value !== value) {
this.value = value;
this.send(value);
}
},
endWarning: function(colorIndex) {

// Check if we're nearing the end of the track and not scratching
if (engine.getValue(this.group, "end_of_track") && !engine.isScratching(this.deckNumber)) {
const framesPerHalfCycle = 30; // 1 second cycle, split into two half-cycles (on/off)

// Toggle between blink and maxVal states based on the half-cycle frame count
if (this.blinkStatus < framesPerHalfCycle) {
colorIndex = DDJSR2.wheelLedCircle.blink; // Blink state for half the cycle
} else if (this.blinkStatus < framesPerHalfCycle * 2) {
colorIndex = DDJSR2.wheelLedCircle.maxVal; // MaxVal state for the other half
} else {
this.blinkStatus = 0; // Reset blink status at the end of the full cycle
}
if (this.color === "TRACK") {
this.send(colorIndex);
}
this.blinkStatus++; // Increment frame counter
} else {
// Reset blink state when not at the end of the track
this.blinkStatus = 0;
}

return colorIndex;
},
getPositionColor: function(position) {
let colorIndex = DDJSR2.wheelLedCircle.maxVal;
// Every time the playposition changes, update the wheel color.
// Timing calculation is handled in seconds!!
const duration = engine.getValue(this.group, "duration");
const elapsedTime = position * duration;
const revolutionsPerSecond = DDJSR2.scratchSettings.vinylSpeed / 60;
const maxColorValue = DDJSR2.wheelLedCircle.maxVal;
// Determine the current position in the color cycle
// The modulo operation ensures the color value stays within the available range
// The multiplication by maxColorValue ensures that the position is spread over the entire range of indexed colors.
colorIndex = Math.round((revolutionsPerSecond * elapsedTime * maxColorValue) % maxColorValue);

return colorIndex;
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const WheelRing = function(options) {
if (options.color === "POSITION") {
options.outKey = "playposition";
options.output = function(value) { this.sendIfChanged(this.getPositionColor(value)); };
} else if (options.color === "TRACK") {
this.endWarning(options.color);
options.outKey = "track_color";
options.output = function(colorRGB) { this.sendIfChanged(this.colorMapper.getValueForNearestColor(colorRGB)); };
} else {
options.trigger = function() {
this.sendIfChanged(DDJSR2.wheelLedCircleColor[options.color]);
};
}
components.Component.call(this, options);
};
WheelRing.prototype = new components.Component({
blinkStatus: 0,
colorMapper: DDJSR2.wheelColorMap,
sendIfChanged: function(value) {
value = this.endWarning(value);
if (this.value !== value) {
this.value = value;
this.send(value);
}
},
endWarning: function(colorIndex) {
// Check if we're nearing the end of the track and not scratching
if (engine.getValue(this.group, "end_of_track") && !engine.isScratching(this.deckNumber)) {
const framesPerHalfCycle = 30; // 1 second cycle, split into two half-cycles (on/off)
// Toggle between blink and maxVal states based on the half-cycle frame count
if (this.blinkStatus < framesPerHalfCycle) {
colorIndex = DDJSR2.wheelLedCircle.blink; // Blink state for half the cycle
} else if (this.blinkStatus < framesPerHalfCycle * 2) {
colorIndex = DDJSR2.wheelLedCircle.maxVal; // MaxVal state for the other half
} else {
this.blinkStatus = 0; // Reset blink status at the end of the full cycle
}
if (this.color === "TRACK") {
this.send(colorIndex);
}
this.blinkStatus++; // Increment frame counter
} else {
// Reset blink state when not at the end of the track
this.blinkStatus = 0;
}
return colorIndex;
},
getPositionColor: function(position) {
let colorIndex = DDJSR2.wheelLedCircle.maxVal;
// Every time the playposition changes, update the wheel color.
// Timing calculation is handled in seconds!!
const duration = engine.getValue(this.group, "duration");
const elapsedTime = position * duration;
const revolutionsPerSecond = DDJSR2.scratchSettings.vinylSpeed / 60;
const maxColorValue = DDJSR2.wheelLedCircle.maxVal;
// Determine the current position in the color cycle
// The modulo operation ensures the color value stays within the available range
// The multiplication by maxColorValue ensures that the position is spread over the entire range of indexed colors.
colorIndex = Math.round((revolutionsPerSecond * elapsedTime * maxColorValue) % maxColorValue);
return colorIndex;
}
});
const WheelRing = function(options) {
components.Component.call(this, options);
}
WheelRing.prototype = new components.Component({
send: function(value) {
if (this.value !== value) {
this.value = value;
components.Component.send.call(this, value);
}
},
outputMaybe: function() {
if (engine.getValue(this.group, "end_of_track") && engine.getValue("[App]", "indicator_500ms")) {
this.send(this.off);
return;
}
components.Component.output.call(this, this.outGetValue(), this.group, this.outKey);
},
trigger: function() {
this.outputMaybe();
},
connect: function() {
if (typeof this.group !== "string") {
throw `invalid group: ${this.group}`;
}
const COs = [, {group: this.group, key: "end_of_track"}, {group: "[App]", key: "indicator_500ms"}];
if (typeof this.outKey === "string") {
COs[0] = {group: this.group, key: this.outKey};
}
this.connections = COs.map(co => engine.makeConnection(co.group, co.key, this.outputMaybe.bind(this)));
}
});

Comment on lines 496 to 500
this.wheelRing = new WheelRing({
midi: [0xBB, 0x20 + channelOffset],
color: DDJSR2.wheelColor[deckNumber],
deckNumber: deckNumber,
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.wheelRing = new WheelRing({
midi: [0xBB, 0x20 + channelOffset],
color: DDJSR2.wheelColor[deckNumber],
deckNumber: deckNumber,
});
switch (DDJSR2.wheelColor[deckNumber]) {
case "TRACK":
this.wheelRing = new WheelRing({
midi: [0xBB, 0x20 + channelOffset],
group: this.group,
outKey: "track_color",
output: function(colorRGB) { this.sendIfChanged(this.colorMapper.getValueForNearestColor(colorRGB)); };
});
break;
case "POSITION":
this.wheelRing = new WheelRing({
midi: [0xBB, 0x20 + channelOffset],
group: this.group,
outKey: "position",
revolutionsPerSecond: DDJSR2.scratchSettings.vinylSpeed / 60;
output: function(position) {
// Every time the playposition changes, update the wheel color.
// Timing calculation is handled in seconds!!
const elapsedTime = position * engine.getValue(this.group, "duration");;
// Determine the current position in the color cycle
// The modulo operation ensures the color value stays within the available range
// The multiplication by maxColorValue ensures that the position is spread over the entire range of indexed colors.
this.send(Math.round((this.revolutionsPerSecond * elapsedTime * maxColorValue) % maxColorValue));
}
});
break;
default:
this.wheelRing = new WheelRing({
midi: [0xBB, 0x20 + channelOffset],
group: this.group,
outValueScale: () => DDJSR2.wheelColor[deckNumber]
});
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a new update to the wheelRing code, to leverage the 500ms timer, and ensure that the endWarning occurs even in TRACK mode. Also fixes the jog wheel color on load of a new track.

Copy link
Member

Choose a reason for hiding this comment

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

can you elaborate on why you chose what you pushed over what I proposed? I think what you pushed is much less clean...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it didn't work. kept getting messages about .call'ing on undefined, for components.Component.output.call and send.call, couldn't work out why it wasn't working. Knew my code worked, just needed the TRACK mode working, and it synced with 500ms, you provided the glue for that.

Copy link
Member

Choose a reason for hiding this comment

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

components.Component.output.call and send.call,

yeah, should have been components.Component.prototype.output.call. my bad.

Copy link
Member

Choose a reason for hiding this comment

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

primarily that the concerns are more cleanly separated. The stock wheelring is the component that implements the end_of_track warning logic. If you were to add another mode, you can just create another customized instance of the wheelring and you get the end_of_track logic for free. See the open-closed principle for more. In addition there are also a couple little things:

  1. indicator_500ms is used wrong. You should respond to the actual value of the CO (1 is on, 0 is off) instead of tracking the state yourself (the .blink property you introduced). Otherwise depending on the exact timing, you may get a wheelrings that blink in phase and other times 180 degrees out-of-phase.
  2. returning some sentinel value "EOT" in getPositionColor makes it clear that your code is not separated into clean chunks (see "separation of concerns").
  3. the need for fix_color is yet another workaround that indicates the your solution is suboptimal. in the solution I posted it should automatically use the correct color (at least in principle; there may be still be bugs that occur when coding blind, I'm happy to fix them if you report those bugs to me).

Copy link
Contributor Author

@WaylonR WaylonR Sep 10, 2024

Choose a reason for hiding this comment

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

Your revised code was fixed, and pushed. Made the adjustment to outputMaybe, to output via the output code, rather than the prototype, which bypassed each modes output. adjusted the endwarning code to blink red/white as otherwise, if one had a red track, no warning would happen. Same if it was a white track, and the blink was to white.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you. It seems you forgot to push however.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, will do that now. Got distracted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

I think thats a LGTM. Thanks for your continued patience. Did you already open a manual PR? I can't find one.

@WaylonR
Copy link
Contributor Author

WaylonR commented Sep 10, 2024

Haven't done a manual PR for it. I need to do that? I also need to set the default color on all decks to "TRACK" before we send this for merging. Also, did you want a credit in it, for usage of the code from your mapping, and your assistance?
What's your byline? Your mapping didn't include one.

@Swiftb0y
Copy link
Member

Haven't done a manual PR for it. I need to do that?

yes: https://github.com/mixxxdj/mixxx/wiki/Contributing%20mappings#documenting-the-mapping

I also need to set the default color on all decks to "TRACK" before we send this for merging.

Sure. feel free to commit that now.

Also, did you want a credit in it, for usage of the code from your mapping, and your assistance?

I appreciate the offer but there is no need ;)

@WaylonR
Copy link
Contributor Author

WaylonR commented Sep 10, 2024

Push done.

@WaylonR
Copy link
Contributor Author

WaylonR commented Sep 10, 2024

Will sort the manual PR another time.

@Swiftb0y
Copy link
Member

No worries ;)

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.

3 participants