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

Behringer CMD-MM1 Rework #1744

Merged
merged 9 commits into from
Oct 20, 2018
Merged

Behringer CMD-MM1 Rework #1744

merged 9 commits into from
Oct 20, 2018

Conversation

Swiftb0y
Copy link
Member

@Swiftb0y Swiftb0y commented Jul 7, 2018

I rewrote the entire Mapping for better maintainability, readability and performance (marginally).
I removed some features which I turned out to be useless in the old mapping, but it is mostly the same.
It still features the customizability from the old mapping, eg you can still toggle between deck- and fx-channels, different knob assignments, deck sequence and fader assignment (volume/rate/super1).
Some of the core methods of the components framework got overwritten to support the multiple layers the original mapping had as well.
Furthermore, some components have almost no / duplicate functionality (like the middlebutton, the encoderbutton, and layer2 of the 1&2-buttons (similar to layer2 of the fxchannels)). Suggestions would be very appreciated.

@Be-ing
Copy link
Contributor

Be-ing commented Aug 2, 2018

Thank you for working on this and sorry for the long time with no response. I'll review this next week (I'll be travelling this weekend). Feel free to leave a comment to remind me if I haven't gotten to it.

@Swiftb0y
Copy link
Member Author

@Be-ing
just reminding you about this PR ;)

@Be-ing
Copy link
Contributor

Be-ing commented Aug 30, 2018

FYI, I haven't forgotten about this, but I'm focusing on getting the build environments and build servers working for 2.2 beta.

@Be-ing
Copy link
Contributor

Be-ing commented Sep 7, 2018

Thanks for putting in so much work just to clean up the code. :) This looks better organized at first glance. The way you set up rotating through layers is interesting. I'm wondering if you could use more descriptive names than just integers for the layers. Also, it would be helpful to use more descriptive names than "buttonINT" for the Components. I'll take a more thorough look tomorrow.

@Swiftb0y
Copy link
Member Author

Swiftb0y commented Sep 7, 2018

I have thought about about using normal/shift/ctrl/shiftctrl as layer names but just numbering seamed easier. (It also enables me to easily call the method. (L36)). The ButtonINT convention might not seem intuitive until you look at the controller. The "naming convention" I went for was actually buttonLABEL. So buttonCue, button1, button2, make sense. I'll add a comment about that in the code.

@mixxxdj mixxxdj deleted a comment Sep 7, 2018
@mixxxdj mixxxdj deleted a comment Oct 2, 2018
Copy link
Contributor

@Be-ing Be-ing left a comment

Choose a reason for hiding this comment

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

Sorry again for the delay reviewing this. Overall this looks quite good. I was able to understand the code pretty clearly. The biggest thing I am still unclear about is how the numbered layers correspond to the different buttons. If I understand correctly:
1: default
2: shift
3: ctrl
4: shift + ctrl
Is that right?

res/controllers/Behringer-CMD-MM1-scripts.js Outdated Show resolved Hide resolved
res/controllers/Behringer-CMD-MM1-scripts.js Outdated Show resolved Hide resolved
res/controllers/Behringer-CMD-MM1-scripts.js Show resolved Hide resolved
res/controllers/Behringer-CMD-MM1-scripts.js Show resolved Hide resolved
res/controllers/Behringer-CMD-MM1-scripts.js Outdated Show resolved Hide resolved
res/controllers/Behringer-CMD-MM1-scripts.js Show resolved Hide resolved
res/controllers/Behringer-CMD-MM1-scripts.js Outdated Show resolved Hide resolved
res/controllers/Behringer-CMD-MM1-scripts.js Outdated Show resolved Hide resolved
CMDMM.layer(1);

// midi.sendSysexMsg(MIDI.ControllerDump,MIDI.ControllerDump.lenght);
// Doesn't return anything. The controller might not be serato certified.
Copy link
Contributor

Choose a reason for hiding this comment

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

You can try asking Behringer if there is a MIDI message you can send to the controller to get it to reply with the state of all the knobs and faders. Unfortunately, not all controllers have that capability.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue might have come from the typo of length, I'll to try to test if that resolves the issue in the next couple of days.

Copy link
Member Author

Choose a reason for hiding this comment

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

It didn't resolve the Issue. I wrote to their technical support. They usually take some time to respond but they were able to help me in a similar inquiry before.

Copy link
Member Author

Choose a reason for hiding this comment

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

Update: They said that it is not possible to control the Controller via SysEx. So I guess there is no Midi dump feature as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

That does not imply there is no MIDI dump feature. Hercules controllers use a regular 3 byte MIDI message to request a MIDI dump.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I asked them more or less explicitly about a MIDI dump feature and their response was:

Dear Nikolaus, Regarding your inquiry: SysEx on the CMD-MM1.
My apologies for the delayed response. Unfortunately, it is not possible to control the CMD-MM1 via Sysex messages. Thank you for your patience.

So I either they don't want to explicitly state that their is/isn't such a feature, or their just isn't one...

Copy link
Contributor

Choose a reason for hiding this comment

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

That response doesn't necessarily mean there isn't a MIDI dump feature. I imagine the customer support person who answered your email just asked an engineer about sysex messages without describing why.

Copy link
Member Author

Choose a reason for hiding this comment

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

Weill, I mentioned the dump feature as an example but you could be right. I could try to fuzz the controller and see if anything happens.

Copy link
Member Author

Choose a reason for hiding this comment

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

On second thought: They have a utility to change the midi channel of the controller, so there must be a way to configure the controller somehow (either via sysex or regular midi).

res/controllers/Behringer-CMD-MM1-scripts.js Outdated Show resolved Hide resolved
@Swiftb0y
Copy link
Member Author

Swiftb0y commented Oct 9, 2018

1: default
2: shift
3: ctrl
4: shift + ctrl
Is that right?

Yes, exactly.
Btw. Should I worry about that CodeFactor issue? I most of the warnings only come from the fact that I used the whole function signature (even though I didn't use most of the arguments) for consistency's sake.

@mixxxdj mixxxdj deleted a comment Oct 9, 2018
@Be-ing
Copy link
Contributor

Be-ing commented Oct 10, 2018

1: default
2: shift
3: ctrl
4: shift + ctrl
Is that right?

Yes, exactly.

Okay, thanks for clarifying. Please either explain this in a comment or use the button names instead of numbers for the different layers.

@mixxxdj mixxxdj deleted a comment Oct 20, 2018
@Be-ing Be-ing merged commit da4d160 into mixxxdj:2.2 Oct 20, 2018
@Be-ing
Copy link
Contributor

Be-ing commented Oct 20, 2018

Thanks again for your patience with this PR. Now that you've gone through the experience of rewriting a mapping using Components, your input in future discussions about how to improve the controller mapping system would be appreciated!

@Swiftb0y Swiftb0y deleted the CMDMM_rework branch October 20, 2018 23:09
@Swiftb0y
Copy link
Member Author

Of course. I'd be grateful to help and improve Mixxx where ever I can.

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.

2 participants