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.

Sign up for GitHub

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

Jump to bottom

Roland DJ-505: Use new ControlIndicator COs for blinking lights #4159

Merged
merged 2 commits into from
Aug 2, 2021
Merged

Roland DJ-505: Use new ControlIndicator COs for blinking lights #4159

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a670fe
Roland DJ-505: Use new indicator_500millis for explicit sync leader
Holzhaus Jul 29, 2021
00c782b
Roland DJ-505: Use indicator_250millis for blinking loop pads
Holzhaus Jul 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 28 additions & 35 deletions res/controllers/Roland_DJ-505-scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,11 +600,24 @@ DJ505.Deck = function(deckNumbers, offset) {
midi: [0x90 + offset, 0x02],
group: "[Channel" + deckNumbers + "]",
outKey: "sync_mode",
flickerState: false,
output: function(value, _group, _control) {
output: function(value, group, control) {
if (this.connections[1] !== undefined) {
this.connections[1].disconnect();
delete this.connections[1];
}

// If the new sync_mode is "Explicit Leader", use the blinking
// indicator for the LED instead.
if (value === 3) {
value = this.flickerState;
if (this.connections[1] === undefined) {
this.connections[1] = engine.makeConnection("[Master]", "indicator_500millis", this.setLed.bind(this));
}
return;
}

this.setLed(value, group, control);
},
setLed: function(value, _group, _control) {
midi.sendShortMsg(this.midi[0], value ? 0x02 : 0x03, this.on);
},
input: function(channel, control, value, _status, _group) {
Expand Down Expand Up @@ -644,20 +657,6 @@ DJ505.Deck = function(deckNumbers, offset) {
script.toggleControl(this.group, "quantize");
};
},
connect: function() {
components.Button.prototype.connect.call(this); // call parent connect
this.flickerTimer = engine.beginTimer(500, function() {
this.flickerState = !this.flickerState;
this.trigger();
}.bind(this));
},
disconnect: function() {
components.Button.prototype.disconnect.call(this); // call parent disconnect
if (this.flickerTimer) {
engine.stopTimer(this.flickerTimer);
this.flickerTimer = 0;
}
},
});

// =============================== MIXER ====================================
Expand Down Expand Up @@ -1611,9 +1610,6 @@ DJ505.SavedLoopMode = function(deck, offset) {
on: this.color,
longPressTimeout: 500,
colorMapper: DJ505.PadColorMap,
blinkTimer: 0,
blinkTimeout: 250,
blinkStateOn: false,
unshift: function() {
this.inKey = "hotcue_" + this.number + "_activateloop";
this.input = components.Button.prototype.input;
Expand All @@ -1638,26 +1634,23 @@ DJ505.SavedLoopMode = function(deck, offset) {
}.bind(this);
},
stopBlinking: function() {
if (this.blinkTimer !== 0) {
engine.stopTimer(this.blinkTimer);
this.blinkTimer = 0;
if (this.connections[2] !== undefined) {
this.connections[2].disconnect();
delete this.connections[2];
}
},
output: function(value, _group, _control) {
this.stopBlinking();
if (value === 2) {
this.blinkTimer = engine.beginTimer(
this.blinkTimeout, function() {
var colorValue = this.colorMapper.getValueForNearestColor(
engine.getValue(this.group, this.colorKey));
if (this.blinkStateOn) {
this.send(colorValue + DJ505.PadColor.DIM_MODIFIER);
this.blinkStateOn = false;
} else {
this.send(colorValue);
this.blinkStateOn = true;
}
}.bind(this));
this.connections[2] = engine.makeConnection("[Master]", "indicator_250millis", function(value, _group, _control) {
var colorValue = this.colorMapper.getValueForNearestColor(
engine.getValue(this.group, this.colorKey));
if (value) {
this.send(colorValue);
} else {
this.send(colorValue + DJ505.PadColor.DIM_MODIFIER);
}
}.bind(this));
Comment on lines +1645 to +1653
Copy link
Member

Choose a reason for hiding this comment

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

Now that we have ES7 support, we should phase out the function expression + .bind(this) pattern and use arrow functions instead IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

Arrow functions are harder to read IMHO, but that aside, when I apply this patch:

+++ b/res/controllers/Roland_DJ-505-scripts.js
@@ -1642,7 +1642,7 @@ DJ505.SavedLoopMode = function(deck, offset) {
         output: function(value, _group, _control) {
             this.stopBlinking();
             if (value === 2) {
-                this.connections[2] = engine.makeConnection("[Master]", "indicator_250millis", function(value, _group, _control) {
+                this.connections[2] = engine.makeConnection("[Master]", "indicator_250millis", (value, _group, _control) => {
                     var colorValue = this.colorMapper.getValueForNearestColor(
                         engine.getValue(this.group, this.colorKey));
                     if (value) {
@@ -1650,7 +1650,7 @@ DJ505.SavedLoopMode = function(deck, offset) {
                     } else {
                         this.send(colorValue + DJ505.PadColor.DIM_MODIFIER);
                     }
-                }.bind(this));
+                });
             } else if (value === 1) {
                 var colorValue = this.colorMapper.getValueForNearestColor(
                     engine.getValue(this.group, this.colorKey));

I get:

Uncaught exception at line 1649 in file file:///home/jan/Projects/mixxx/res/controllers/Roland_DJ-505-scripts.js.

Exception:
  TypeError: Property 'send' of object [object Object] is not a function

Backtrace:
@file:///home/jan/Projects/mixxx/res/controllers/Roland_DJ-505-scripts.js:1649

Copy link
Member

Choose a reason for hiding this comment

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

Thats strange. I'll try to reproduce.

Copy link
Member

@Swiftb0y Swiftb0y Aug 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Arrow functions are harder to read IMHO

I disagree, they're just like closures from most other functions (C++ lambdas, Rust Fn, etc). And they get rid of the .bind(this) that most people without indepth JS knowledge don't understand.

Copy link
Member Author

Choose a reason for hiding this comment

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

Arrow functions are harder to read IMHO

I disagree, they're just like closures from most other functions (C++ lambdas, Rust Fn, etc). And they get rid of the .bind(this) that most people without indepth JS knowledge don't understand.

I'd argue that people without indepth JS knowledge wouldn't even recognize that foo => bar is a function. I think function(foo) { return bar; } is much clearer to a beginner. Just my 2 cents.

Copy link
Member

Choose a reason for hiding this comment

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

it depends, I think function expressions are being thought quite early now especially with the rising popularity of functional programming and APIs that use callbacks.

Copy link
Member

Choose a reason for hiding this comment

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

I tried to reproduce the issue but I was unable to:

let Foo = function () {
    components.Button.call(this);
}
Foo.prototype = new components.Button({
    init: function() {
        engine.makeConnection("[Channel1]", "play", (value, group, control) => {
            print(typeof this.send);
        });
    }
});

var MyController = new Foo();

Obviously the example is a bit atypical but the point is the same.

Copy link
Member

@Swiftb0y Swiftb0y Aug 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't really care if its an arrow function or a plain old function expression in this case. This is just a controller mapping after all. Though I'd like to establish the use of arrow functions for any new code in the future with the ES6 rewrite of components JS.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, let's just merge this then, and I'll try to make a PR to use arrow functions later.

} else if (value === 1) {
var colorValue = this.colorMapper.getValueForNearestColor(
engine.getValue(this.group, this.colorKey));
Expand Down