WIP: MIDI Fighter 64 driver #69
Conversation
| } else { | ||
| if (timeLeft > SPIN_YIELD_PRECISION) { | ||
| yield(); | ||
| Thread.yield(); |
There was a problem hiding this comment.
It wouldn't compile unless I did this. Is there a reason Thread. is currently omitted?
There was a problem hiding this comment.
Weird, don't know why it would refuse to compile, maybe you have a strict setting on. But I'm also surprised my IDE didn't complain. Fixed here: 441bbb0
|
The MF64 driver is intended to support two operations:
The first one's working well. I'm not sure how best to implement the second one. Currently I'm just calling LX.log() instead. |
|
Hi @raldi, it's nice to see another midi controller getting spun up. I'm curious what you're using this for? We have an early model lying around and I could see it as a lighting effects controller from the dj booth. Midi buttons of course can be mapped at runtime from the UI... the downside of custom mappings is they are one-directional, ie the lights don't change on the controller when a button is mapped. Maybe a nice implementation for a generic midi controller like this in LX would be for the surface to detect new midi mappings and then listen to the mapped parameter and sync the button's light accordingly. It would be great to have one color for parameter=off and one for parameter=on so the user can see which buttons have been mapped. The surface would still be open for generic mapping but the behavior would be two-directional. We could use that one other midi surfaces as well. If the color syncing was handled, then linking to a button from code would be as simple as adding a mapping to the midi engine. |
|
We just want to have patterns be able to color the buttons and then do whatever they want when they're pressed. |
mcslee
left a comment
mcslee
left a comment
There was a problem hiding this comment.
The code looks sensible enough, I just have fairly minor style comments. But higher-level, I don't think I'd understood how you were planning to use this, and don't think it should be an LXMidiSurface anymore. I thought you were going to have the buttons perform generic functions, e.g. activating stuff the clip grid, or selecting channels/patterns, etc.
But if you're looking for a more project-specific way of doing custom I/O mappings, then I think you should just make a global singleton instance of this class on your project and bake access to it into your TE abstractions. You won't need to reproduce much of the LXMidiSurface stuff... just find the right LXMidiInput and LXMidiOutput at initialization time, then construct your object and add a listener to the LXMidiInput so you get those input messages.
Just note that if you're going to have patterns be able to do whatever they want, you'll need some way of sorting out which pattern is active or focused at any given time... since multiple patterns can be running at the same time on multiple channels, or through a transition, and might both try to speak to the controller.
That all make sense?
| } else { | ||
| if (timeLeft > SPIN_YIELD_PRECISION) { | ||
| yield(); | ||
| Thread.yield(); |
There was a problem hiding this comment.
Weird, don't know why it would refuse to compile, maybe you have a strict setting on. But I'm also surprised my IDE didn't complain. Fixed here: 441bbb0
| // Holds the colors that will be sent out to the device. This array | ||
| // starts with (0,0), (0,1), ... (7,7) of the LEFT page, and then | ||
| // the same ordering for the RIGHT page. | ||
| private int[] colors; |
There was a problem hiding this comment.
I would make this private final int[] since you always copy into this array, you don't replace it. Some constants for the size checks would be great too.
public static final int NUM_ROWS = 8;
public static final int NUM_COLUMNS = 8;
public static final int GRID_SIZE = 64;
public static final int NUM_PAGES = 2;
public static final int NUM_COLORS = GRID_SIZE * NUM_PAGES;
There's a lot of magic 64/128 floating around.
| } | ||
|
|
||
| public void setColors(int[] newColors) { | ||
| if (newColors.length != 128) { |
There was a problem hiding this comment.
NUM_COLORS or maybe even better this.colors.length
| } | ||
| } | ||
| System.arraycopy(newColors, 0, this.colors, 0, 128); | ||
| this.sendColors(); |
There was a problem hiding this comment.
General LX coding style, I don't include the this. on method calls. For member variables I find it helpful to quickly disambiguate between arguments. But static function calls are (or should be) so rare in Java that I don't find it adds much on method invocations.
| throw new IllegalArgumentException("newColors contained value " + newColors[i]); | ||
| } | ||
| } | ||
| System.arraycopy(newColors, 0, this.colors, 0, 128); |
There was a problem hiding this comment.
I'd actually just use this.colors.length instead of 128 or a constant
|
|
||
| public MidiFighter64(LX lx, LXMidiInput input, LXMidiOutput output) { | ||
| super(lx, input, output); | ||
| this.colors = new int[8*8*2]; |
| @Override | ||
| public void dispose() { |
| } | ||
|
|
||
| @Override | ||
| public void noteOffReceived(MidiNote note) { |
| } | ||
| } | ||
|
|
||
| private void initialize(boolean reconnect) { |
There was a problem hiding this comment.
You don't call initialize in the base case, so I might actually ditch this method and just put the sendColors() call directly in the reconnect case. Otherwise this method's naming/existence is potentially confusing, as there's no actual first-time initialization.
|
|
||
| @Override | ||
| public void noteOnReceived(MidiNoteOn note) { | ||
| Mapping mapping = new Mapping(note); |
There was a problem hiding this comment.
Were you going to store these Mappings for re-use somewhere? Allocating a new object on each note received doesn't seem like the best approach, could just be a method call
|
Review comments aside - I do think there's a sensible use case for making a generic MidiFighter64 control surface implementation in which the big grid of buttons is a quick pattern selector on channels. e.g. each column is a channel and the light-up buttons are the patterns on that channel, and you quickly pop them to switch patterns. Same as what the APC40mk2 grid does, basically. Same could go for clip mode, use the side-button to toggle modes and have option to control either of those things fully generically in a control-surface style. |
3 participants
No description provided.