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 beatmix mapping #12422

Draft
wants to merge 12 commits into
base: 2.4
Choose a base branch
from

Conversation

uncleeugene
Copy link
Contributor

Reloop Beatmix gen.1 mapping added. Documentation is pulled as well.

@JoergAtGithub
Copy link
Member

Welcome at Mixxx!
As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

Copy link
Member

@ronso0 ronso0 left a comment

Choose a reason for hiding this comment

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

Thank you for this mapping!

I just skimmed through an added some first comments.

Please sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

res/controllers/Reloop Beatmix.midi.xml Outdated Show resolved Hide resolved
res/controllers/Reloop Beatmix.midi.xml Outdated Show resolved Hide resolved
res/controllers/Reloop Beatmix.midi.xml Outdated Show resolved Hide resolved
res/controllers/Reloop_Beatmix.js Outdated Show resolved Hide resolved
res/controllers/Reloop_Beatmix.js Outdated Show resolved Hide resolved
res/controllers/Reloop_Beatmix.js Outdated Show resolved Hide resolved
res/controllers/Reloop_Beatmix.js Outdated Show resolved Hide resolved
uncleeugene and others added 7 commits December 11, 2023 12:23
StartupWMClass added. With this property set now the icon in gnome dock
shown properly and it is now possible to pin mixxx app to dock.
Agree. I took this line from some other script i used as a base.

Co-authored-by: ronso0 <[email protected]>
Same here, this line is inherited from base script.

Co-authored-by: ronso0 <[email protected]>
Readability iprovement

Co-authored-by: ronso0 <[email protected]>
@uncleeugene
Copy link
Contributor Author

I've signed the agreement.
Git is giving me hard times :) One have to use it for some time to wrap his head around it. I have pushed another commit in that same branch by mistake, but reverted the commit. sorry about that.
Anyways, i updated all that matters with another push. Is that the proper way to update a pull request by just pushing all the changes?

@ronso0
Copy link
Member

ronso0 commented Dec 11, 2023

Thank you.

That's a good moment to learn about git rebase -i and git push -f. I suggest to rebase onto 2.4 and use the s option to squash all commits into one.

Actually rebasing while a review has been published is not advised, but with this rather small PR I see no issues. If you commit substantial changes later on you commit as usual.

@uncleeugene
Copy link
Contributor Author

That's a good moment to learn about git rebase -i and git push -f. I suggest to rebase onto 2.4 and use the s option to squash all commits into one.

I kinda understand what rebase is about but my understanding of the whole structure of git workflow is loose. So i definitely need to start from very basics :)

Comment on lines 817 to 825
<control>
<group>[Master]</group>
<key>headGain</key>
<status>0xB4</status>
<midino>0x51</midino>
<options>
<normal/>
</options>
</control>
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this does not affect the hardware output? (disable this mapping to double-check)
Just wondering because on my TerminalMix 2 and 4's it does, and they're also from 2012.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked it just now by closing headphone out with hardware pot and opening it back with mouse. Since there's just one headphone volume control i assume it is software :) Master is the only hardware control. Well there's mic gain and jog sensitivity trims as well.

@mxmilkiib
Copy link
Contributor

The file name convention is to use hyphens instead of spaces.

See also #12333

(I thought I left a comment before but most not have hit enter!)

@uncleeugene
Copy link
Contributor Author

The file name convention is to use hyphens instead of spaces.

See also #12333

(I thought I left a comment before but most not have hit enter!)

Done :)

Copy link

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Mar 12, 2024
@JoergAtGithub JoergAtGithub added needs review and removed stale Stale issues that haven't been updated for a long time. labels Mar 16, 2024
Comment on lines +146 to +150
if (control === QUICKJOGR || control === QUICKJOGL) {
engine.setValue(group, "jog", newValue/2);
} else {
engine.setValue(group, "jog", newValue/16);
}
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason you have two jog modes, not jog and scratch?
That would free Shift+Load for Eject (currently it's the scratch toggle).
Note that Eject is not purely "eject" in Mixxx 2.4, it also does un-eject and un-overload, see https://manual.mixxx.org/2.4/en/chapters/appendix/mixxx_controls.html#control-[ChannelN]-eject

Btw does holding the 🔍 button affect the jog wheel? Like long-press it and it does not toggle the LED permanently?
Reason I ask is, on the TerminalMix the wheel has three modes, as well:
regular (no touch signal), scratch (touch), search (:mag: hold)


// Beatmix.navKnob handles navigation dial. It will scroll through library.
Beatmix.navKnob = function(_channel, control, value, _status, _group) {
//console.log("control: " + control + ", value: " + value + ", status: " + status);
Copy link
Member

Choose a reason for hiding this comment

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

Is this still needed?

}
};

// Beatmix.loopSize handles Loop Size knob. This know ajusts auto loop size.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Beatmix.loopSize handles Loop Size knob. This know ajusts auto loop size.
// Beatmix.loopSize handles Loop Size knob. This now adjusts auto loop size.

Beatmix.loopSize = function(_channel, _control, value, _status, group) {
const newValue = value - 64;
if (newValue < 0) {
engine.setValue(group, "loop_halve", 1);
Copy link
Member

@ronso0 ronso0 Mar 18, 2024

Choose a reason for hiding this comment

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

I suggest to use script.triggerControl(group, control, delay) to release the button.

</options>
</control>
<control>
<group>[EffectRack1_EffectUnit1]</group>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really happy with this since this control has no GUI indicator. (though, units are enabled by default after start)
Also, for On/Off you already have the Mix knob mapped.

Can we use it for something else instead which is not covered, yet?
Press: ON
Shift+Press: OFF

or

Press: toggle Mix between 0 and 1
Shift+Press: toggle Quick Effect

or

Press: un/assign unit1 to deck1
Shift+Press: un/assign unit1 to deck2 / un/assign unit2 to deck1

@ronso0 ronso0 added this to the 2.4.2 milestone Apr 13, 2024
@daschuer daschuer marked this pull request as draft September 17, 2024 06:06
@daschuer daschuer removed this from the 2.4.2 milestone Sep 17, 2024
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.

5 participants