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

Reloop Mixage MIDI mappings #10892

Closed
wants to merge 33 commits into from
Closed

Conversation

HorstBaerbel
Copy link
Contributor

Redoing pull request. MIDI mappings for the Reloop Mixage DJ controller. Corresponding manual pull request is #516.

@HorstBaerbel
Copy link
Contributor Author

No newline at the end of the file - oh, come on...

@ronso0
Copy link
Member

ronso0 commented Sep 12, 2022

No newline at the end of the file - oh, come on...

You may squash fixup commits as long as no one started reviewing the changes.
Or we squash when merging, after all issues and review comments are resolved. No big deal for a mapping.

@ronso0 ronso0 added this to the 2.3.4 milestone Oct 28, 2022
@JosepMaJAZ
Copy link
Contributor

I had a quick look at the source (I don't own this controller) and it looks good.
A classic non-components mapping with some added functionality (browsing with wheel, beat moving and some effect functionality).

The only thing that cached my eye was the use of a timer to update the vumeters. While this solution is ok, Mixxx has a control trigger to update the vumeters, just like it does for button leds.
You can check several controllers about this functionality, but two of them with which I am familiar is Hercules DJ Console 4-mx (non-compoments) and Numark Mixtrack Platinum (components).

@HorstBaerbel
Copy link
Contributor Author

HorstBaerbel commented Nov 25, 2022

I had a quick look at the source (I don't own this controller) and it looks good. A classic non-components mapping with some added functionality (browsing with wheel, beat moving and some effect functionality).

The only thing that cached my eye was the use of a timer to update the vumeters. While this solution is ok, Mixxx has a control trigger to update the vumeters, just like it does for button leds. You can check several controllers about this functionality, but two of them with which I am familiar is Hercules DJ Console 4-mx (non-compoments) and Numark Mixtrack Platinum (components).

Thanks for your review. I can take a look at the other solutions for updating the vumeters. Could you link to the related Mixxx docs / functions? I seem to have missed this in the docs when implementing...

@ronso0
Copy link
Member

ronso0 commented Nov 25, 2022

The script for the Hercules DJ Console 4-mx mapping:

engine.connectControl("[Channel" + i + "]", "VuMeter", "Hercules4Mx.onVuMeterDeck" + i);

@Swiftb0y
Copy link
Member

Swiftb0y commented Nov 25, 2022

fyi thats not a great example. Prefer this instead:

updateVUMetersTimer[0] = engine.makeConnection("[Channel1]", "VuMeter", function(val) {
        midi.sendShortMsg(0x90, 29, val * 7);
});

and then disconnect on shutdown:

updateVUMetersTimer[0].disconnect()

See the wiki on engine.makeConnection

@HorstBaerbel
Copy link
Contributor Author

I had a quick look at the source (I don't own this controller) and it looks good. A classic non-components mapping with some added functionality (browsing with wheel, beat moving and some effect functionality).

The only thing that cached my eye was the use of a timer to update the vumeters. While this solution is ok, Mixxx has a control trigger to update the vumeters, just like it does for button leds. You can check several controllers about this functionality, but two of them with which I am familiar is Hercules DJ Console 4-mx (non-compoments) and Numark Mixtrack Platinum (components).

I've refactored the VU meter updates as suggested. Tell me when there's anything else...

@daschuer daschuer removed this from the 2.3.4 milestone Feb 23, 2023
@HorstBaerbel
Copy link
Contributor Author

HorstBaerbel commented Mar 19, 2023

The mappings got removed from the 2.3.4 milestone. Could you queue them for the next? Is there anything else I can do to get this PR accepted? Manual page should be ok too. Tell me if it needs work...

@Swiftb0y
Copy link
Member

Sorry, I have a couple review comments pending but didn't get around to finish the review yet. please give me a couple days and feel free to remind me in case I forget.

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.

first couple of comments

res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
@HorstBaerbel
Copy link
Contributor Author

I am done with the updated mapping. The only remaining issue is the open comment. Feedback is appreciated.

res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
res/controllers/Reloop-Mixage.scripts.js Outdated Show resolved Hide resolved
…able. Map PAN button. Refactor rotatry button presses
@HorstBaerbel
Copy link
Contributor Author

HorstBaerbel commented Mar 23, 2023

Integrated your suggestions and updated the docs. Also added master balance (there is no deck balance in Mixxx). Thanks for all the suggestions! Mapping is much improved, more logical and faithful to the original one (where possible) with some extra functionality.
The pipeline seems to have choked on my commit though...

@HorstBaerbel
Copy link
Contributor Author

@Swiftb0y So... This was not included in any of the 2.3 releases afair, which I find a bit sad, and the final 2.3 is out the door. But well. @gqzomer (Gersom Zomer) has been working on some improvements and refactored the JS. We're thinking about adding this to the 2.4 version to hopefully get released. How would we go about this?

@HorstBaerbel
Copy link
Contributor Author

We've improved the mapping in this PR #12296 for Mixxx 2.4. I'm closing this PR now.

@Swiftb0y
Copy link
Member

Thank you. I'm sorry I couldn't find the energy to look at this for now.

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.

6 participants