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

Add mapping for DJControl Starlight #2061

Merged
merged 45 commits into from
Jun 12, 2019

Conversation

kerrickstaley
Copy link
Contributor

@kerrickstaley kerrickstaley commented Mar 22, 2019

Add mapping for DJControl Starlight controller.

The original version was done by DJ Phatso, I touched up the code and added some features/tweaks (see forum thread https://www.mixxx.org/forums/viewtopic.php?f=7&t=12570&sid=a6efb78f13c1d2b7ce559e78de306be5).

@kerrickstaley
Copy link
Contributor Author

I fixed the unused variables warning (by disabling that check for the file; eslint doesn't seem to provide a nicer way to do this other than repeatedly disabling and disabling the check, which adds a lot of noise). Please review when you have time!

@uklotzde uklotzde added this to the 2.3.0 milestone Apr 24, 2019
@kerrickstaley
Copy link
Contributor Author

The Codacy linter is giving several false positives like

The numeric literal '0x91' will have at different value at runtime.

This is a known issue (here is the issue on GitHub) with older versions of the PMD linter library that Codacy uses. They use PMD 5.0.0, and the issue was fixed in the PMD 6.5.0 release about 10 months ago.

We can fix this issue by changing the hex literals to decimal literals, but I think that would make the code less readable. I'll look into how we can disable this check for the file.

@kerrickstaley kerrickstaley force-pushed the add-djcontrol-starlight-mapping branch from cb1ea96 to bf08ad3 Compare May 30, 2019 02:53
@kerrickstaley
Copy link
Contributor Author

kerrickstaley commented May 30, 2019

@uklotzde this is now ready for review, can you assign a reviewer?

Codacy upgraded their version of PMD so now there should be no Codacy issues.

@uklotzde
Copy link
Contributor

@DJPhatso Any comments on this PR? You might be able to test the mapping.

@kerrickstaley Has the wiki page been updated according to this PR?

If the mapping works as documented and if there are no obvious coding issues we can merge it. I'm neither a JS nor mapping wizard so someone else might chime in.

@DJPhatso
Copy link
Contributor

@uklotzde Sure, I can test out the updated version. Where/how to I get the files? (Sorry, nooby here...)

@uklotzde
Copy link
Contributor

@DJPhatso Direct GitHub links for download: XML / JS

@DJPhatso
Copy link
Contributor

First thing I noticed is that the issue with the Vinyl button @kerrickstaley had initially corrected seems to be back.

@kerrickstaley
Copy link
Contributor Author

@DJPhatso I can't seem to reproduce the issue you're describing. On my setup with this .xml file and .js file, the [Vinyl] button is lit up when you start Mixxx, pressing it once disables vinyl mode and turns off the light, and pressing it again turns vinyl back on.

Can you double-check that there aren't any other .xml/.js files for the DJControl Starlight in your controllers folder? Also, can you describe the exact sequence of steps you did and what happened?

@DJPhatso
Copy link
Contributor

DJPhatso commented Jun 3, 2019

@kerrickstaley Can't reproduce it either. Might have been only on the first trial, but the controller folder was empty before copying the files. I even tried another computer this morning with a fresh install of Mixxx and still no luck.

I just loaded a song and tried to scratch and it didn't start until lifted my finger and tried again so it reminded me of the original issue, but thinking back on it now, I'm wondering if it wasn't something with the jog touch instead.

@kerrickstaley
Copy link
Contributor Author

@DJPhatso one thing I've noticed with this controller is that sometimes the jog wheels don't correctly register touches on the top surface (so you get a bend instead of a scratch). I think it's a hardware issue with my specific unit but may also be a problem that affects your unit. It's not related to the software since I see the same issue in Serato.

@DJPhatso
Copy link
Contributor

DJPhatso commented Jun 3, 2019

@kerrickstaley Good to know. I'll keep an ear out and ask a few questions about that. I've had many during development and never encountered that issue, but not impossible that something could go wrong once in a while during production.

@uklotzde
Copy link
Contributor

Ready to merge? Code is readable, Wiki page exists. LGTM.

@kerrickstaley
Copy link
Contributor Author

kerrickstaley commented Jun 12, 2019

Yes, this is ready to merge from my end.

@uklotzde
Copy link
Contributor

LGTM, thank you! We still have the beta phase if any fixes are needed.

@uklotzde uklotzde merged commit 11e8abe into mixxxdj:master Jun 12, 2019
@reginaldojunior
Copy link

This is disponible what version?

@Holzhaus
Copy link
Member

This is disponible what version?

This mapping will be part of the 2.3 release.

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.

7 participants