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

MC4000 update #1102

Merged
merged 3 commits into from
Jan 11, 2017
Merged

MC4000 update #1102

merged 3 commits into from
Jan 11, 2017

Conversation

timrae
Copy link
Contributor

@timrae timrae commented Jan 1, 2017

Update MC4000 to follow @Be-ing's great work in #1062 and #1063.
I also managed to get midi spying working on OSX so that I was able to pull the Sysex message that makes the controller send its current settings to mixxx.

Known problems:

  • The default key lock setting might be getting lost after adding the sysex

@timrae
Copy link
Contributor Author

timrae commented Jan 1, 2017

I'm sure it's not related to this PR, but one of the unit tests are segfaulting:

�[0;32m[ RUN      ] �[mAutoDJProcessorTest.TransitionTimeLoadedFromConfig

Loading resources from  "/Users/travis/build/mixxxdj/mixxx/res/"

Available QtSQL drivers: ("QSQLITE")

DB status: "/Users/travis/build/mixxxdj/mixxx/src/test/test_data/mixxxdb.sqlite" = true

/Users/travis/build.sh: line 57:  7277 Segmentation fault: 11  ./mixxx-test



travis_time:end:1dbfce45:start=1483292065451527000,finish=1483292071091367000,duration=5639840000
�[0K

�[31;1mThe command "./mixxx-test" exited with 139.�[0m

@Be-ing
Copy link
Contributor

Be-ing commented Jan 4, 2017

I don't think the test crashing has anything to do with a controller mapping.

Using XML to map the effects section doesn't provide any way to control individual parameters of effects. The library in #1054 takes care of all the details of switching between modes for you, including managing soft takeover without interfering with loading the initial state of the knobs. Map pressing the beats encoder to the showParametersButton and turning it to the dryWetKnob. As mentioned in the inline documentation, you'll have to override dryWetKnob.inValueScale() to handle signals from the encoder. You can adapt the function you already wrote for this, just make it take one parameter and return the new value. You'll also have to map your shift button to call the shift() function of the EffectUnit (unless you make the EffectUnit a property of another ControlContainer, in which case calling shift() on the parent will recursively call it on the EffectUnit).

The library has been totally refactored from how it was before. It now uses much more readable and memorable options objects instead of long lists of arcane parameters (that I couldn't even remember :P ).

@timrae
Copy link
Contributor Author

timrae commented Jan 4, 2017

Using XML to map the effects section doesn't provide any way to control individual parameters of effects.

Thanks for the feedback.

In all honesty, I don't feel that such a feature is needed. I prefer the simplicity of this PR the way it is (closely emulating the Serato mapping with minimal code). I want users to be able to use the mapping without even reading the documentation.

I don't want them to accidentally switch into parameter editing mode and then have to dig through the documentation because they can't get it back how it used to be. If somebody else wants to add an advanced MC4000 mapping for controller enthusiasts that would be great, but for now there are a bunch of other issues in mixxx that I would rather spend my time working on tbh.

It wasn't clear whether your other comments are generally applicable to this PR or only related to the hypothetical case where I use your JS library.

@timrae
Copy link
Contributor Author

timrae commented Jan 4, 2017

By the way, I had to implement the headphoneSplit() function because the headSplit control acts as a toggle whereas on the controller it's an ordinary switch. Is there an XML attribute that I can add to force it to act as a switch without having to use JS?

@Be-ing
Copy link
Contributor

Be-ing commented Jan 4, 2017

I do think users will want the ability to control individual parameters and be frustrated if the mapping does not provide that when other controller mappings do. Part of the point of writing the EffectUnit object in the library is to make it easy to map the full capability of the new effects UI. I think there should be a consistent user experience across different controllers so people can get any controller with more than basic effects controls and have a similar experience with Mixxx. One mapping implementing all the features while another does not should not be a deciding factor for people considering what controller to get.

I have just modified the library today so that it works similarly when parameters are hidden and showing. It is now possible to not have any effect focused when parameters are showing, and in this case the knobs continue to control the metaknobs just like when parameters are hidden. This is the default state; users have to explicitly focus an effect with shift + an enable button or clicking a focus button on screen to control the effect's individual parameters. If you do not want to control individual parameters, you don't have to do anything. I think this new way is both intuitive and powerful.

My intention with providing controllers with a way to manipulate individual parameters is both for live use and to help set up custom effect chains. Users could use their controllers to set parameter knobs where they want them, set up the metaknob linkings how they like, then they could just use the metaknobs while mixing.

I don't think a feature not being entirely self-explanatory is a good reason to exclude it. Master sync is not self-explanatory, but that's not a good reason not to implement it.

@Be-ing
Copy link
Contributor

Be-ing commented Jan 4, 2017

By the way, I had to implement the headphoneSplit() function because the headSplit control acts as a toggle whereas on the controller it's an ordinary switch. Is there an XML attribute that I can add to force it to act as a switch without having to use JS?

I don't think so.

@timrae
Copy link
Contributor Author

timrae commented Jan 4, 2017 via email

@Be-ing
Copy link
Contributor

Be-ing commented Jan 4, 2017

In case it wasn't clear, there's no need to use the library for the rest of the mapping to use its EffectUnit object.

@timrae
Copy link
Contributor Author

timrae commented Jan 4, 2017

Of course it was clear, but I prefer to use it for everything or nothing. At the moment it's a simple "static" mapping between physical controls and features in Mixxx. If I'm going to make it dynamic then why stop at the FX? One can argue it should be able to control four decks for example, since other controller mappings support this...

@timrae
Copy link
Contributor Author

timrae commented Jan 11, 2017

Can this be merged now? It only relies on #1062 and #953 which have both been merged.

@Be-ing
Copy link
Contributor

Be-ing commented Jan 11, 2017

LGTM. Please update the wiki page and open a Launchpad ticket for implementing control of individual effects parameters so it isn't forgotten about.

@timrae
Copy link
Contributor Author

timrae commented Jan 11, 2017

I updated the wiki. Just ping me on github when your JS library gets accepted and I'll have a play with integrating it on the MC4000.

@rryan rryan merged commit c69adca into mixxxdj:master Jan 11, 2017
@Be-ing
Copy link
Contributor

Be-ing commented Jul 14, 2017

Hi @timrae, the Components library has been merged and we've settled on the UX design and API for the EffectUnit object. @DJMaxergy and @ronso0 have used it for mapping other controllers, so I think the code and documentation are fairly robust at this point. I've done a bit of usability testing and confirmed that if you don't know about or don't want to use any of the fancy effect focusing stuff, it is completely out of your way. But if you understand how to use it, it's a lot of fun. :D Could you update the MC4000 mapping to use the library for the effects? Please ask questions if the documentation isn't clear about anything.

@timrae timrae deleted the mc4000-update branch July 14, 2017 00:15
@timrae
Copy link
Contributor Author

timrae commented Jul 14, 2017

OK, I'm not sure when / if I'll have time for this though

@Be-ing
Copy link
Contributor

Be-ing commented Jul 14, 2017

Okay. The API is pretty simple, so hopefully it shouldn't take very long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants