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.

Sign up for GitHub

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

Jump to bottom

controllers: ION Discover DJ Pro #266

Merged
merged 1 commit into from
Mar 11, 2021
Merged

controllers: ION Discover DJ Pro #266

merged 1 commit into from
Mar 11, 2021

Conversation

meltedpianoman
Copy link
Contributor

As requested by mixxxdj/mixxx#2893

I add the ION Discover DJ Pro hardware to the controller hardware section in the Mixxx manual.

@meltedpianoman meltedpianoman marked this pull request as draft October 29, 2020 20:40
@meltedpianoman meltedpianoman marked this pull request as ready for review October 29, 2020 20:41
@meltedpianoman
Copy link
Contributor Author

@Holzhaus : Can you please have a look?
Thanks!

@meltedpianoman meltedpianoman mentioned this pull request Oct 29, 2020
Merged
@Holzhaus
Copy link
Member

Holzhaus commented Oct 29, 2020 •
edited
Loading

Thanks. Here are some vector images of the Controller: ion_discover_dj_pro.zip

Can you add numbered labels with Inkscape and then document the controls like on the Pioneer DDJ-200 page? The font for the labels is "Open Sans". If possible, try to use the same numbering as in the official manual.

EDIT I just googled the controller manual PDF. I think it's easier to read if you have 2 sections (deck and mixer) like on the DDJ-200 page instead of one large diagram with all controls numbered. Check the DDJ-200 or the Roland DJ-505 pages, this is how controller documentation should look like IMHO.

@Holzhaus Holzhaus marked this pull request as draft November 4, 2020 15:56
@Holzhaus Holzhaus changed the base branch from manual-2.3.x to 2.3 November 6, 2020 19:41
@meltedpianoman
Copy link
Contributor Author

Hi @Holzhaus , I updated the manual as you requested similar to the Pioneer DDJ-200 with a separate Deck & Mixer section.

Can you please have a look and give me your comments on this?

Thanks,
Ivo

@Holzhaus
Copy link
Member

Holzhaus commented Feb 6, 2021

Can you please open the SVG files in Inkscape and save them as "Optimized SVG" to reduce line count and file size?

Holzhaus
Holzhaus reviewed Feb 6, 2021
View reviewed changes
Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

Thank you.

What about the STUTTER buttons, the ALL MUTE buttons and the CUE VOLUME/MASTER VOLUME knobs?

These are currently not documented.

source/hardware/controllers/ion_discover_dj_pro.rst Outdated Show resolved Hide resolved
source/hardware/controllers/ion_discover_dj_pro.rst Outdated Show resolved Hide resolved
source/hardware/controllers/ion_discover_dj_pro.rst Outdated

- Library browsing
- Cue control
- Autoloop
Copy link
Member

Choose a reason for hiding this comment

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

This should be easy to map. Check the beatloop_X_activate controls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my remarks below... I really would like to finish this controller mapping and leave some features that are not yet working for the community to work on further... I hope you can live with that.

source/hardware/controllers/ion_discover_dj_pro.rst Outdated Show resolved Hide resolved
@meltedpianoman
Copy link
Contributor Author

Thank you.

What about the STUTTER buttons, the ALL MUTE buttons and the CUE VOLUME/MASTER VOLUME knobs?

These are currently not documented.

Hi @Holzhaus,

The CUE volume & Master volume knobs indeed control the audio of the two internal sound output channels, so I indeed will document that.

I understand the controller mapping is far from perfect now. What I mainly did was taking the intial effort from joachim and updated that one to the new XML scheme, inverted some sliders and fixed a filepath that did not work on a case sensitive file system like on an Linux machine.

I'm personally not a DJ, but I'm trying to get this controller supported in Mixxx so my son can use it (also with future versions of the Mixxx software). I was hoping that even though it is not complete and perfect, it at least is a first start to get this controller officially supported by Mixxx. Maybe other users that are missing certain functionality will make this controller map a little bit better in the future... My guess is that still some users are glad to have this controller mapping with at least some basic functionality, instead of no support for this controller at all.

The STUTTER & ALL MUTE buttons are currently not mapped at all. I propose that I list that in the "not yet working" section, together with the AUTOLOOP buttons.

Is it acceptable by you to accept the current state of this controller mapping, even with its shortcomings? If yes: then I'll update the manual with your remarks and upload it for another review. If not: then I'm afraid that I'm going to give up on this Pull Request because I find it difficult to sufficient find time to contribute further on this. Don't get me wrong: I understand that you have to guard the quality of the mixxx software. So if the status of this controller is not acceptable, I'll can accept that.

Regards,
Ivo

@meltedpianoman meltedpianoman marked this pull request as ready for review February 22, 2021 21:06
Holzhaus
Holzhaus requested changes Mar 7, 2021
View reviewed changes
Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to review. Already looks good, just two removals, then we can merge IMHO. Thanks you.

source/hardware/controllers/ion_discover_dj_pro.rst Outdated Show resolved Hide resolved
source/hardware/controllers/ion_discover_dj_pro.rst Outdated
Comment on lines 38 to 39
.. note:: Only controls are documented that are known to be working.

Copy link
Member

Choose a reason for hiding this comment

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

This should be removed, this is redundant with the "Known Issues" section.

Suggested change
.. note:: Only controls are documented that are known to be working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Holzhaus
Holzhaus approved these changes Mar 11, 2021
View reviewed changes
Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@Holzhaus Holzhaus merged commit 70cee93 into mixxxdj:2.3 Mar 11, 2021
Holzhaus
Holzhaus reviewed Mar 11, 2021
View reviewed changes
source/hardware/controllers/ion_discover_dj_pro.rst

- `Mixxx User Forum <https://mixxx.discourse.group/t/ion-discover-dj-pro-mapping-script/12120>`__

.. versionadded:: 2.2.5
Copy link
Member

Choose a reason for hiding this comment

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

Oops, there will be no 2.2.5, I'll change this to 2.3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thank @Holzhaus

But.. what about the GitHub wiki?
Is that no longer maintained?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Holzhaus Holzhaus Holzhaus approved these changes

Assignees
No one assigned
Labels
controllers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@meltedpianoman @Holzhaus @ronso0