Conversation
|
Congratulations at this effort so far!
Implementing the sysex will be a significant effort as it appears to have a lot of bitpacking (search for "bitmask") but at least the bitpacking is rational I guess.
I have not looked at your code or editor yet -- and it'll be a bit before I can do so -- but I'm wondering how you're handling the bitmasks. So if you change, say, Voice Toggle 1, you have to send out "VoiceToggle1to8", which packs 8 parameters (voice toggles 1 through 8) together into a single NRPN value. I presume in Edisyn you have separate voice toggles for 1 through 8, rather than a single widget called "VoiceToggle1to8", and you're packing?
You can get Edisyn to dump all its parameters using NRPN, but it's gonna be slow.
On the other hand, dumping the patch like Behringer's doing it with the MIDI File Dump Standard is pretty stupid. It's extremely complex and nonstandard. :-( And it's not adequately documented. So get ready for some annoyance....
Sean
|
|
Unsubscribing for the time being (too many commit emails!). When you think the code is ready for prime time, or you need to get ahold of me for suggestions or help, just send mail to my standard email address (see the front page of the manual). |
Yes, I'm collecting all relevant parameters in a "group" and sending them in one go. Works well!
Yeah, the SysEx File Dump structure is a mystery to me! I initially thought that the sysex data was trivial to map to specific parameters, but it's still not obvious to me. As an example, here is a dump of 4 specific bytes that are changed when altering the loudness attack. The bytes are at index 28 and 29, as the sysex guide says, but then also at index 33 and 34 which, according to the guide, relate to other parameters. There's most definitely a pattern in how they change, but not a simple one. If you have any pointers, I'd be grateful :) Edit, just to clarify the structure of the dump, |
|
I think this is already useful enough to be merged (modulo any comments that you might have). File Dump support will have to be retrofitted :) |
|
I haven't tried out the code yet -- but just looking at the screenshot, I think it might do for some rearranging and compression. I try hard to keep the window proportions under a certain size so people can use Edisyn on their laptops. I might be able to help with some suggestions there... Do you have librarian support yet? Individual parameters in and out? Writing/Updating/Loading/Saving? Banks? Just wondering. |
|
I have NO IDEA why bytes 33 and 34 would be modified as well -- 33 is a byte for VCA sustain and 34 is a byte for VCA decay. You might check to see if those values are changing on the machine's front panel? It seems mysterious. |
|
I'm trying to think about how to integrate this editor. I tend only to add editors that are fully formed, but you've clearly put a lot of work into this and it might be useful as is. I would however suggest some UI changes. First off, you've got dials with long names. Try using addAdditionalLabel(...) to break a name into two or even three rows so the labels sit more closely next to one another. Second, I would stack choosers vertically using VBox, then put multiple VBoxes into an HBox, rather than the other way around; this would cause them to line up in width and look much better. I'd also absolutely stack your checkboxes in VBoxes. Third, Edisyn almost universally has stacked choosers and checkboxes to the left, followed by dials to the right. If you have lots of choosers, you might put them below perhaps. Fourth, Edisyn tries to keep its window sizes small enough for medium-resolution-screen laptops. I know you have a billion parameters, but surely there's gotta be a way to shrink this window, it's enormous. I don't like additional tabs but they might be needed. Fifth, some color stuff. Edisyn always uses white for a single category at top left of the first tab, with the name of the synth, with some general-purpose stuff in it like patch names. The others need to be the three other colors. Sixth and last (for the moment!) why do you have oscillator and filter categories on the "Main" tab? Why aren't they in the oscillator and filter tabs? Etc. |
|
@eclab Thanks for all your feedback! I'll try to address them here:
Since there are a gazillion parameters (as you mention), I thought it would be nice to have the ones that Behringer placed on the front panel in a tab by itself. This way, the UI should appear more familiar to the user.
Agree, it definitely did not fit on a Laptop screen. In the following screen dump, I've re-arranged the controls on the main tap a lot, to fit everything with in something like a Overall I think it improves the UI a great deal. I've attached a screenshot (not yet merged), what do you think?
|
|
I think the big issue here is that it's not consistent with Edisyn's other patch editor layouts, which try (not always successfully) to put choosers (combo boxes) to the left of dials and stacked vertically in lined-up groups, and check boxes either to the left of that or under them in some way. There's some value in consistency, and I break it only when I really need to cram things in or when there is really strong value for arranging things differently from the user's perspective -- but I think this isn't the case here maybe? For example you might try something laid out like this: |
|
@eclab Thanks for the mock-up, tried replicating it below. I moved the filter dials below the combos, in order to have the rows be approx. the same width. I haven't got anything for the patch names yet in the "Behringer UB-Xa" category, as this requires supporting the sysex format. If I get to that at some point, it can be fitted into that spot. WDYT?
Edit: I think I'm missing something in the modulation category, will add that later. |
|
Definitely getting closer now! Here's what you might try:
The goal here is to strike a balance between similarity with other patch editors (so the musician can rely on muscle memory and doesn't have to rely on memorizing location per-editor -- familiarity is very useful when you have this number of editors! See the mess that is CTRLR...) and space saving and window size. And also relationship to how the synth itself is laid out.... It can be a real challenge. |
|
Thinking of ways to reduce the height of the window. I'd probably:
That'd reduce the height by a lot without increasing the width at all or looking odd. What do your other tabs look like? We've just been focusing on the first tab, I guess as a Hello World. |
|
While we're at it, you've got a lot of lower-case stuff like "f-env" and "Osc2 half" or "Osc2 sync" (while elsewhere you have "OSC2"). It'd be good to have things consistent -- I tend to make everything mixed case, like "Filter Envelope" instead of "f-env" , or "Osc 2 Sync" instead of "Osc2 sync", but you can also try to be consistent with how Behringer labels it. Some more items. All combo boxes ought to be labeled. Your filter combo isn't labeled, instead it says "2-Pole Filter" and I presume "4-Pole Filter" or whatnot. Instead, you might label it "Type" and the values would be "2-Pole" and "4-Pole" etc. I can also help stretch at least the second envelope display to the end of the screen (just add it to the category using addLast). |
|
BTW, is the OSC1 chooser actually a boolean option (on/off?) and should that be a checkbox? Why is there no equivalent OSC2 chooser? There's one called OSC2 State... |
A valid point! All additional tabs are auto-generated via a data structure (table) that I copied from the specification in the manual. It's of course not optimal, but there are just so many options, so it's a lot of work to cover all. On the other hand, I think that if the user wants to edit them, this is much better than the menu diving on the synth. My hope is to cover the other tabs little by little, once I start messing more with the synth. |
|
I agree that if OSC2 state is off/half/full, then even though OSC1 state is off/full, it oughta be a chooser to be consistent. |
Could you provide a screenshot or two so we know what we're looking like? |
|
@eclab Any thoughts on next steps? My hope is that we can get this PR merged, and I'll start focusing on parsing the sysex dump :) |
|
It is unfortunate that the other tabs don't have a look consistent with Edisyn:, or even particularly lined up; and they also are very sparse. As a result you have a zillion tabs, which the user can't easily traverse because he has to scroll through the tabs to find the one he wants. I would suggest converting many of those tabs to categories and grouping them together under the same tab. I know you want to do the layout in an automated fashion, but as a result it looks ... automated ... rather than constructed by hand for the user. |
That's a good idea. I'm sure I can work something out that groups together meaningfully, and hopefully also avoids the sparse tabs. |
|
I might be able to come up with something so that the selectors (choosers) are left of the dials. It seems that there is (almost) always space enough. |
|
Hi grav, I'm releasing a new version of Edisyn. It doesn't have to have the UB-Xa editor in it (yet), we can release another with the UB-Xa when it's ready. But it'd be good to hear what the status of the editor is. How ready for prime time is it? I think my biggest concern is that all the tabs are auto-generated and not super elegant; I understand the UB-Xa has a bunch of parameters though. But I'd like to work with you to get this ready to go. |
|
Hi again. So assuming the UB-Xa editor isn't quite ready for prime time yet, I'm going to go ahead and release the next version of Edisyn now. When the UB-Xa is finalized, we can put out another Edisyn version. |
Hey, sorry for the late reply. Yeah, let's wait with it. Honestly I'd love to have it at least be able to receive the current patch, before releasing it. |
There was a problem hiding this comment.
Pull request overview
This pull request adds a new synthesizer editor for the Behringer UB-Xa, implementing MIDI NRPN-based communication to control synth parameters. The editor provides a comprehensive interface with multiple tabs covering keyboard-accessible parameters and additional advanced settings.
Changes:
- Adds complete Behringer UB-Xa editor implementation with NRPN support for parameter transmission/reception
- Implements partial SysEx parsing functionality (work in progress as noted in PR description)
- Registers the new synth in the Edisyn configuration and documentation
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| edisyn/synth/behringerubxa/BehringerUBXa.java | Main editor implementation with GUI construction, NRPN handling, and partial SysEx parsing |
| edisyn/synth/behringerubxa/ParameterList.java | Comprehensive parameter definitions including dials, checkboxes, and selectors with NRPN mappings |
| edisyn/synth/behringerubxa/BehringerUBXaRec.java | SysEx message recognition for identifying UB-Xa messages |
| edisyn/synth/synths.txt | Registration of new synth in Edisyn's synth list |
| README.md | Documentation update adding UB-Xa to supported synths |
| .gitignore | Adds "out" directory to ignore build artifacts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (remaining <= 0) break; | ||
| int chunkLen = Math.min(7, remaining); |
There was a problem hiding this comment.
In the unpack7b method at line 806, chunkLen is calculated as Math.min(7, remaining), but if remaining is negative (which the check at line 805 tries to prevent), this could lead to unexpected behavior. However, the break at line 805 should prevent this. Consider making the logic clearer by checking remaining < chunkLen instead of remaining <= 0 to be more explicit about the requirement.
| if (remaining <= 0) break; | |
| int chunkLen = Math.min(7, remaining); | |
| int chunkLen = 7; | |
| // Require a full 7-byte payload following the header | |
| if (remaining < chunkLen) { | |
| break; | |
| } |
|
|
||
| @Override | ||
| public void updateNumberAndBank(Patch patch){ | ||
| int i = 42; |
There was a problem hiding this comment.
The method updateNumberAndBank has a dummy implementation with an unused local variable int i = 42;. This appears to be placeholder code that should either be properly implemented or have a TODO comment explaining why it's not implemented yet.
| int i = 42; | |
| // TODO: Implement patch number and bank update for Behringer UB-Xa if/when | |
| // the patch format and location handling are defined. For now, this method | |
| // is intentionally a no-op because the patch data does not encode location. |
| // if (packetNum == 0) { | ||
|
|
||
|
|
||
| byte[] encodedData = Arrays.copyOfRange(data, offset, offset + packageLength); | ||
| verify7bitizedData(encodedData); | ||
| byte[] decodedData = unpack7b(encodedData); | ||
| // TODO - how to interpret the decoded data | ||
|
|
||
| // } | ||
| // patchDump.add(Arrays.copyOfRange(data, offset, packageLength + offset)); |
There was a problem hiding this comment.
The commented-out code block (lines 843-851) should be removed if it's no longer needed, or uncommented if it's still required. Keeping large blocks of commented code reduces code readability and maintainability. If this code might be needed in the future, consider documenting why it's commented out or tracking it in version control history instead.
| // if (packetNum == 0) { | |
| byte[] encodedData = Arrays.copyOfRange(data, offset, offset + packageLength); | |
| verify7bitizedData(encodedData); | |
| byte[] decodedData = unpack7b(encodedData); | |
| // TODO - how to interpret the decoded data | |
| // } | |
| // patchDump.add(Arrays.copyOfRange(data, offset, packageLength + offset)); | |
| byte[] encodedData = Arrays.copyOfRange(data, offset, offset + packageLength); | |
| verify7bitizedData(encodedData); | |
| byte[] decodedData = unpack7b(encodedData); | |
| // TODO - how to interpret the decoded data |
| if(lastDump != null){ | ||
| for (int i = 0; i < lastDump.length; i++) { | ||
| if(data[i] != lastDump[i]) { | ||
| System.out.println("diff " + i+": "+lastDump[i] + " != " + data[i]); | ||
| } | ||
| } | ||
| } | ||
| this.lastDump = data; |
There was a problem hiding this comment.
The variable lastDump is used to compare with current dump data for debugging, but it's never initialized to anything other than null. On the first parse, the comparison loop at lines 771-775 will be skipped (which is fine), but this variable appears to serve only debugging purposes. Consider removing it or clearly documenting its purpose if it's intended for future use.
| if(lastDump != null){ | |
| for (int i = 0; i < lastDump.length; i++) { | |
| if(data[i] != lastDump[i]) { | |
| System.out.println("diff " + i+": "+lastDump[i] + " != " + data[i]); | |
| } | |
| } | |
| } | |
| this.lastDump = data; |
| // FIXME | ||
| int val = data[pos];// + data[pos+1] << 2; | ||
| int i1 = data[pos] + (data[pos + 1] << 1); |
There was a problem hiding this comment.
The comment "FIXME" at line 782 indicates unfinished code. The calculation appears incomplete - it only uses data[pos] and the subsequent calculation on line 784 is not used. Since the PR description mentions SysEx support is incomplete, this is expected, but the FIXME should ideally be expanded with more context about what needs to be fixed.
| // FIXME | |
| int val = data[pos];// + data[pos+1] << 2; | |
| int i1 = data[pos] + (data[pos + 1] << 1); | |
| // FIXME: Implement reverse mapping from SysExPosToKeyAndSize into model parameters | |
| // when full bidirectional SysEx support is required. For now, parsePatch() uses | |
| // KeyToSysExPos above to populate the model, so this loop is intentionally a stub. | |
| // int val = data[pos];// + (data[pos+1] << 2); | |
| // int i1 = data[pos] + (data[pos + 1] << 1); |
|
|
||
| private String lastDialEmitKey = null; | ||
| private int lastDialEmitIdx = -1; | ||
|
|
There was a problem hiding this comment.
The private field lastDialEmitKey and lastDialEmitIdx are used for caching, but the variable lastDialReceiveIdx is also used for similar caching on the receive side. However, there's no lastDialReceiveKey to match. This asymmetry suggests the caching strategy might be incomplete. Consider either adding lastDialReceiveKey for consistency or documenting why it's not needed on the receive side.
| // Receive-side caching: parallel to lastDialEmitKey/lastDialEmitIdx | |
| private String lastDialReceiveKey = null; |
| for(int i=0; i<KeyToSysExPos.length; i+=2){ | ||
| String key = (String)KeyToSysExPos[i]; | ||
| int[] pos = (int[])KeyToSysExPos[i+1]; | ||
| int val = parsePosData(data[pos[0]], pos.length>1?data[pos[1]]:-1, pos.length>2?data[pos[2]]:-1, pos.length>3?data[pos[3]]:-1); | ||
| for (int po : pos) { | ||
| System.out.println(po + "=" + data[po]); | ||
| } | ||
| System.out.println(key+": "+val); | ||
| getModel().set(key, val); | ||
|
|
There was a problem hiding this comment.
In the parsePatch method, when iterating through KeyToSysExPos, the code assumes data[pos[0]], data[pos[1]], etc. exist without bounds checking. If the data array is shorter than expected (due to incomplete or corrupted SysEx data), this will throw an ArrayIndexOutOfBoundsException. Consider adding bounds checking before accessing array elements to provide better error handling and debugging information.
| int chunkLen = Math.min(7, remaining); | ||
| for (int i = 0; i < chunkLen; i++) { | ||
| int msb = (header >> (6 - i)) & 0x01; | ||
| int payload = data[offset + 1 + i] & 0x7F; // ensure 7-bit payload |
There was a problem hiding this comment.
This array access might be out of bounds, as the index might be equal to the array length + 6.
| return byteArray; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
This method overrides Synth.requestCurrentDump; it is advisable to add an Override annotation.
| @Override |
| labels = splitAtCapitalLetter(lbl, 10); | ||
| } | ||
|
|
||
| LabelledDial comp = new LabelledDial(labels[0], this, key, Style.COLOR_A(), minVal, maxVal, sub) { |
There was a problem hiding this comment.
This method overrides LabelledDial.isSymmetric; it is advisable to add an Override annotation.
| LabelledDial comp = new LabelledDial(labels[0], this, key, Style.COLOR_A(), minVal, maxVal, sub) { | |
| LabelledDial comp = new LabelledDial(labels[0], this, key, Style.COLOR_A(), minVal, maxVal, sub) { | |
| @Override |
|
Last we talked you thought that the UB-Xa editor wasn't ready for prime time yet. And I had mentioned that it deviated quite a lot from Edisyn's style (no Categories, very sparse tabs, choosers under dials rather than to the left, etc.). I haven't checked on your recent changes. How should we be treating the recent changes? Do you want me to examine them at this point? Should we be doing a pull [not necessarily adding it to build into Edisyn], or keep it as is for the moment? |
Hi @eclab, I forgot this PR existed and notified you on every change 😅 Currently I'm having some fun with reverse-engineering the file dump from the Synth - I've managed to decode most of the 3 and 4 byte parameters. Wrt layout: I'm quite happy with the first tab. It currently looks like this, based on your guidelines The other tabs I haven't done much about, and don't really have that as a priority from my side. I'd much rather get patch dumps to work. I think the editor already would bring much value to UB-Xa owners as it is, and I cannot promise any timelines for improving the other tabs. To me, it would make sense to release as-is, maybe hiding the "messy" tabs per default with an option to show them? And then the next iteration would include a fully function "Request current tab". WDYT? |
|
Some suggestions on that first tab.
- Surely UB-Xa patches have patch numbers. I'd add that in the white category area.
- UB-Xa patches don't have names? Perhaps not? The Matrix series didn't; but this is Behringer thing, not an exact remake of the OB-Xa...
- In your oscillators choosers, the names are things like "Osc 1 VCO LFO 0 degrees". But that's obvious given the name of the chooser: I'd change it to just "0 degrees". And just "Pulse" and just "Off", and just "Half" rather than "Osc 2 Half" etc. This is much more readable and will give you some more space.
- The envelope displays are really tall, but I don't see much way around it given all those checkboxes and no way to break them up. I guess you could do the trick you did in Modulation, but it'd save like one line at most.
- The tab names are huge. You should strive to shorten them to squeeze them onto one window. Like "Envelopes" -> "Env" or "Oscillators" -> "Osc" or "Amplifier" -> "VCA". If you put all the subcategories of the same type on one tab, you could save some text space too; it's all a bin-packing game. Like putting Envelopes, LFO, and ModMatrix together and calling it "Modulation". Speaking of which, you have your Loudness and Filter Envelopes in "Main", but you have a separate "Envelopes" tab. You couldn't swap some things so the Envelope stuff is all together?
Otherwise looking pretty good!
Sean
… On Feb 1, 2026, at 9:38 AM, Mikkel Gravgaard ***@***.***> wrote:
grav
left a comment
(eclab/edisyn#81)
Last we talked you thought that the UB-Xa editor wasn't ready for prime time yet. And I had mentioned that it deviated quite a lot from Edisyn's style (no Categories, very sparse tabs, choosers under dials rather than to the left, etc.). I haven't checked on your recent changes. How should we be treating the recent changes? Do you want me to examine them at this point? Should we be doing a pull [not necessarily adding it to build into Edisyn], or keep it as is for the moment?
Hi @eclab, I forgot this PR existed and notified you on every change 😅
Currently I'm having some fun with reverse-engineering the file dump from the Synth - I've managed to decode most of the 3 and 4 byte parameters.
Wrt layout: I'm quite happy with the first tab. It currently looks like this, based on your guidelines
Screenshot.2026-02-01.at.15.33.23.png (view on web)
The other tabs I haven't done much about, and don't really have that as a priority from my side. I'd much rather get patch dumps to work.
I think the editor already would bring much value to UB-Xa owners as it is, and I cannot promise any timelines for improving the other tabs.
To me, it would make sense to release as-is, maybe hiding the "messy" tabs per default with an option to show them? And then the next iteration would include a fully function "Request current tab".
WDYT?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|













First iteration of the Behringer UB-Xa(d) editor.
The editor consists of multiple tabs, with the first containing the parameters accessible on the keyboard version, and the rest of the tabs containing all additional parameters.
Screenshot
Details
The editor is using MIDI NRPNs for transmitting and receiving datga from the UB-Xa.
All parameters in the manual (starting on page 24) are supported.
To send values to the UB-Xa, the synth's default settings must be changed:
To receive values from the synth, two changes are required to the settings:
I've been mainly looking at the DSI Prophet '08 / Tetra / Mopho editor for inspiration, as it is also using NRPN.
Status of SysEx support
The editor is able to request patches via SysEx from the UB-Xa but still cannot interpret them.
I've been able to decode the SysEx data as specified in the refernce guide, but there's something I'm not fully understanding about the layout. I'll see if Behringer can help me, or if I can get further on reverse-engineering the data.