Skip to content

Numark N4 remake#1815

Merged
Be-ing merged 38 commits intomixxxdj:2.2from
Swiftb0y:N4v2
Nov 26, 2018
Merged

Numark N4 remake#1815
Be-ing merged 38 commits intomixxxdj:2.2from
Swiftb0y:N4v2

Conversation

@Swiftb0y
Copy link
Copy Markdown
Member

@Swiftb0y Swiftb0y commented Sep 21, 2018

I felt very limited by the old mapping so I created my fully fledged remake of the old mapping.
Features:

  • seamless integration of crossfader orientation and PFL-buttons (work differently on the N4 than in Mixxx)
  • all the necessary controls for looping
  • support for up to 16 hotcues
  • full control over the library navigation
  • basic fx unit control
  • switching Deck3&4 between PC- and line-in
  • used components.js for better read- and maintainability
  • all the usual/labeled controls on the N4 work
    I tried to PR the mapping earlier this year but it still had too many bugs and I didn't have time to fix them back then, so I closed the PR.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Oct 8, 2018

Thanks for taking care of this older controller. Is the wiki up to date with your latest changes?

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Oct 20, 2018

Ping @Swiftb0y, can you confirm that the wiki is up to date with the mapping before I review it?

@Swiftb0y
Copy link
Copy Markdown
Member Author

Oh sorry. I forgot about this because of #1830 but I'll try to get around updating the wiki this evening.

@Swiftb0y
Copy link
Copy Markdown
Member Author

@Be-ing turns out there wasn't much to change. The Wiki is up-to-date now.

@rryan
Copy link
Copy Markdown
Member

rryan commented Nov 4, 2018

Status? It'd be nice to have this in 2.2!

@Swiftb0y
Copy link
Copy Markdown
Member Author

Swiftb0y commented Nov 4, 2018

Its pretty much done but I recently discovered a "bug" where the hotcuepage doesn't reset when a new track gets loaded, So I want to update that before merging. I'll try to fix that today.

@Swiftb0y
Copy link
Copy Markdown
Member Author

Swiftb0y commented Nov 5, 2018

I don't think I will be able to solve that hotcuepage issue in time as it would probably require a change within the C++ source (which I'm not comfortable with this shortly before release, especially because I can't test for unintended side effects within other mappings). https://mixxx.zulipchat.com/#narrow/stream/113295-controller-mapping/topic/Binding.20to.20whenever.20a.20Deck.20loads.20a.20new.20track

@Swiftb0y
Copy link
Copy Markdown
Member Author

Swiftb0y commented Nov 6, 2018

I already updated the wiki with my changes as well.

Copy link
Copy Markdown
Contributor

@Be-ing Be-ing 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 the delay reviewing this and thanks for your patience.

There are lots of places throughout the code that are a bit hard to read because the code is squeezed together. Please use spaces around operators and after commas. Also, please break if statements and functions across multiple lines.

Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
Comment thread res/controllers/Numark-N4-4channel-scripts.js Outdated
@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 24, 2018

I notice the name of the XML file is "Numark N4 4channel.midi.xml", but the name of the controller is simply "Numark N4". Could you rename the XML file to overwrite the old mapping?

@mixxxdj mixxxdj deleted a comment Nov 24, 2018
@mixxxdj mixxxdj deleted a comment Nov 25, 2018
@Swiftb0y
Copy link
Copy Markdown
Member Author

I have incorporated all requested changes from the review but I wasn't able to test the them yet as I don't have access to the controller right now. so DO NOT MERGE yet. I'll be able to test them tomorrow.

@mixxxdj mixxxdj deleted a comment Nov 25, 2018
Comment thread res/controllers/Numark-N4-scripts.js Outdated
Comment thread res/controllers/Numark-N4-scripts.js Outdated
@mixxxdj mixxxdj deleted a comment Nov 25, 2018
@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 25, 2018

Okay, leave a comment when you have tested the latest changes and it's ready for merge.

@Swiftb0y
Copy link
Copy Markdown
Member Author

Swiftb0y commented Nov 26, 2018

@Be-ing, The mapping is now finally ready to merge. I still have to update the Wiki with the latest tweaks but the script files are done.

@Be-ing Be-ing merged commit 1d4087a into mixxxdj:2.2 Nov 26, 2018
@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Nov 26, 2018

Great, thanks!

@mixxxdj mixxxdj deleted a comment Nov 26, 2018
@Swiftb0y Swiftb0y mentioned this pull request Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants