edisyn icon indicating copy to clipboard operation
edisyn copied to clipboard

Introducing support for Novation A station

Open roomsg opened this issue 1 year ago • 5 comments

Introducing support for Novation A Station.

Reason: this synth is another example of a hard-to-program synth (read: a lot of menu diving, fiddling with buttons/switches that can have double use, thx to infamous "shift" button). I was simply not using it because of that. Edisyn however is now opening-it-up

Not saying it is fully complete, but it does support all basic patch loading/writing, supporting all parameters and handling real time changes bi-directionally. Definitely a much better alternative than configuration on the real HW.

So, wanted to give already a heads up, curious for your comments, let's discuss what needs to be added/fixed at least to make it a candidate for submission.

(and thx a lot for your changes in Midi.java - those are also incorporated here)

What I still gonna do for sure the coming days is include some more documentation and code cleanup.

Looking forward to your review/comments !

Guy.

roomsg avatar Oct 14 '24 19:10 roomsg

Lemme know when you think this is ready for prime time. I'll have to revise Midi.java to merge it and tweak some other stuff so it'd be best that it's really close. Including an About panel HTMl file...

eclab avatar Oct 21 '24 20:10 eclab

I'm close to finishing off coding/doc/cleanup. Will be doing some extensive testing next, I presume to wrap this up by the end of this weekend. Will keep you posted.

roomsg avatar Oct 22 '24 20:10 roomsg

Yup, this should be it: ready for review (..). did some extensive tests & also ran the "SanityCheck" on it. Open for comments OFC Do note I made sure it compiles with JDK11 (not sure what expectations are here), let me know if I need to go lower.

roomsg avatar Oct 26 '24 20:10 roomsg

sorry, added yet another commit. Also noticed the SanityCheck is sometimes failing, on a boolean value (checkbox). Not yet sure what's wrong there. Will have another look tomorrow

roomsg avatar Oct 26 '24 20:10 roomsg

Good to have the SanityCheck class around: revealed a copy-paste issue still. Fixed now, SanityCheck running stable. So, at last, this one is ready for review :)

roomsg avatar Oct 27 '24 10:10 roomsg

Okay, after checking it out, a few comments. First, could you please make all the labels titled case rather than lowercase? Second, you might add some category copy/paste between the oscillators, between the LFOs, and perhaps between the envelopes. I'd change "PW" to "Pulse Width". How does the A-station handle bank dumps? Let me know if you need help with librarian support.

eclab avatar Nov 02 '24 15:11 eclab

Another minor item. You have some choosers for non-categorical (metric)) data, such as Keysync Phase, Unison Voices, [Oscillator] Octave, and [Arp] Octaves. It's fine to keep them as choosers but you might set them as metric parameters so the mutation methods deal with them better. Similarly, I'm not sure if [Equalizer] Level should be a chooser or not, nor [chorus] Rate Sync or [Delay] Time Sync. I think [Arp] Pattern shoudl definitely be a chooser.

eclab avatar Nov 02 '24 16:11 eclab

I think aftrtch should be "Aftertouch" and modwheel should be "Mod Wheel", etc.. I know you're trying to keep the dials neat but the label legibility really matters in my experience. Also I'd to "LFO 1" rather than lfo1 etc. I would break Portamento, Pitch & AMP modulation into three separate categories. You can drop Moulation" from the title it's fine, so you'd have "Portamento", "Pitch" (or "Pitch Mod" or "Pitch Modulation") and "Amplifier". Last, if the category is called "External Audio", is there a need to include the term "external audio" in the checkboxes? Why not just say "Trigger" and "Direct to FX"?

eclab avatar Nov 02 '24 16:11 eclab

Thx for the review, I just uploaded a new version which should fix (almost all)

  • not yet fixed: Library support. See later..
  • The rest of the comments should be "handled", check commit msg for more detailed information

