-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Mapping for MIDI Controller Behringer BCR2000 #3342
Mapping for MIDI Controller Behringer BCR2000 #3342
Conversation
I've come across a few Wiki articles that might be interesting for the mapping. I'd appreciate if someone could shortly comment on the progress on these topics:
|
These are old proposals that will not be implemented. The "New controller system" page has a summary of current plans.
Yes, it will be an entirely different API. |
This PR contains several independent building blocks that together form a mapping for the Behringer BCR2000 controller. I'd like to explain these building blocks a little bit so that the whole stuff is easier to understand. Maybe a part of this code can be re-used for other things, your thoughts and comments are welcome. General stuff (independent of the controller)File
|
This allows more than 1 GenericMidiContollers at a time.
…GenericMidiController
This is not strictly necessary for shift handling but avoids binding to an undefined mixxx control. This reverts commit 612e456.
@git-developer Sorry for this mess. Maybe you're interested in joining the Zulip discussion regarding the new API? |
I understand your concerns. It would be no problem for me to "hide" the code by moving it so that others don't use it unintentionally. Just one thing: some of those components are used in my mapping for the Behringer DDM4000 mixer. Thus, I'd like to use a common namespace for these components. What do you think of changing the namespace I'm new to Zulip. Is there a topic in the controller stream for the discussion on the new mapping system? |
Duplicating the code would be the worst decision. Using one of the |
I replaced Can I do something about the failing build? I'm not sure what's wrong, looks like a problem downloading a key finder library, unrelated to this PR. |
Looks like the build is failing in my fork only. |
|
…r-mapping-behringer-bcr2000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After renaming the files I consider this ready to be merged. The corresponding manual PR is in place, though still failing to build.
Hopefully, there are still some owners of this/these devices who will appreciate the retrofitting efforts! :)
…apping-behringer-bcr2000
…apping-behringer-bcr2000
No, this hasn't even begun review. |
This PR is marked as stale because it has been open 90 days with no activity. |
This PR was marked as stale but is actually waiting for a review or merge. |
Yes, sorry for the bot marker. Reviewers are a scarce resource and Mixxx is understaffed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry nobody was able to properly review your PR. I'm unable to review the mapping properly as well because of its enormous size. The code looks pretty solid at first glance and you seem to know your JS, so I trust you that this code is of decent quality.
does anyone object to merging this?
This was also my impression when skimming over the code. Neither other mappings nor the core application would be affected if it doesn't work as expected. I don't expect that we are able to find someone for testing who owns this legacy controller. After the mapping has been merged and becomes accessible for regular users we might get feedback. |
Ok for me. In general I don't want to maintain another abstraction library. It should be clear that other controller mappings shouldn't build upon that library. But I'm okay with merging it. Documentation PR looks good, so I'm okay with merging this. |
Thanks! |
@Swiftb0y please always merge the corresponding manual changes, too. |
Yes, I'm sorry I didn't have that on my radar. |
Documentation PR: mixxxdj/manual#310
This PR contains more than an average mapping:
We'll probably discuss a little bit about all this, which could be done in this PR or the forum thread.