Skip to content

Update SCS3 mappings#762

Closed
sbalmer wants to merge 30 commits intomixxxdj:1.12from
sbalmer:1.12
Closed

Update SCS3 mappings#762
sbalmer wants to merge 30 commits intomixxxdj:1.12from
sbalmer:1.12

Conversation

@sbalmer
Copy link
Copy Markdown
Contributor

@sbalmer sbalmer commented Oct 30, 2015

This is an update to the Stanton SCS3 mappings. It features 4-deck support and controlling the filter racks. There are many things I would like to add and a few things where I am unsatisfied with the solution. But overall it works well and is stable in use.

I've practiced with this mapping for a few months now and also used it in a few livesets. Unfortunately I couldn't get feedback from other users, so it's a "works for me" deal.

There is a terse function reference which I could expand and use to update the manual once the new version is released. Forum thread: http://mixxx.org/forums/viewtopic.php?f=7&t=7335

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Oct 30, 2015

Thanks for submitting this. It may take a few days to review this. I'll see what I can get through tomorrow. Written descriptions are really helpful for reviewing, so please do keep working on that.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 2, 2015

Thanks for looking at it.

You can find the descriptions on the forum thread, http://mixxx.org/forums/viewtopic.php?f=7&t=7335#p26105 . They are very terse, and a few details changed since. I don't really know where to put them otherwise. I presume committing them with the code is not useful, because nobody would go looking for them there.

@Pegasus-RPG
Copy link
Copy Markdown
Member

Hello.

You could edit the Wiki page for the device which is the definitive user
guide: http://mixxx.org/wiki/doku.php/stanton_scs.3d

Make sure to specify with a footnote which features are "introduced in
1.12."

Sincerely,
Sean M. Pappalardo
"D.J. Pegasus"
Mixxx Developer - Controller Specialist

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 5, 2015

I apologize for the delay reviewing this. I've been busy this week with a new job and bicycle troubles.

@illuusio
Copy link
Copy Markdown
Contributor

illuusio commented Nov 5, 2015

Sorry to nack about style but 'res/controllers/Stanton SCS.3d.midi.xml' is currently very hard to read because everything is in same row.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 6, 2015

@illuusio, thanks for having a look! And @Be-ing, as a cyclist I sympathize :-)

Re style of the XML:
That's actually generated code and the lines differ in <status> and <midino> only. If you view the file with line-wrap disabled the numbers line up nicely and give a compact view of the messages caught. This is much easier to scan by eye than when every tag gets a new line. I'd like to keep it that way.

On the other hand: I'm done with that file. So if this is an issue I can autoformat it. If Mixxx offered the possibility I would just re-route all messages to the script and be done with it.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 6, 2015

@Pegasus-RPG I will certainly want to update the wiki. Because of the heavy changes I think it would be confusing to keep the different versions in the same document. I would prefer having separate pages for the pre 1.12 versions.

I'm unsure at what point I should start with that.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 6, 2015

For now, you can make a new section on the same wiki page.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 9, 2015

I added documentation for the proposed SCS3D mapping at the bottom of the page:

http://mixxx.org/wiki/doku.php/stanton_scs.3d#proposed_mapping_for_112

At some point I hope to find the time to do the SCS3M part as well.

@illuusio
Copy link
Copy Markdown
Contributor

Ok then we go to ugly part because code and WIKI LGTM.
1# There is missing header in javascript

////////////////////////////////////////////////////////////////////////
// JSHint configuration                                               //
////////////////////////////////////////////////////////////////////////
/* global engine                                                      */
/* global script                                                      */
/* global print                                                       */
/* global midi                                                        */
////////////////////////////////////////////////////////////////////////

2# you should format your XML! I do it with libxml2 'xmllint -format' or http://www.freeformatter.com/xml-formatter.html (with '2 spaces per indent level').
3# Javascript doesn't pass jshint (http://jshint.com/) which it doesn't (but that can be done in other PR because it really clutters what have been changed here) at least replace tabulators with spaces most warnings are very basic but to get on point Zero with script we should solve them. In javascript we use same styling as C++ (which can be achieved with http://jsbeautifier.org/ on basic settings)
If you have node.js you can install them on you machine.

@ywwg
Copy link
Copy Markdown
Member

ywwg commented Nov 21, 2015

A diff of this size is really Too Big To Review -- is there anyone else with a SCS3 that can test this mapping in practice? @Pegasus-RPG I think you said you have one of these?

@mixxx-buildbot
Copy link
Copy Markdown

Automated Message from BuildBot: Mixxx Admins, is this safe to test? Respond with: "ok to test", "add to whitelist", "skip ci", or "test this please" (to retest).

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 22, 2015

I don't see the "gain" Control Object being manipulated anywhere in the script. Don't you mean volume?

Changed in the wiki.

And if you do mean volume, can you incorporate a way to control the gain of the deck into the mapping?

I added that functionality.

This touches on a larger issue where there is a duplication of controls between 3d and 3m. I have not added a way to control the crossfader from the 3d for example. The pre-gain is the same, I control it from the 3m. I'm not at all sure how to approach this because people who have only a 3d might want to access controls I can reach through the 3m. Not that I think it's pressing we discuss this further, just to let people know that this is an unsettled question as far as I'm concerned.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 22, 2015

Light the LED when the new play_indicator control is 1. play_indicator behaves differently depending on the cue mode selected by the user in the preferences. Some people (such as myself) don't like blinking lights and play_indicator can be set to a mode that doesn't blink in that case. Also make the cue button LED react to the cue_indicator control for the same reason.

Ah thanks, that's how these indicator controls work. Simplifies the code too. Pity for the relaxed on-off-off blinking pattern I'd used to signal play-ready :'-)

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 24, 2015

Thanks @Be-ing for going through the documentation. I expanded the wiki to address your comments.

Wiki pages can link to each other or link to other sections within themselves. After this is merged, it'd be nice if you went back and made links where the text of the wiki page refers to a different section.

I agree.

What do you mean by "destructive"? Does that word need to be there? "Destructive" sounds scary. What is it destroying?

I meant to say that you maybe don't want to do these operations on a live deck. I use "disruptive" now.

Does the FX hold mode apply when the FX button is held down while FX mode is already active? Or can you hold down FX while in another mode, let go of the FX button, and go back to that previous mode?

No matter what mode you're in, holding down a mode-button will switch to that mode and you can perform "hold" operations. After you release the button, the device switches back to the original mode which might very well be the same mode minus the "hold" overlay.

While FX is held, the pitch mode slider can be used to load the next (touch top half of the slider) or previous (touch bottom half) effect.

What effect chain does this change the loaded effect on?

I expanded the effect loading and assignment procedure with an example. I still think it takes too much concentration to do this but I couldn't come up with a better mapping.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 24, 2015

For the 3m, it's not clear to me from reading the wiki page what button(s) switch between EQ and FX mode.

The unlabeled buttons below EQ can be used to select an effect chain to control. Holding FX allows assigning chains with the same buttons. There is no single FX mode.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 24, 2015

// Issues
// - Each deck rembembers the mode it was in, confusing? Would it be better to
// keep the current mode on deck switch?

Particularly for this device, it might depend on how the controller is being used. It may be different if this controller is being used by itself or there are multiple of them together or it is used in conjunction with another controller. It could also just be personal preference. Could you make this an option that users could easily change by changing a value of a boolean at the top of the script?

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 24, 2015

I added a little to the Hold FX section that I think makes it clearer. Please check that I am understanding it correctly. Something I am still unclear about is "While FX is held, the pitch mode slider can be used to scroll through different effects." This is referring to S2, correct? Please indicate the slider this is referring to as it is labeled in the diagram.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 24, 2015

Don't worry too much if the mapping is awkward for effects. This mapping squeezes a lot out of this little device; it shouldn't try to be the best at everything. Mixxx's effects engine is also in an awkward state, so that doesn't help. I don't think this should hold up merging this mapping, but if you come up with a better way to map effects in the future, please open another pull request.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 24, 2015

There are a handful of places scattered around the code where you use == and != instead of === and !==. Please change them to the latter unless you have a good reason to keep them how they are.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 24, 2015

Also there are a few places where you use ! to invert the value of a boolean without a space between the ! and the boolean. IMO it is generally easier to read if there is a space between the ! and the boolean. @illuusio , what do you think about this?

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 24, 2015

A few comments explaining the differences between similarly named functions would be helpful. What are the differences between "watch", "receive", and "expect"?

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 24, 2015

Particularly for this device, it might depend on how the controller is being used. It may be different if this controller is being used by itself or there are multiple of them together or it is used in conjunction with another controller. It could also just be personal preference. Could you make this an option that users could easily change by changing a value of a boolean at the top of the script?

The simpler mapping would just reference the same state-variable for all four decks, instead of having a separate state variable for each deck. This should be solvable at initialization I think. So the effort would be small. I'll try adding this.

@Pegasus-RPG
Copy link
Copy Markdown
Member

Okay @sbalmer I've finally had a chance to test these.

  • I found a bug on the SCS.3m: the high and low EQ positions are swapped, at least on the left side.
  • Also I've been able to confuse the spinny dot on the SCS.3d where only one of the 16 ring LEDs flashes. (Don't ask me how. I just know it happened after multiple track loads. It might be a problem in Mixxx itself.)

In our Certified presets, we try to match functionality and behavior with that of manufacturer-supplied presets for other DJ software as much as we can, ensuring a consistent user experience and making it easy for someone to switch to Mixxx and be able to use it right away with familiar controller behavior. As well, we prefer not to change or remove functionality that was working well in a previous version so as to maintain consistency for upgrading users as much as possible.

To these ends, your presets behave quite differently than the previous versions, even in modes that were fully implemented in the originals. There's nothing wrong with this of course (especially since these controllers beg for customization!) but it does mean we wouldn't replace the official presets with these. We would however be happy to include them as additional options. Please rename the files (add "-sbalmer" or some other suffix of your choice before the .js/.midi.xml) so they can be differentiated. (We may be able to converge on the SCS.3m preset though.)

You're welcome to have only your name as the author too. I appreciate the acknowledgement of my work, but you did indeed (re)write just about all of the code in your presets! Plus, if anyone has questions about specifics, they would know the right person to ask. That's another big reason we include the Author field. You can acknowledge me in a script code comment if you want. :)

Thank you again for the time you've spent on these, for sharing them with the community, and for your patience with us!

@illuusio
Copy link
Copy Markdown
Contributor

@Pegasus-RPG Nice review and I'm with you. Let's have option for users but then we have to make more explanation what is the diffrence between these two.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 26, 2015

I added a little to the Hold FX section that I think makes it clearer.

Thanks for helping with that, I fixed the slider name to S2.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 26, 2015

I found a bug on the SCS.3m: the high and low EQ positions are swapped, at least on the left side.

Ah thanks, I've been confused by that myself but thought I remembered it wrong, not the device :-)

Also I've been able to confuse the spinny dot on the SCS.3d where only one of the 16 ring LEDs flashes. (Don't ask me how. I just know it happened after multiple track loads. It might be a problem in Mixxx itself.

So it worked at first then all of a sudden only one led flashed when the playposition passed? Did the rest of the device function normally, including lighting the blue LED in LOOP or TRIG modes? Was it one of the top or bottom LED that kept working, or one in-between?

I know it might be hard to remember or reproduce. To me it sounds like internal state in the script was garbled, which is always a very bad sign.

To these ends, your presets behave quite differently than the previous versions, even in modes that were fully implemented in the originals. There's nothing wrong with this of course (especially since these controllers beg for customization!) but it does mean we wouldn't replace the official presets with these. We would however be happy to include them as additional options. Please rename the files (add "-sbalmer" or some other suffix of your choice before the .js/.midi.xml) so they can be differentiated. (We may be able to converge on the SCS.3m preset though.)

I see how the 3d mapping cannot be reconciled. I'll try to add a classic mode where the mode buttons work like in the original. But I fear it would become too hard to maintain. So I'll probably branch into a separate file. The good news of that to me is that I don't have to keep any pretenses that the mapping is a continuation, which makes it easier for me to change a few more details.

Could you expand a bit on the 3m, what would have to change? I think keeping the number of alternative mappings down is a worthy goal but I'm afraid I'm stuck in my ways now ;-)

You're welcome to have only your name as the author too. I appreciate the acknowledgement of my work, but you did indeed (re)write just about all of the code in your presets! Plus, if anyone has questions about specifics, they would know the right person to ask. That's another big reason we include the Author field. You can acknowledge me in a script code comment if you want. :)

Thanks, I'll do that. And thanks for looking at them and your comments! It's very helpful to know they work on somebody else's machine too :-)

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 27, 2015

Re negation style, these are the possibilities:

1. if (!flag)
2. if (! flag)
3. if ( ! flag)

On a quick survey, most mappings use style one, this is also the style I'm most comfortable with. Style 2 is used in Electrix-Tweaker-scripts.js, not sure whether there are other examples. And style three is what I would argue would be most useful to make the negation more visible, but I didn't see that style used anywhere.

The dominant style is style one so my laziness insists there be a reason to change it. Unless there is a "new good" style I'd prefer leaving it as-is. I do think the other styles may have a slight advantage regarding visibility, so I'm somewhere between neutral to sympathetic.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 27, 2015

I don't have a strong opinion either way about spacing with !. So, LGTM :)

Thank you for sticking around for this long review process, and again I apologize for taking so long to start it. I encourage you to join the mailing list (https://lists.sourceforge.net/lists/listinfo/mixxx-devel ) to stay involved. I'm planning on making a JS library soon to make mapping easier. You're clearly quite knowledgeable about JS, so your input would be appreciated.

I moved your description of the changes to the SCS3m mapping to the main SCS3m wiki page (http://mixxx.org/wiki/doku.php/stanton_scs.3m ). I think it's unnecessary to have two separate pages for a controller.

I am in favor of replacing the old 3m mapping with this one. Are there any features removed from the old 3m mapping that should be added back, @Pegasus-RPG ?

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Nov 27, 2015

I'm surprised by the level of attention I get and I appreciate it! I think that software will be ready when it's ready. I'm not worried about a long process as long as this helps quality. I feel this is not only the motivation but also the result in Mixxx. If anything I'm worried that you lot spend far too much time on my dinky controllers :-)

I'm already on the mailing-list, and I loosely follow the discussions. I think using a declarative style in the mappings makes them easier to work with. I have a few ideas on how to expand on it but in the end what I have now is working. I didn't look much into it but I think mixco https://www.npmjs.com/package/mixco goes further in this approach. I might choose that library if I started anew.

Most people are more accustomed to procedural logic and I'm not sure how much we should 'force' an unfamiliar style. I'm willing to help anyway. (Um, time permitting and all that.)

@Pegasus-RPG
Copy link
Copy Markdown
Member

The good news of that to me is that I don't have to keep any pretenses that the mapping is a continuation, which makes it easier for me to change a few more details.

Right. I don't want anyone limiting their creativity! :)

Could you expand a bit on the 3m, what would have to change? I think keeping the number of alternative mappings down is a worthy goal but I'm afraid I'm stuck in my ways now ;-)

I agree, it would be best to have just one, and since your coding style is easier for most people to read, I'd rather just use yours. I don't exactly know what would need to change as I haven't examined the behavior closely. But I will in the next few days.

@sbalmer
Copy link
Copy Markdown
Contributor Author

sbalmer commented Dec 13, 2015

I created fresh commits with separate files for my '4deck' version and made pull request #809. My commits in this pull request would lead to a confusing version history. I hope you agree that starting a new PR is a good way to solve this.

Thanks for all the feedback!

@sbalmer sbalmer closed this Dec 13, 2015
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.

6 participants