-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
midi-components-0.0.js: Add JogWheel component #4474
Conversation
if (engine.isScratching(this.deck)) { | ||
engine.scratchTick(this.deck, value); | ||
} else { | ||
this.inSetValue(value); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assumes a 1 : 1 correspondence between scratchTick values and jog
values. Is this appropriate for all hardware?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true, maybe this.inSetParameter(this.inValueScale(value));
is more appropriate? I don't really know what scales jog
is even supposed to be used with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really know what scales jog is even supposed to be used with.
Does anyone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our docs even say "jog" is deprecated?!
Deprecated since version ??: Use the JavaScript engine.scratch functions instead.
Is that even true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I'd say that the ticks per rotation are somewhat proportional to the physical wheel size. I'd say this is fine for now. If you disagree, I could add a jog scaling factor to the component to make this tweakable if it really needs to.
I'd say this is a decent stop-gap solution and usable wrapper in case we ever decide to change the CO API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"jog" and "wheel" control objects have varying amounts of magic to try to get them to work well, like magic numbers that multiply their values depending on play/pause status. I always end up needing to adjust these parameters in my javascript anyway. People still use them because they are easy to write for -- the jog CO has a built-in filter to smooth out and decay values.
I would say the jog control is ok to use for a first implementation, but it would be nice to move the filtering / processing code out of C++ (it's in ratecontrol.cpp and rotary.cpp) and into javascript to drive the scratch2 parameter instead. That could be done in a followup.
I would not worry about relying on "jog" here -- it may be "deprecated" or not preferred, but that code is not going anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, but this misses the point a bit. The point of introducing this JogWheel component was to add an abstraction around our COs, so most users don't have to deal with the quirks of the current system and can just map the input handlers without having to worry about the details in most cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but you asked whether the jog CO was deprecated, so I thought it might be useful to have some background / context information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, whatever it currently is, we should probably clarify it in the manual then because it says Use the JavaScript engine.scratch functions instead.
there
friendly ping ;) |
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474 and mixxxdj#4495
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474 and mixxxdj#4495
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474 and mixxxdj#4495
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
This PR is marked as stale because it has been open 90 days with no activity. |
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Another friendly ping.... |
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
I will take a look at this! |
Thank you @ywwg for taking the time to review this long-standing PR. |
d1a877b
to
6001b21
Compare
done |
6001b21
to
e0b5c79
Compare
Sorry I had to rebase on a newer 2.3 commit to fix the failing pre-commit hooks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing this! Do you want to work on the clock-based jogwheel component next? I am happy to guide / review that.
Please merge at your leisure once you've fixed any conflicts etc.
Not yet really. This PR had priority for me because its a prerequisite for #4513. Also I don't really know how the
Thank you for finally reviewing this. I kinda lost hope. I'd really appreciate if you could take a look at #4513 next since that is also way overdue (also its a fairly idiomatic ComponentsJS-based implementation so I think the required review work should be minimal). |
Introduces a component intended to be mapped to the standard jogwheel encoder + touch sensitive surface combo present on most controllers nowadays. Co-authored-by: Owen Williams <owen-github@ywwg.com>
e0b5c79
to
8d60978
Compare
Added wiki entry https://github.com/mixxxdj/mixxx/wiki/Components-JS#jogwheelbasic |
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512
Initial commit of a mapping for the Reloop Ready controller. The Padmodes are not implemented and just default to the hotcues right now. The mapping currently depends on mixxxdj#4474, mixxxdj#4495 and mixxxdj#4512 fix(controllers): Reloop Ready fx2 level knob Co-authored-by: majekw <majek@w7i.pl> various features (i'll squash later anyways) * Add option to correct for offset tempo fader notches * Improve LED feedback of various buttons when pressing and shifting * add eject function to load button when shift-pressing * remove dad code * turn of big pads when shutting down * fix 4th pad in ManualLoopPadMode (loop_in_goto) * Add beatJumpMode for ScratchBank * fix slip button Add pitch play mode (WIP) (untested)
Introduces a component intended to be mapped to the standard
jogwheel encoder + touch sensitive surface combo present on
most controllers nowadays.
I'm targeting 2.3 because I'm planning to use the component for a 2.3 mapping.