2 remarks:

  • the "category copy/paste". I tried in the past, but was surprised it just did not work on my machine; also not for other, existing synths. So, that was confusing Just now I found out it does not work on my linux machine which I am using for development. On my old mac (os 10.13) which I am using for doing music it does seem to work. Admittedly, never used that feature, now I will ;) (Yet, I'm still having problems to compile on linux and run on mac-JDK9, so I was not able to verify my own changes on mac - need to line up sdk/jre versions here, less obvious as I would have thought)
  • Do you consider library support as a must have or would it be acceptable for you I reserve this development for a subsequent PR ?

wrt A Station support for downloading patches/banks: from what I remember from the manual, it

  • provides the possibility to trigger (sysex) loading patches in batches of (only) 2. Not sure if that is a way forward considering there are 400 patches in total
  • provides the possibility to dump banks (yet only triggered from the hardware) Again, I certainly don't mind to have this sorted out/implemented, but it will for sure take me some time to get that working/fully tested. (I also need to get acquanted with the library, since have to admit I haven't used it yet).

Anyhow, in short, if acceptable for you, I would suggest to have this handled in a subsequent PR.

Let me know yr thoughts; looking forward to your view & comments ;)

roomsg avatar Nov 03 '24 16:11 roomsg

All right, time to pull out the linux box and see why category copy/paste aren't working. I couldn't think of any possible reason for this. Is it that the pop-up menu doesn't show? Or that you can select copy and paste from the pop up menu but nothing happens?

eclab avatar Nov 03 '24 16:11 eclab

Library support isn't a must. But it's easy to implement! You probably should check out how the D-Station editor supports it in its minimal fashion.

eclab avatar Nov 03 '24 16:11 eclab

Is it that the pop-up menu doesn't show?

indeed, no window is popping up upon right/shift clicking a category.

FYI - running Ubuntu 22.04.5 on a (very) old MacbookPro, for edisyn development purposes now using jdk 11.0.24 Let me know if you want me to write an issue for that.

roomsg avatar Nov 03 '24 17:11 roomsg

indeed, no window is popping up upon right/shift clicking a category.

Actually, it's not right- or shift-clicking. It's just plain old mouse clicking now. Documentation wrong?

The code in question starts on line 404 of edisyn/gui/Category.java

You might stick a print statement in there and see if it's responding to mouse clicks at all, or if pop.show() isn't working. Pretty weird.

eclab avatar Nov 07 '24 21:11 eclab

wrt A Station support for downloading patches/banks: from what I remember from the manual, it

* provides the possibility to trigger (sysex) loading patches in batches of (only) 2. Not sure if that is a way forward considering there are 400 patches in total

I checked out the manual, and sure enough, you can download single patches or pairs. What a crazy design decision. Anyway, if you can request to load one patch at a time (as in requesting a load into the editor), the Librarian can just do that no problem. I just won't be particularly fast.

* provides the possibility to dump banks (yet only triggered from the hardware)

I didn't see a special bank dump message. But you'll probably need to be able to recognize the pair message and parse something from it....

eclab avatar Nov 07 '24 21:11 eclab

Actually, it's not right- or shift-clicking. It's just plain old mouse clicking now. Documentation wrong?

ack, simple mouse click is good enough

The code in question starts on line 404 of edisyn/gui/Category.java You might stick a print statement in there and see if it's responding to mouse clicks at all, or if pop.show() isn't working.

Did some debugging & found the culprit: line427: "Category.this.remove(pop)" is causing the popup to not appear.

Testing it all with

  • commented: line 425: "Category.this.add(pop)" line 427: "Category.this.remove(pop)"
  • uncommented line 609: "Category.this.add(pop)"

and can confirm this is properly working on both my ubuntu 22.04.5 and mac 10.13.3

Let me know if you want me to include these changes in my PR. I'm sure you've put it there for a good reason..

roomsg avatar Nov 08 '24 13:11 roomsg

small update: just now, also tested the (category) behavior on windows (11), there the situation is even different

  • The official build: popup is showing up, but pasting values does not always seems to work: sometimes it does, sometimes it does nothing
  • with the fix described above: all working fine ;)

Wait, which fix?

roomsg avatar Nov 08 '24 14:11 roomsg

* ["build" changes](https://github.com/eclab/edisyn/pull/84/commits/47636cceb758aba8a3e86143cc40b86b656452fa): enforcing java9 (source/target) + open up debugport on run target (shoot if you disagree on something)

That might be a problem for the moment. Java's app builder > v8 does not build Mac apps properly. I'm working on it, but for the moment my only real option is to build <= v8.

eclab avatar Nov 08 '24 15:11 eclab

That might be a problem for the moment. Java's app builder > v8 does not build Mac apps properly. I'm working on it, but for the moment my only real option is to build <= v8.

Hmm, surprised by that; I also tried compiling for java8 but (again on linux) got some errors on the generic edisyn code, relying on java9: ao: https://docs.oracle.com/en/java/javase/17/docs/api/java.desktop/java/awt/desktop/AboutHandler.html ("since 9)

That's why I assumed you were at least using java9, hence I also used some (convenient) java9 apis in my code.

roomsg avatar Nov 08 '24 15:11 roomsg

could this work for you ?

roomsg avatar Nov 08 '24 15:11 roomsg

The issue is -- or was -- a combination of Oracle changing from javapackager to jpackage, and Apple changing from intel to M1 with fat binaries. I had an intel machine that could not generate M1 binaries because Oracle insisted in providing a thin java binary depending on your platform, and this broke everything with jpackage. But javapackager won't work with anything after 8 or 9. :-(

I now have an M1, so I can build with jpackage I think, but I still want to be able to build with Java 8 and jpackager in some situations. On my platform the decision of which Java version is used in building is determined not by the Makefile but by my command line environment. So that's why I'm hesitant to add in anything that locks the version in the Makefile -- I'd have to yank it out over and over. But maybe we could add it this verison...

eclab avatar Nov 08 '24 16:11 eclab

As to the AboutHandler, yes, that was a remnant of me trying to figure out some way to build with jpackage and Java >= 9, and I, um, committed it.

eclab avatar Nov 08 '24 16:11 eclab

As to the debug port: I'd prefer not to have that in the release. Of course you can stick that in the Makefile yourself when you need it.

eclab avatar Nov 08 '24 16:11 eclab

Wait, which fix?

I was referring to the three (un)commented lines in Category.java. I did noy include those yet in this PR, since based on comments it looks like that construction was introduced for a particular reason

roomsg avatar Nov 08 '24 23:11 roomsg

Okay, I've revised Category.java. I have changed from PopupMenu to JPopupMenu. PopupMenu has certain advantages (it looks normal on the Mac) but it might be causing some of the problems. Let's start with the JPopupMenu and see if it's working now in Windows. Then I will back off to PopupMenu with a slight revision and see if that works instead. If it does, we'll keep PopupMenu, if not, we'll keep JPopupMenu. So anyway let me know if it seems to be working now at least for step 1.

Also, I've updated Midi.java to the revised version of NRPN. Note I have NOT tested it yet; but at any rate your pull request branch is still conflicting because it has the old Midi.java, you'll need to tweak that.

eclab avatar Nov 09 '24 01:11 eclab

Have also updated to versions of Makefile (with jpackage rather than javapackager) and a new Mac.java, both of which require a Java version >= 11 I believe. I'm presently using 20.

eclab avatar Nov 09 '24 02:11 eclab

merged after some testing

  • Midi.java: seems to be working just fine ;)
  • category (copy/paste) on JPopupMenu: working fine on Mac, Linux & Windows 11,

roomsg avatar Nov 11 '24 13:11 roomsg

Curious about the race condition. I know why it'd happen but it ought not to....

eclab avatar Nov 16 '24 15:11 eclab

Curious about the race condition. I know why it'd happen but it ought not to....

check Private Message.

roomsg avatar Nov 24 '24 11:11 roomsg