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

Update controller mapping for Behringer DDM4000 mixer #4262

Merged

Conversation

git-developer
Copy link
Contributor

@git-developer git-developer commented Sep 4, 2021

This PR contains a few fixes and updates for the controller mapping of the Behringer DDM4000 mixer:

  • show_xfader is no longer in group [Master] but in group [Skin]
  • The ShiftButton supports default button behavior (by deriving Buttons input function)
  • The DirectionEncoder supports a relative mode (as known for Pots)
  • New component RangeAwarePot that combines the behavior of a RangeAwareEncoder with soft-takeover
  • Controller feedback for BackLoopButton
  • Allow sendShifted for effect components
  • Minor fixes and updates to the documentation

The changes are fully backwards-compatible (except the show_xfader fix, of course). They have been tested extensively with a custom mapping that maps an effect unit and deck effects (loop, reverse) to the mic, crossfader and sampler sections of the mixer. I don't plan to merge this custom mapping here (because the current mapping resembles the mixer functions best), but I plan to share it on the forums when this PR is accepted. I can provide the mapping if desired.

There's intentionally no PR for the manual because the changes do not affect the documentation.

@uklotzde
Copy link
Contributor

uklotzde commented Sep 7, 2021

We also need a PR for updating the manual accordingly. Both should be merged together. Please link it in the description.

@uklotzde uklotzde added this to the 2.3.1 milestone Sep 7, 2021
@uklotzde uklotzde added the changelog This PR should be included in the changelog label Sep 7, 2021
@git-developer
Copy link
Contributor Author

I'd love to add a PR for the manual, but there's no change in the documentation involved. All implementation changes are internally; just a few bug fixes and additional features that may be used by custom mappings.

Comment on lines -108 to +117
input: function(_channel, _control, value, _status, _group) {
inSetValue: function(value) {
if (value) {
this.target.shift();
} else {
this.target.unshift();
}
components.Button.prototype.inSetValue.call(this, value);
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 explain what the purpose of this change is?

Copy link
Contributor Author

@git-developer git-developer Sep 9, 2021

Choose a reason for hiding this comment

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

I mapped the Shift button to a physical button that has a LED, and noticed that the LED does not work. This was caused by Mixxx not sending anything when the outKey control changed, because the input function of Button was overridden.

The current change does not override input so the original implementation survives, and delegates to the Button's inSetValue function so that Shift behaves just as any other button does.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I'm still confused (likely because I have 0 idea how the accompanying library you wrote works).
The shift button does not seem to be coupled to any CO, so the outKey is irrelevant.

you can still overwrite your input implemenation, you just need to send the midi output yourself using this.send(value) for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, let me explain it a little bit more detailed; you don't need to understand the whole library for that.

The shift button is first of all a just a normal Button that is connected to a user-defined control and additionally calls shift() and unshift() on some other ("target") component. The user may configure the connected Mixxx control as well as other options (like the button's type, for example).

In the BCR2000 mapping, the connected Mixxx control is configured as group: [Controls] & key: touch_shift, and the target is the top level ComponentContainer that contains all components of the mapping. When the user presses the shift button, shift() is called on the top level container and then delegated to all child components. This is effectively a shift for all components of the whole mapping. When the user's skin contains a control for [Controls],touch_shift, the shift state is also visualized in the UI.

I didn't use a default for group and key in the ShiftButton's constructor because it may as well be used for just a group of components or just for a single component. [Controls],touch_shift would be misleading in that case.

It's true that I could override input() and call this.send(value) instead of overriding inSetValue(). But when I do so, the ShiftButton loses the feature of button types (push, toggle, powerWindow), because this is implemented in Button.input().

Copy link
Member

Choose a reason for hiding this comment

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

Ah now I see, thanks for your explanation. I'm sorry I forced you to write this detailed explanation.

res/controllers/Behringer-Extension-scripts.js Outdated Show resolved Hide resolved
var script = global.script;
var group = this.group;
Copy link
Member

Choose a reason for hiding this comment

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

why is this line required?

Copy link
Contributor Author

@git-developer git-developer Sep 9, 2021

Choose a reason for hiding this comment

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

This is required because group is no longer an argument of the surrounding function. group is used 7 times within the function, so it either has to be declared like this or I'd have to change the 7 occurrences to this.group.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, makes sense. I guess thats valid.

Comment on lines 1351 to 1355
var shiftType = effectUnitDefinition.sendShiftedFor;
if (typeof shiftType === "function") {
effectUnit.forEachComponent(function(component) {
if (component instanceof shiftType) {
component.sendShifted = true;
Copy link
Member

Choose a reason for hiding this comment

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

whats the shiftType === "function" for?

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 property sendShiftedFor is a filter tor the type of a JS component , e.g. c.Button, c.Pot or c.Component. If the user provides a string, number, boolean or object instead, the following instanceof statement will cause a runtime error.

Copy link
Member

Choose a reason for hiding this comment

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

Mhmm makes sense, but its not self explaining. Could you add a comment that explains this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, done.

Co-authored-by: Swiftb0y <12380386+Swiftb0y@users.noreply.github.com>
@uklotzde
Copy link
Contributor

@Swiftb0y Merge?

@Swiftb0y Swiftb0y merged commit 9517200 into mixxxdj:2.3 Sep 12, 2021
@git-developer git-developer deleted the feature/controller-mapping-behringer-ddm4000 branch September 14, 2021 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog controller mappings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants