Skip to content

Dj compact#918

Merged
ywwg merged 13 commits intomixxxdj:masterfrom
ywwg:dj-compact
Dec 22, 2016
Merged

Dj compact#918
ywwg merged 13 commits intomixxxdj:masterfrom
ywwg:dj-compact

Conversation

@ywwg
Copy link
Copy Markdown
Member

@ywwg ywwg commented Apr 7, 2016

I need to do a little testing but I think this is good enough for initial commit.

feel free to bikeshed the naming scheme

ywwg added 3 commits April 3, 2016 16:17
It's possible to send different light information when Shift is held.
For now we just send the same lights either way.
}

HercDJCompact.init = function(id) {
scratch = false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be an array to keep track of it for each deck?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nevermind, I see that this device has a single "Scratch" button for both decks. It'd be helpful to add a comment here about that.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 7, 2016

Do you plan on documenting this now or when you finish the mapping? There's already a wiki page started for a different, unfinished mapping: http://mixxx.org/wiki/doku.php/hercules_djcontrol_compact

}
}

if(engine.getValue(input.group, "scratch2_enable")) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO the code would be easier to understand if this was moved up above line 52. I wasn't clear what lines 54-56 did until I read this far.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 7, 2016

Looks good so far other than the minor issues pointed out above. There's not much more to comment on without documentation.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 7, 2016

Before @sblaisot has to jump in to say it :P
Update the Windows uninstaller to include the new files: https://mixxx.org/wiki/doku.php/contributing_mappings#windows_installer_update

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Apr 7, 2016

Do you plan on documenting this now or when you finish the mapping? There's already a wiki page started for a different, unfinished mapping: http://mixxx.org/wiki/doku.php/hercules_djcontrol_compact

I was given this controller by hercules for the purpose of making an official mapping, so yes I'll be documenting it. As I stated this is just an initial commit.

@sblaisot
Copy link
Copy Markdown
Member

sblaisot commented Apr 8, 2016

please add file removal from windows uninstaller
https://mixxx.org/wiki/doku.php/contributing_mappings#windows_installer_update

EDIT : Damn it! @Be-ing has been faster than me ;)

"HercDJCompact.OnRecordingStatusChange");

// Tell controller to send midi to update knob and slider positions.
midi.sendShortMsg(0xB0, 0x7F, 0x7F);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool! I didn't see anything about this in the P32 MIDI documentation, but it works on that too! :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah it's a great feature. (It was doc'ed in the pdf on hercules' site). I'm really impressed with the midi programming on this controller, a lot of convenience features.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tried this with my Electrix Tweaker just to see if it would work but it didn't. Is this a Hercules thing? I think I recall looking at one other mapping that had such a feature, but I don't remember what mapping it was or what MIDI signal was sent.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

my vestax VCI400 just does it on connect, for instance, no midi trigger required. It's certainly not any sort of standard, so I'd guess that someone at Hercules thought it up and they reuse firmwares on multiple devices.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Apr 30, 2016

notes addressed

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 30, 2016

Hm, considering this controller lacks a high EQ knob, would it be helpful to (optionally?) map the mid EQ knob to the HPF (QuickEffect parameter3) or QuickEffect superknob?

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Apr 30, 2016

I am writing a framework for creating "controller preferences," which will allow this type of setting to be changed in the UI. For now I'll add a comment that users can remap the knob with the midi wizard (rather than adding javascript), or maybe even put some comments in the xml about how to do this.

When controller prefs comes in, I'll make a proper set of preferences for the knob.

It is a little silly that it's set to "mid" :/.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 30, 2016

I am writing a framework for creating "controller preferences," which will allow this type of setting to be changed in the UI.

Awesome!

For now I'll add a comment that users can remap the knob with the midi wizard (rather than adding javascript), or maybe even put some comments in the xml about how to do this.

Sounds good.

</options>
</control>
<control>
<group>[EqualizerRack1_[Channel2]_Effect1]</group>
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing Apr 30, 2016

Choose a reason for hiding this comment

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

Would it be better to use the description element for this? That shows up in the mapping GUI.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the description tells them to edit the XML, but I don't think that's the place to go into this detail. This comment is right at the top of the file so if someone does get as far as editing the XML, they'll find it (I hope)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alright, fine to leave it as is then

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Apr 30, 2016

lgty?

<name>Hercules DJControl Compact</name>
<author>Owen Williams</author>
<description>Controller mapping for Hercules DJControl Compact. Users
who want the labelled "mid" knob to adjust high eq instead may use the midi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

spelling: "labeled"

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 30, 2016

JSHint warnings:
res/controllers/Hercules-DJControl-Compact-scripts.js: line 4, col 17, Weird construction. Is 'new' necessary?
res/controllers/Hercules-DJControl-Compact-scripts.js: line 6, col 1, Missing '()' invoking a constructor.
res/controllers/Hercules-DJControl-Compact-scripts.js: line 6, col 2, Missing semicolon.
res/controllers/Hercules-DJControl-Compact-scripts.js: line 52, col 19, Use '!==' to compare with '0'.
res/controllers/Hercules-DJControl-Compact-scripts.js: line 72, col 19, Use '!==' to compare with '0'.
res/controllers/Hercules-DJControl-Compact-scripts.js: line 83, col 2, Missing semicolon.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 30, 2016

I'm guessing the beatlooproll_X_activate mappings are for loop mode with shift pressed. Is that correct? Could you document the mapping on http://mixxx.org/wiki/doku.php/hercules_djcontrol_compact ? I think 2x2 tables describing the pad functionality in each mode would be helpful.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Apr 30, 2016

I am writing a framework for creating "controller preferences," which will allow this type of setting to be changed in the UI.

A bit off topic, but how do you intend to implement this? I think passing a QScriptValue containing all the options to the script's init function could work well.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Apr 30, 2016

wiki will be updated after this is in.

RJ and I have been collaborating on a design for controller preferences, it's similar to what you describe. Once it's settled I'll post the design doc to the mailing list.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented May 3, 2016

ping

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented May 4, 2016

As I said before, there isn't much to comment on without having documentation to reference. I can't tell if it looks good unless I wen through the MIDI documentation for this device and compared it to the XML.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented May 4, 2016

you're not expected to go through every button in/out and confirm that it's correct, this review is mostly for the style of the javascript functions. I've gone through all the buttons and functions I've mapped and made sure the controller functions nicely.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Dec 11, 2016

updated documentation. Most functions are the same as manual (linked on wiki), differences are noted. http://mixxx.org/wiki/doku.php/djcontrol_compact

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Dec 11, 2016

Thanks for documenting it. I added pictures of the device to the wiki from the other wiki page, reorganized the page, and made some edits. There is so little information to convey that I think referring users to Hercules' manual is more of an annoyance, so I put the little bit of information from there onto the wiki. Please confirm that it's all correct.

I like the way you've mapped the pads in loop mode. That is more flexible than the way described in Hercules' manual.

I think it would be a better use of the sampler buttons to map the left side to samplers 1-4 and right side to samplers 5-8.

It doesn't look like you have mapped shift+play to anything. I find it useful to map this to start_stop (go to beginning of track and stop).

After the changes in progress for the effects UI (see PRs #1062, #1063, #1068), I think the mapping for the effects pad mode should be reconsidered. I think enabling deck 1 for EffectUnit1 and deck 2 for EffectUnit 2 in the init function with each side toggling the effects within the EffectUnits would make sense. Shift + a pad could cycle to the next effect.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Dec 12, 2016

There's a word or two missing in your edit of the jog wheel instructions: "Enable keylock on a deck by clicking the if you do not want the pitch to change when adjusting the tempo."

I'll let you know when I've addressed the notes.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Dec 18, 2016

Mixxx only has 4 samplers on startup, so specifying samplers 5-8 causes a warning to appear

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Dec 18, 2016

Do any of our skins support more than four samplers? It doesn't look like either Latenite nor Deere do

Fix jog wheels while playing
Change samplers, although those samplers don't exist
@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Dec 18, 2016

Mixxx only has 4 samplers on startup, so specifying samplers 5-8 causes a warning to appear

Create more by setting [Master],num_samplers to 8 in the script's init() function.

Do any of our skins support more than four samplers? It doesn't look like either Latenite nor Deere do

Yes, Deere does by flipping through the multiple pages of samplers available. It's clunky, but it's there.

@ywwg
Copy link
Copy Markdown
Member Author

ywwg commented Dec 21, 2016

Create more by setting [Master],num_samplers to 8 in the script's init() function.

done

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Dec 21, 2016

LGTM 👍

@ywwg ywwg merged commit 4c2419d into mixxxdj:master Dec 22, 2016
@ywwg ywwg deleted the dj-compact branch December 22, 2016 01:19
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