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

midi-components: Improve error reporting #2583

Closed
wants to merge 2 commits into from
Closed

midi-components: Improve error reporting #2583

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a2bf8d
res/controllers/midi-components: Throw JS errors instead of using print
Holzhaus Mar 21, 2020
0fe974a
res/controllers/midi-components: Improve error reporting in outputColor
Holzhaus Mar 21, 2020
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
30 changes: 12 additions & 18 deletions res/controllers/midi-components-0.0.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,7 @@

var HotcueButton = function(options) {
if (options.number === undefined) {
print("ERROR: No hotcue number specified for new HotcueButton.");
return;
throw Error("No hotcue number specified for new HotcueButton.");
Copy link
Member

Choose a reason for hiding this comment

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

Who catches the exception? If Mixxx is quit, this can be a party stopper, which is a kind of no go.

Copy link
Member Author

@Holzhaus Holzhaus Mar 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

If this exception is thrown and the controller mapping does not wrap it in a try-catch-block (which would be weird), then mixxx will open a popup that a script error occurred (as we already do in other cases when the mapping throws an error, e. g. when an undefined value is called as a function).

Copy link
Member

@daschuer daschuer Mar 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

OK, so the party is going on in any case, right?

Copy link
Member

Choose a reason for hiding this comment

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

IIRC yes. Only the current "codepath" is affected. So if the error is thrown in a handler, the handler will not work properly (as expected), if the error is thrown in a constructor or the init method, the error will be shown on startup. The only case where this could break if the error is thrown in the constructor of a component and the mapping is creating Objects while running. In that case the objects wouldn't be constructed and thus the handlers of these objects won't be present. But even in this case, the rest of the mapping would still be functional iirc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this code snippet to my controller mapping:

    this.someButton = new components.HotcueButton({
        group: "[Channel1]",
        midi: [0x9F, 0x02],
    });

Directly after loading the controller, this message popups up:
2020-03-23-105112_806x621_scrot

This is much more discoverable than using print which only prints to the console when --controllerDebug is specified (and my controller output Midi clock signals so I can only see it for the fraction of a seconds before it scrolls off-screen).

}
if (options.colorMapper !== undefined || options.sendRGB !== undefined) {
this.colorKey = "hotcue_" + options.number + "_color";
Expand Down Expand Up @@ -334,18 +333,18 @@
// get the MIDI value for the nearest supported color and send it.
var nearestColorValue = this.colorMapper.getValueForNearestColor(colorCode);
this.send(nearestColorValue);
} else {
} else if (this.sendRGB !== undefined) {
// Since outputColor has been called but no ColorMapper is
// available, we can assume that controller supports arbitrary
// RGB color output.
this.sendRGB(colorCodeToObject(colorCode));
} else {
throw Error(
"HotcueButton is unable to send RGB color: colorKey has " +
"been set, but no color mapper was specified and the " +
"sendRGB(colorObject) method is not implemented!");
}
},
sendRGB: function(_colorObject) {
// This method needs to be overridden in controller mappings,
// because the procedure is controller-dependent.
throw Error("sendRGB(colorObject) not implemented - unable to send RGB colors!");
},
connect: function() {
Button.prototype.connect.call(this); // call parent connect
if (undefined !== this.group && this.colorKey !== undefined) {
Expand All @@ -359,8 +358,7 @@
});
var SamplerButton = function(options) {
if (options.number === undefined) {
print("ERROR: No sampler number specified for new SamplerButton.");
return;
throw Error("No sampler number specified for new SamplerButton.");
}
this.volumeByVelocity = options.volumeByVelocity;
this.number = options.number;
Expand Down Expand Up @@ -509,8 +507,7 @@
ComponentContainer.prototype = {
forEachComponent: function(operation, recursive) {
if (typeof operation !== "function") {
print("ERROR: ComponentContainer.forEachComponent requires a function argument");
return;
throw Error("ComponentContainer.forEachComponent requires a function argument");
}
if (recursive === undefined) { recursive = true; }

Expand All @@ -535,8 +532,7 @@
},
forEachComponentContainer: function(operation, recursive) {
if (typeof operation !== "function") {
print("ERROR: ComponentContainer.forEachComponentContainer requires a function argument");
return;
throw Error("ComponentContainer.forEachComponentContainer requires a function argument");
}
if (recursive === undefined) { recursive = true; }

Expand Down Expand Up @@ -677,8 +673,7 @@
this.deckNumbers = [deckNumbers];
}
} else {
print("ERROR! new Deck() called without specifying any deck numbers");
return;
throw Error("new Deck() called without specifying any deck numbers");
Holzhaus marked this conversation as resolved.
Show resolved Hide resolved
}
};
Deck.prototype = new ComponentContainer({
Expand Down Expand Up @@ -827,8 +822,7 @@
this.setCurrentUnit(unitNumbers);
}
} else {
print("ERROR! new EffectUnit() called without specifying any unit numbers!");
return;
throw Error("new EffectUnit() called without specifying any unit numbers!");
}

this.dryWetKnob = new Pot({
Expand Down