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

Hercules P32 mapping: pregain instead of dry/wet knob (optional) #3538

Conversation

pierrelegall
Copy link
Contributor

This feature is optional (see the dryWetKnobAsPregain flag).

@pierrelegall pierrelegall force-pushed the mapping/hercules-p32/pregain-on-drywetknobs branch from 544d6ad to 6765507 Compare January 6, 2021 17:21
@pierrelegall pierrelegall force-pushed the mapping/hercules-p32/pregain-on-drywetknobs branch from 6765507 to 3732aef Compare January 7, 2021 14:54
@pierrelegall
Copy link
Contributor Author

Documentation PR: mixxxdj/manual#343

@Be-ing
Copy link
Contributor

Be-ing commented Jan 21, 2021

I like the idea to implement a way to control the deck gain. But I think we may be able to come up with a better way to design this. I would not want to sacrifice any of the existing functionality of the mapping to make room for the gain control. My first thought was to use shift + dry/wet knob, but that would be complicated to implement and dealing with soft takeover is never ideal. Is there a way we could use one of the encoders? All possible actions with the encoders with and without shift are already mapped. Could we make another modifier? Maybe using the shift button of the opposite side? Do you have any other ideas?

@pierrelegall
Copy link
Contributor Author

Maybe using the shift button of the opposite side?

I did'nt know it was possible to map this kind of binding. Nice to know! But I like to be able to change pregain with just one hand.

I found it nice at use (the chosen potentiometer feet the use case well). However, agreed to the fact this is not an ideal design ; and I don't have any better idea yet.

I don't really know if it's a good idea or not to merge this into Mixxx because it complixify the reading of the mapping. It's not really complex but it's not just one simple if statement as we can suppose. But don't forget it is an optional feature, the default behavior does not change.

@github-actions
Copy link

github-actions bot commented May 3, 2021

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 May 3, 2021
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.

Hey @pierrelegall
Is there still any desire to get this merged? We'd also need a small PR to the manual in that case.

@pierrelegall
Copy link
Contributor Author

pierrelegall commented Mar 24, 2023

Is there still any desire to get this merged?

I do not use a P32 anymore.

But it's ok for me, I could write the documentation if @Be-ing approve this changes.

@Swiftb0y
Copy link
Member

I don't think they do either. IMO this mapping is a minimal improvement, but if you don't think its worth the effort to document the changes, we can just close the PR too. Either is completely fine for me. I just want to shrink our PR backlog a bit.

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.

LGTM, this doesn't change any behavior nor does it increase complexity much.

@Swiftb0y Swiftb0y merged commit 72518d6 into mixxxdj:2.3 Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controller mappings stale Stale issues that haven't been updated for a long time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants