From: Hiroo H. <hir...@co...> - 2004-08-01 19:34:37
|
Torsten and Rib came back. And Joachim joined us. During Torsten's and Rib's absent I've done many of issues on my TODO list. Shall we restart discussion of interface structure? On Mon, 03 May 2004 16:54:30 -0700 (PDT) Rib Rdb <ri...@ya...> wrote: Rib> Attached is a jar file containing possible interfaces for Driver, Rib> BankDriver and Patch. Also included is an abstract parent class of Rib> device containing methods that should be common to all devices. I Rib> used interfaces for drivers so that a synth can have a bank driver Rib> that is a subclass of any Driver (or IDriver for that matter). (For Rib> example, I wanted YamahaMotifBankDriver to extend Rib> YamahaMotifSingleDriver). I will also upload the jar to yahoo groups Rib> since i don't think message attachments are archived. Here is my idea. 1. interface IDriver (I don't have good name for this now.) This is an interface for Device.driverList. All of drivers (single driver, bank driver, converter) implement this. public String getPatchType(); public String getAuthors(); public boolean supportsPatch(StringBuffer patchString, Patch p); 2. class Converter implements IDriver abstract public IPatch[] extractPatch(IPatch p); I don't think we need to define interface for converter. 3. interface IPatchDriver extends IDriver This includes methods shared by ISingleDriver and IBankDriver which are used by LibraryFrame, etc. 4. interface ISingleDriver extends IPatchDriver This includes methods only for Single Driver. 5. interface IBankDriver extends IPatchDriver This includes methods only for Bank Driver. Note 1: This requires no change in synth drivers. Note 2: These interface includes only methods which core class uses. In other words they do not include methods which are used only by extending drivers. For example the followings are not included in ISingleDriver. public void send(byte[] sysex); public void send(int status, int d1, int d2); public void send(int status, int d1); Note 3: IBankDriver does not extend ISingleDriver. It is simply because a bank driver does not extend a single driver, but (sometimes) uses single driver methods. IBankDriver does not need all methods ISingleDriver. And a BankDriver uses same name methods with its Single Driver. It each method on BankDriver does not extend one of SingleDriver, but uses it. This is the reason I propose to pass a SingleDriver object to BankDriver constructor. Here is Rib's reply which I've not answered yet. On Tue, 04 May 2004 00:57:09 -0700 (PDT) Rib Rdb <ri...@ya...> wrote: Rib> --- Hiroo Hayashi <hir...@co...> wrote: Rib> > I used interfaces for drivers so that a synth can have a bank Rib> > driver Rib> > that is a subclass of any Driver (or IDriver for that matter). Rib> > Rib> > (For example, I wanted YamahaMotifBankDriver to extend Rib> > YamahaMotifSingleDriver). me> I think what you want to do can be done by passing the Single me> driver object to the constructor of the bank driver as I do in me> RolandTD6 driver. me> Here is the part of constructor RolandTD6Device. me> me> // add drivers me> TD6SingleDriver singleDriver = new TD6SingleDriver(); me> addDriver(singleDriver); me> addDriver(new TD6BankDriver(singleDriver)); Rib> Hmm. I suppose this would work, although it's not as elegant as Rib> simply being able to say "MyBankDriver extends MySingleDriver Rib> implements BankDriver". My proposal is just saying "MyBankDriver implements IBankDriver". First I tried to use 'super' in my TD6 BankDriver but it did not work well. It's not extending the Single Driver but only uses it. Having single driver object in the bank driver made the code straight forward and simple. I've been having a question. Who calls Driver.calculateCheckSum(IPatch), code in core or synth driver? It seems that now both call it, because it is not clear. If it is core's responsibility, each synth driver does not have to call it. If it is a synth-driver's responsibility, we can drop it from Interface. -- Hiroo Hayashi |
From: Rib R. <ri...@gm...> - 2004-08-13 07:30:45
|
On Mon, 03 May 2004 16:54:30 -0700 (PDT) Hiroo Hayashi <hiroo@co...> wrote: > Here is my idea. > > 1. interface IDriver (I don"t have good name for this now.) > > This is an interface for Device.driverList. All of drivers (single > driver, bank driver, converter) implement this. > > public String getPatchType(); > public String getAuthors(); > public boolean supportsPatch(StringBuffer patchString, Patch p); > > 2. class Converter implements IDriver > abstract public IPatch[] extractPatch(IPatch p); > > I don"t think we need to define interface for converter. > > 3. interface IPatchDriver extends IDriver > This includes methods shared by ISingleDriver and IBankDriver which are > used by LibraryFrame, etc. > > 4. interface ISingleDriver extends IPatchDriver > > This includes methods only for Single Driver. > > 5. interface IBankDriver extends IPatchDriver > > This includes methods only for Bank Driver. > > Note 1: This requires no change in synth drivers. > > Note 2: These interface includes only methods which core class uses. In > other words they do not include methods which are used only by extending > drivers. For example the followings are not included in ISingleDriver. > > public void send(byte[] sysex); > public void send(int status, int d1, int d2); > public void send(int status, int d1); I like this. > Note 3: IBankDriver does not extend ISingleDriver. > > It is simply because a bank driver does not extend a single driver, > but (sometimes) uses single driver methods. IBankDriver does not need > all methods ISingleDriver. And a BankDriver uses same name methods > with its Single Driver. It each method on BankDriver does not extend > one of SingleDriver, but uses it. This is the reason I propose to > pass a SingleDriver object to BankDriver constructor. > > Here is Rib"s reply which I"ve not answered yet. > > On Tue, 04 May 2004 00:57:09 -0700 (PDT) > Rib Rdb <ribrdb@ya...> wrote: > > Rib> --- Hiroo Hayashi <hiroo.hayashi@co...> wrote: > Rib> > I used interfaces for drivers so that a synth can have a bank > Rib> > driver > Rib> > that is a subclass of any Driver (or IDriver for that matter). > Rib> > > Rib> > (For example, I wanted YamahaMotifBankDriver to extend > Rib> > YamahaMotifSingleDriver). > me> I think what you want to do can be done by passing the Single > me> driver object to the constructor of the bank driver as I do in > me> RolandTD6 driver. > me> Here is the part of constructor RolandTD6Device. > me> > me> // add drivers > me> TD6SingleDriver singleDriver = new TD6SingleDriver(); > me> addDriver(singleDriver); > me> addDriver(new TD6BankDriver(singleDriver)); > > Rib> Hmm. I suppose this would work, although it"s not as elegant as > Rib> simply being able to say "MyBankDriver extends MySingleDriver > Rib> implements BankDriver". > > My proposal is just saying "MyBankDriver implements IBankDriver". > > First I tried to use "super" in my TD6 BankDriver but it did not work > well. It"s not extending the Single Driver but only uses it. Having > single driver object in the bank driver made the code straight forward > and simple. I think the difference here is that YamahaMotifSingleDriver isn't really a single driver. It was an abstract class with common initialization code and a few other functions overriding methods in driver. The BankDriver would use the NormalVoiceDriver or the BankVoiceDriver as you describe. Does anyone have any problems with these interface ideas? I'd like to commit this and the package reorganization before I work more on the XML drivers, and I'd like to try to get the XML drivers mostly working before school starts in about a month. |
From: Rib R. <ri...@gm...> - 2004-08-13 19:01:11
|
On Fri, 13 Aug 2004 07:56:17 -0500, Hiroo Hayashi <hir...@co...> wrote: > Rib> Does anyone have any problems with these interface ideas? I'd like to > Rib> commit this and the package reorganization before I work more on the > Rib> XML drivers, and I'd like to try to get the XML drivers mostly working > Rib> before school starts in about a month. > > Of course I don't see problem. > > Do you plan to do both implementing new interfaces and package > reorganization at the same time? I was thinking to do interface first. > We may make 0.20 before package reorganization because it affects on synth > drivers a lot. I was planning to do the interfaces first, then the package structure. I was thinking that it would just be reorganization, and that other than imports all that would be affected are things like the devicelist writer. If I'm wrong, maybe we can at least decide which package the XML driver files should go in, so that the cvs history can stay together for these new files. Then we can move the other stuff when we're ready. |
From: Hiroo H. <hir...@co...> - 2004-08-14 14:31:37
|
Rib, It seems that we are thinking same thing. Which package structure you need to decide now? I'll stop changing core files for a while not to create conflict with you. Are there anything I can help you for the interface change? Rib> > Do you plan to do both implementing new interfaces and package Rib> > reorganization at the same time? I was thinking to do interface first. Rib> > We may make 0.20 before package reorganization because it affects on synth Rib> > drivers a lot. Rib> Rib> I was planning to do the interfaces first, then the package structure. Rib> I was thinking that it would just be reorganization, and that other Rib> than imports all that would be affected are things like the devicelist Rib> writer. If I'm wrong, maybe we can at least decide which package the Rib> XML driver files should go in, so that the cvs history can stay Rib> together for these new files. Then we can move the other stuff when Rib> we're ready. -- Hiroo Hayashi |
From: Rib R. <ri...@gm...> - 2004-08-14 17:39:25
|
I guess we need to choose the packages where the support files for xml drivers, the drivers written in xml, and the ruby support classes should go. For easy reference, my original recomendation for these were, respectively, org.jsynthlib.jsynthlib.core.xml, org.jsynthlib.jsynthlib.xmldrivers, and org.jsynthlib.ruby. I believe that Hiroo suggested org.jsynthlib.jsynthlib.xml and org.jsynthlib.jsynthlib.drivers. I'd be fine with having xml under jsynthlib instead of core, but I wondered if it might be confused with the actual drivers written in xml. I recommended a separate directory for different types of drivers because the parser can't tell the difference between a java file and a messed up xml file. But I suppose I could easily solve this by requiring xml drivers to have a common extension (.xml, .device, or .driver come to mind as possibilities). On Sat, 14 Aug 2004 08:31:33 -0500, Hiroo Hayashi <hir...@co...> wrote: > Rib, > > It seems that we are thinking same thing. Which package structure you > need to decide now? > > I'll stop changing core files for a while not to create conflict with > you. Are there anything I can help you for the interface change? It'll probably be easier if I do it all myself. I'll try to do it today. |
From: Hiroo H. <hir...@co...> - 2004-08-14 19:36:10
|
Rib, Rib> I guess we need to choose the packages where the support files for xml Rib> drivers, the drivers written in xml, and the ruby support classes Rib> should go. Rib> Rib> For easy reference, my original recomendation for these were, Rib> respectively, org.jsynthlib.jsynthlib.core.xml, Rib> org.jsynthlib.jsynthlib.xmldrivers, and org.jsynthlib.ruby. Rib> Rib> I believe that Hiroo suggested org.jsynthlib.jsynthlib.xml and Rib> org.jsynthlib.jsynthlib.drivers. Rib> Rib> I'd be fine with having xml under jsynthlib instead of core, but I Rib> wondered if it might be confused with the actual drivers written in Rib> xml. I recommended a separate directory for different types of Rib> drivers because the parser can't tell the difference between a java Rib> file and a messed up xml file. But I suppose I could easily solve Rib> this by requiring xml drivers to have a common extension (.xml, Rib> .device, or .driver come to mind as possibilities). Yes. Actually DeviceListWriter finds device classes by using filename 'synthdrivers/*/*Device.class' now. (This is the only rule of class name for synth drivers.) We can do same thing for XML drivers. I think putting all synth driver in one place would have some more benefits. Rib> > I'll stop changing core files for a while not to create conflict with Rib> > you. Are there anything I can help you for the interface change? Rib> Rib> It'll probably be easier if I do it all myself. I'll try to do it today. OK. Happy hacking! -- Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-16 03:49:10
|
Hi, Rib> Ok. I commited the interfaces. Eclipse gave some kind of error when Rib> commiting, but it looks like it sent everything. Let me know if it Rib> did miss anything though. Your quick job always impresses me. I don't see any major problem. I've made the following change. 1. I updated the javadoc of new interfaces in core. 2. I replaced IPatchDriver.send(byte[]) with IPatchDriver.send(MidiMessage). 3. I reduced IPatchDriver reference in synthdrivers/*/*. I'm seeing the following code in Driver.java. protected void calculateChecksum(Patch patch, int start, int end, int ofs) { CalculateChecksum(patch, start, end, ofs); } public final static void CalculateChecksum(Patch p, int start, int end, int ofs) { ... // real code } My take is to have only one method; public static void calculateChecksum(Patch patch, int start, int end, int ofs) I understand you did not do this, because this broke synthdriver compatibility. But I think it is worth to be changed. Any comments? -- Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-16 13:18:14
|
Rib, My baseline is that calculateChecksum(Patch, int, int, int) should be static. My next question would be why Converter still extended the Driver:-) The checksumming is the only reason? Or did you see any other problems? On Sun, 15 Aug 2004 22:34:20 -0700 Rib Rdb <ri...@gm...> wrote: Rib> Actually I made the public static one because I had originally made Rib> Converter just implement IDriver instead of extending Driver, and it Rib> needed to do checksumming. But then I decided to leave Converter the Rib> way it was, so I don't think the CalculateChecksum method is needed. Rib> Rib> On Sun, 15 Aug 2004 21:49:15 -0500, Hiroo Hayashi Rib> <hir...@co...> wrote: Rib> > Hi, Rib> > Rib> > Rib> Ok. I commited the interfaces. Eclipse gave some kind of error when Rib> > Rib> commiting, but it looks like it sent everything. Let me know if it Rib> > Rib> did miss anything though. Rib> > Rib> > Your quick job always impresses me. Rib> > Rib> > I don't see any major problem. Rib> > Rib> > I've made the following change. Rib> > Rib> > 1. I updated the javadoc of new interfaces in core. Rib> > 2. I replaced IPatchDriver.send(byte[]) with Rib> > IPatchDriver.send(MidiMessage). Rib> > 3. I reduced IPatchDriver reference in synthdrivers/*/*. Rib> > Rib> > I'm seeing the following code in Driver.java. Rib> > Rib> > protected void calculateChecksum(Patch patch, int start, int end, int ofs) { Rib> > CalculateChecksum(patch, start, end, ofs); Rib> > } Rib> > Rib> > public final static void CalculateChecksum(Patch p, int start, int end, int ofs) { Rib> > ... // real code Rib> > } Rib> > Rib> > My take is to have only one method; Rib> > public static void calculateChecksum(Patch patch, int start, int end, Rib> > int ofs) Rib> > Rib> > I understand you did not do this, because this broke synthdriver Rib> > compatibility. But I think it is worth to be changed. Any comments? -- Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-21 14:50:11
|
I've made calculateChecksum(Patch, int, int, int) static. It's in head. Thanks. On Mon, 16 Aug 2004 07:18:20 -0500 Hiroo Hayashi <hir...@co...> wrote: Hiroo> Rib, Hiroo>=20 Hiroo> My baseline is that calculateChecksum(Patch, int, int, int) should b= e Hiroo> static. Hiroo>=20 Hiroo> My next question would be why Converter still extended the Driver:-) Hiroo> The checksumming is the only reason? Or did you see any other probl= ems? Hiroo>=20 Hiroo> On Sun, 15 Aug 2004 22:34:20 -0700 Hiroo> Rib Rdb <ri...@gm...> wrote: Hiroo>=20 Hiroo> Rib> Actually I made the public static one because I had originally = made Hiroo> Rib> Converter just implement IDriver instead of extending Driver, a= nd it Hiroo> Rib> needed to do checksumming. But then I decided to leave Convert= er the Hiroo> Rib> way it was, so I don't think the CalculateChecksum method is ne= eded. Hiroo> Rib>=20 Hiroo> Rib> On Sun, 15 Aug 2004 21:49:15 -0500, Hiroo Hayashi Hiroo> Rib> <hir...@co...> wrote: Hiroo> Rib> > Hi, Hiroo> Rib> >=20 Hiroo> Rib> > Rib> Ok. I commited the interfaces. Eclipse gave some kind of= error when Hiroo> Rib> > Rib> commiting, but it looks like it sent everything. Let me= know if it Hiroo> Rib> > Rib> did miss anything though. Hiroo> Rib> >=20 Hiroo> Rib> > Your quick job always impresses me. Hiroo> Rib> >=20 Hiroo> Rib> > I don't see any major problem. Hiroo> Rib> >=20 Hiroo> Rib> > I've made the following change. =2E.. Hiroo> Rib> > I'm seeing the following code in Driver.java. Hiroo> Rib> >=20 Hiroo> Rib> > protected void calculateChecksum(Patch patch, int start, = int end, int ofs) { Hiroo> Rib> > CalculateChecksum(patch, start, end, ofs); Hiroo> Rib> > } Hiroo> Rib> >=20 Hiroo> Rib> > public final static void CalculateChecksum(Patch p, int s= tart, int end, int ofs) { Hiroo> Rib> > ... // real code Hiroo> Rib> > } Hiroo> Rib> >=20 Hiroo> Rib> > My take is to have only one method; Hiroo> Rib> > public static void calculateChecksum(Patch patch, int sta= rt, int end, Hiroo> Rib> > int ofs) Hiroo> Rib> >=20 Hiroo> Rib> > I understand you did not do this, because this broke synthdri= ver Hiroo> Rib> > compatibility. But I think it is worth to be changed. Any c= omments? Hiroo>=20 Hiroo> --=20 Hiroo> Hiroo Hayashi --=20 Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-21 17:07:45
|
After taking look at the latest code, I've concluded that I was wrong. I wrote I won't need IConverter interface. But it's much better to have IConverter interface. It's in head. On Mon, 16 Aug 2004 07:18:20 -0500 Hiroo Hayashi <hir...@co...> wrote: Hiroo> My next question would be why Converter still extended the Driver:-) Hiroo> The checksumming is the only reason? Or did you see any other problems? Hiroo> Hiroo> On Sun, 15 Aug 2004 22:34:20 -0700 Hiroo> Rib Rdb <ri...@gm...> wrote: Hiroo> Hiroo> Rib> Actually I made the public static one because I had originally made Hiroo> Rib> Converter just implement IDriver instead of extending Driver, and it Hiroo> Rib> needed to do checksumming. But then I decided to leave Converter the Hiroo> Rib> way it was, so I don't think the CalculateChecksum method is needed. -- Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-21 21:07:18
|
Hi Rib, On Sat, 21 Aug 2004 11:55:26 -0700 Rib Rdb <ri...@gm...> wrote: Rib> > My next question would be why Converter still extended the Driver:-) Rib> > The checksumming is the only reason? Or did you see any other problems? Rib> Rib> I didn't see any problems at the time, but I just thought of a problem Rib> when I saw that you added dissect() to IPatch. If Converter is not an Rib> IPatchDriver, so there would be no way for a Patch's driver to be a Rib> converter. I think instead we need to make dissect(byte[]) and Rib> dissect(SysexMessage[]), or else make createPatch return IPatch[] and Rib> move createPatch from IPatchDriver to IDriver. What do you think? I was thinking about the problem, too Your mail made the problem clear. My take is almost the latter. Let me explain what I'm thinking now. IConverter is used by Patch.dissect() now. IPatch.dissect() is called by ImportAllDialog.doImport(), ImportMidiFile.doImport(), LibraryFrame.importPatch(), SceneFrame.importPatch(), and SysexGetDialog.pasterInfoSelectedFrame(). The input data for SysexGetDialog.pasterInfoSelectedFrame() is SysexMessage[] and driver is known. What we need is IPatch[] IPatchDriver.createPatch(SysexMessage[]) The most of code in SysexGetDialog.pasterInfoSelectedFrame() will be included in this method. The current pasterInfoSelectedFrame() tries other device/drivers when the received data is not expected one. I think we should simply cause error on this case. The input data for ImportAllDialog.doImport(), ImportMidiFile.doImport(), LibraryFrame.importPatch(), and SceneFrame.importPatch() is byte[] and driver is unknown for them. I think this is the job of patch factory you wrote. static IPatch[] Patch.createPatch(byte[]) This static method will be used for any IPatch object. Using Patch class (an implementation of IPatch) may not be proper. On the other hand I feel it might be overkill to create a new class (PatchFactory.java?) only for this, and I don't see a proper class which will include this... This method will use IDriver.supportsPatch(String, IPatch) (we may want to change the second argument to byte[]) and the selected driver will use IPatch[] IDriver.createPatch(byte[]) which you proposed. And if we have IDriver.supportsPatch(String, byte[]), a Patch's driver does not has to be a converter. IPatch IPatchDriver.createPatch(byte[]) will be replaced by IPatch[] IDriver.createPatch(byte[]). IPatch.dissect() is no longer an interface method. How do you think? If you agree with me and don't mind, I'll try to make this change during this weekend. PS. To make consistent with IPatch IPatchDriver.createPatch(byte[]), Patch IPatchDriver.createNewPatch() can be renamed to IPatch IPatchDriver.createPatch(). Of course Patch Driver.createNewPatch() will stay the same. -- Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-24 05:48:45
|
Hi, I've checked in the change for Patch factory. Here is the summary of the interface changes. Patch static IDriver chooseDriver(byte[]) is added. static IPatch[] valueOf(byte[]) is added. static String getPatchHeader(byte[]) is added. # It seems that valueOf() and getInstance are used for factory. I used # valueOf() for the factory method. IPatch IPatch[] dissect() is removed. void trimSysex() is removed. IDriver IPatch[] createPatch(byte[]) is added. boolean supportsPatch(String, IPatch) is removed. boolean supportsPatch(String, byte[]) is added. IPatchDriver IPatch[] createPatch(byte[]) is removed. IPatch[] createPatch(SysexMessage[]) is added. void trimSysex(IPatch) is removed. IConverter extractPatch() was removed. I tried not to change the program behavior, but I might make a mistake. Let me know if you see anything. Thanks. On Sat, 21 Aug 2004 15:07:13 -0500 Hiroo Hayashi <hir...@co...> wrote: Hiroo> Hi Rib, Hiroo> Hiroo> On Sat, 21 Aug 2004 11:55:26 -0700 Hiroo> Rib Rdb <ri...@gm...> wrote: Hiroo> Hiroo> Rib> > My next question would be why Converter still extended the Driver:-) Hiroo> Rib> > The checksumming is the only reason? Or did you see any other problems? Hiroo> Rib> Hiroo> Rib> I didn't see any problems at the time, but I just thought of a problem Hiroo> Rib> when I saw that you added dissect() to IPatch. If Converter is not an Hiroo> Rib> IPatchDriver, so there would be no way for a Patch's driver to be a Hiroo> Rib> converter. I think instead we need to make dissect(byte[]) and Hiroo> Rib> dissect(SysexMessage[]), or else make createPatch return IPatch[] and Hiroo> Rib> move createPatch from IPatchDriver to IDriver. What do you think? Hiroo> Hiroo> I was thinking about the problem, too Your mail made the problem clear. Hiroo> My take is almost the latter. Hiroo> Hiroo> Let me explain what I'm thinking now. Hiroo> Hiroo> IConverter is used by Patch.dissect() now. Hiroo> Hiroo> IPatch.dissect() is called by ImportAllDialog.doImport(), Hiroo> ImportMidiFile.doImport(), LibraryFrame.importPatch(), Hiroo> SceneFrame.importPatch(), and SysexGetDialog.pasterInfoSelectedFrame(). Hiroo> Hiroo> The input data for SysexGetDialog.pasterInfoSelectedFrame() is Hiroo> SysexMessage[] and driver is known. What we need is Hiroo> Hiroo> IPatch[] IPatchDriver.createPatch(SysexMessage[]) Hiroo> Hiroo> The most of code in SysexGetDialog.pasterInfoSelectedFrame() will be Hiroo> included in this method. Hiroo> The current pasterInfoSelectedFrame() tries other device/drivers when Hiroo> the received data is not expected one. I think we should simply cause Hiroo> error on this case. Hiroo> Hiroo> The input data for ImportAllDialog.doImport(), ImportMidiFile.doImport(), Hiroo> LibraryFrame.importPatch(), and SceneFrame.importPatch() is byte[] and Hiroo> driver is unknown for them. I think this is the job of patch factory Hiroo> you wrote. Hiroo> Hiroo> static IPatch[] Patch.createPatch(byte[]) Hiroo> Hiroo> This static method will be used for any IPatch object. Using Patch Hiroo> class (an implementation of IPatch) may not be proper. On the other Hiroo> hand I feel it might be overkill to create a new class Hiroo> (PatchFactory.java?) only for this, and I don't see a proper class which Hiroo> will include this... Hiroo> Hiroo> This method will use IDriver.supportsPatch(String, IPatch) (we may want Hiroo> to change the second argument to byte[]) and the selected driver will Hiroo> use IPatch[] IDriver.createPatch(byte[]) which you proposed. Hiroo> Hiroo> And if we have IDriver.supportsPatch(String, byte[]), a Patch's driver Hiroo> does not has to be a converter. Hiroo> Hiroo> IPatch IPatchDriver.createPatch(byte[]) will be replaced by IPatch[] Hiroo> IDriver.createPatch(byte[]). Hiroo> Hiroo> IPatch.dissect() is no longer an interface method. Hiroo> Hiroo> How do you think? If you agree with me and don't mind, I'll try to make this Hiroo> change during this weekend. Hiroo> Hiroo> PS. Hiroo> To make consistent with IPatch IPatchDriver.createPatch(byte[]), Hiroo> Patch IPatchDriver.createNewPatch() can be renamed to IPatch Hiroo> IPatchDriver.createPatch(). Of course Patch Driver.createNewPatch() Hiroo> will stay the same. Hiroo> -- Hiroo> Hiroo Hayashi Hiroo> Hiroo> Hiroo> Hiroo> ------------------------------------------------------- Hiroo> SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media Hiroo> 100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33 Hiroo> Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift. Hiroo> http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285 Hiroo> _______________________________________________ Hiroo> Jsynthlib-devel mailing list Hiroo> Jsy...@li... Hiroo> https://lists.sourceforge.net/lists/listinfo/jsynthlib-devel -- Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-24 13:46:12
|
Hi Rib, I have to go office soon. Quick answer. Rib> Hi Hiroo. I have a couple questions/comments about these changes. Rib> First, in Patch(byte[], int) it looks like it should be Rib> System.arraycopy(sysex, offset, this.sysex, 0, sysex.length - offset); Rib> instead of Rib> System.arraycopy(sysex, offset, sysex, 0, sysex.length - offset); You are correct. But now Patch(byte[], int) is not used by my last change. We can remove this constructor. Rib> Also, I was thinking it might be better to have Rib> IPatchDriver.supportsMessage so that each message can be checked Rib> instead of just whatever's at the beginning. Then it would probably Sorry, I don't understand the purpose of this method. Does this check if IPatch.getDriver() returned correct value? (I guess this is wrong.) Rib> be better to have something like Patch.chooseDriver(SysexMessage[]) Rib> and Patch.valueOf(SysexMessage[]). Should these be final? I did not defined them just because I did not need them to reimplement the current JSynthLib. If we need them, let's define. I've not sure this is the case, but I agree with you we should use more 'final' properly. It will help synth driver developers. Rib> I had one other question about Converter, but I figured it out. Rib> Should I go back include my changes to my Converter extend Object now? I'm not sure. If Converter extends Object (not Driver), don't you have to make change the subclasses of the Converter? I start thinking Driver, BankDriver, Convert, and Patch classes are for compatibility. I guess you may need AbstractConverter class. I also think we may need AbstractPatch class for common methods of Patch and XMLPatch class. Rib> I was working on the xml driver today. I'd guess it's about 40% done. Good to hear! -- Hiroo Hayashi |
From: Hiroo H. <hir...@co...> - 2004-08-25 04:13:27
|
Rib, I still don't understand. Sorry for my poor English. Rib> > Rib> Also, I was thinking it might be better to have Rib> > Rib> IPatchDriver.supportsMessage so that each message can be checked Rib> > Rib> instead of just whatever's at the beginning. Then it would probably Rib> > Rib> > Sorry, I don't understand the purpose of this method. Does this check Rib> > if IPatch.getDriver() returned correct value? (I guess this is wrong.) Rib> Rib> I was thinking of using this instead of supportsPatch(String, byte[]) Rib> in the patch factory method. If there are multiple messages it can Rib> check if there is a single driver that supports all the messages. If Rib> so, it will use that driver; otherwise it can send messages Rib> individually to drivers that support them. I understand you mean multiple patch (IPatch) by multiple message instead of multiple sysex message (SysexMessage). Am I correct? If I'm correct, this is the only job of Converter. Converter's createPatch() divides the input message into multiple patch and assigns a proper driver onto each patch by using IDriver.supportsPatch(). The process does not care if all of the patch are supported by a single driver or not. And now a converter can override createPatch() method directly instead of Converter.extractPatch(). Converter has more flexibility than before. Am I on the same page with you? -- Hiroo Hayashi |
From: <tt_...@gm...> - 2004-08-02 17:56:25
|
Hi Hiroo, Hiroo Hayashi wrote: > Torsten and Rib came back. And Joachim joined us. During Torsten's and > Rib's absent I've done many of issues on my TODO list. Shall we restart > discussion of interface structure? > > On Mon, 03 May 2004 16:54:30 -0700 (PDT) > Rib Rdb <ri...@ya...> wrote: > > Rib> Attached is a jar file containing possible interfaces for Driver, > Rib> BankDriver and Patch. Also included is an abstract parent class of > Rib> device containing methods that should be common to all devices. I > Rib> used interfaces for drivers so that a synth can have a bank driver > Rib> that is a subclass of any Driver (or IDriver for that matter). (For > Rib> example, I wanted YamahaMotifBankDriver to extend > Rib> YamahaMotifSingleDriver). I will also upload the jar to yahoo groups > Rib> since i don't think message attachments are archived. > > > Here is my idea. > > 1. interface IDriver (I don't have good name for this now.) > > This is an interface for Device.driverList. All of drivers (single > driver, bank driver, converter) implement this. > > public String getPatchType(); > public String getAuthors(); > public boolean supportsPatch(StringBuffer patchString, Patch p); > > 2. class Converter implements IDriver > abstract public IPatch[] extractPatch(IPatch p); > > I don't think we need to define interface for converter. > > 3. interface IPatchDriver extends IDriver > This includes methods shared by ISingleDriver and IBankDriver which are > used by LibraryFrame, etc. > > 4. interface ISingleDriver extends IPatchDriver > > This includes methods only for Single Driver. > > 5. interface IBankDriver extends IPatchDriver > > This includes methods only for Bank Driver. > > Note 1: This requires no change in synth drivers. > > Note 2: These interface includes only methods which core class uses. In > other words they do not include methods which are used only by extending > drivers. For example the followings are not included in ISingleDriver. > > public void send(byte[] sysex); > public void send(int status, int d1, int d2); > public void send(int status, int d1); > > Note 3: IBankDriver does not extend ISingleDriver. > > It is simply because a bank driver does not extend a single driver, > but (sometimes) uses single driver methods. IBankDriver does not need > all methods ISingleDriver. And a BankDriver uses same name methods > with its Single Driver. It each method on BankDriver does not extend > one of SingleDriver, but uses it. This is the reason I propose to > pass a SingleDriver object to BankDriver constructor. > > Here is Rib's reply which I've not answered yet. > > On Tue, 04 May 2004 00:57:09 -0700 (PDT) > Rib Rdb <ri...@ya...> wrote: > > Rib> --- Hiroo Hayashi <hir...@co...> wrote: > Rib> > I used interfaces for drivers so that a synth can have a bank > Rib> > driver > Rib> > that is a subclass of any Driver (or IDriver for that matter). > Rib> > > Rib> > (For example, I wanted YamahaMotifBankDriver to extend > Rib> > YamahaMotifSingleDriver). > me> I think what you want to do can be done by passing the Single > me> driver object to the constructor of the bank driver as I do in > me> RolandTD6 driver. > me> Here is the part of constructor RolandTD6Device. > me> > me> // add drivers > me> TD6SingleDriver singleDriver = new TD6SingleDriver(); > me> addDriver(singleDriver); > me> addDriver(new TD6BankDriver(singleDriver)); > > Rib> Hmm. I suppose this would work, although it's not as elegant as > Rib> simply being able to say "MyBankDriver extends MySingleDriver > Rib> implements BankDriver". > > My proposal is just saying "MyBankDriver implements IBankDriver". > > First I tried to use 'super' in my TD6 BankDriver but it did not work > well. It's not extending the Single Driver but only uses it. Having > single driver object in the bank driver made the code straight forward > and simple. Sorry, but I can't give you a skillfull answer to your questions. My experience with OOP is limited to this project. But I want give you my opinion to some points: The most important point from my point of view is the item 3 "IPatchDriver". I like the idea to put all information about a patch itself into an own structure very much. All parameters which are currently hold in the constructors of single resp. bank driver and methods like setPatchname/getPatchName, putPatch/getPatch should be collect in own classes, which can be loaded by each driver. So maybe we should create ISinglePatchDriver and IBankPatchDriver. BTW, I like to put some additional parameter to the bank patch information. I defined these parameters in my DX7 family drivers to use variables in the algorithms for some methods. This would give us at least 2 benefits: - Drivers doesn't need to know anything about patches. They have only to know how to request, send and store patches from/to a singular synth resp. how to call an editor. We could also implement the possibility to give the user the ability to choose a specific editor if several editors are available for one specific patch type (which was also discussed in the past). - The library itself could use the information of IPatchDriver to handle patches without loading synth drivers. Maybe we can implement a feature that the library itself is looking automatically for a needed IPatchDriver if a patchlib file (or sysex file) is loaded with patches which driver isn't already loaded. One additional remarks to the ISingleDriver and IBankDriver: If we put all information of a patch into an own structure, single and bank drivers uses the same methods like createNewPatch, editPatch, requestPatchDump, sendPatch and storePatch. If I don't miss something we don't need to distinct between single and bank drivers, but between single and bank patches. Am I missing something or am I misunderstanding your intention? But maybe my proposals are breaking the current device/driver structure, so we have to change all synthdrivers too, which would be a lot of work. > I've been having a question. Who calls Driver.calculateCheckSum(IPatch), > code in core or synth driver? It seems that now both call it, because > it is not clear. > If it is core's responsibility, each synth driver does not have to call > it. If it is a synth-driver's responsibility, we can drop it from > Interface. Since also the library operations (like creating and editing single resp. bank patches) need to calculate checksums I would say it's core's responsibility. But on the other side there are many kinds of checksum algorithms depending the singular patch types so maybe it's easier to put the responsibility to synth drivers. Generally I preferr to put the calculateCheckSum(IPatch) code into the core. Bye Torsten |
From: Hiroo H. <hir...@co...> - 2004-08-03 04:39:09
|
Torsten, Thank you for your reply. I'm trying to understand what you wrote, but I could not yet. I'm guessing you are thinking similar thing with me, and I could not express my intention well. Let me try again. First I'm not extending functionality at all. I'm just trying to define a minimum set of methods which a synthdriver have to provide (in other words set of methods which core uses). Now synth driver implements 3 classes Driver BankdDriver extends Driver (optional) Converter extends Driver (optional) Many methods in Driver class does not required for BankDriver and Converter. Extending Driver class is overkill for BankDriver and Converter. Even Driver class itself has many methods which does not required by core. For example Driver.calculateChecksum(IPatch, int, int, int) is a method only to implement Driver.calculateChecksum(IPatch). We don't have to implement Driver.calculateChecksum(IPatch, int, int, int). We need to make clear what a developer of synth driver has to implement. me> 1. interface IDriver (I don't have good name for this now.) me> 2. class Converter implements IDriver me> 3. interface IPatchDriver extends IDriver me> 4. interface ISingleDriver extends IPatchDriver me> 5. interface IBankDriver extends IPatchDriver ISingleDriver is the interface for the current Driver class. IBankDriver is the interface for the current BankDriver class. Converter class is a class which has only methods required for Converter. IDriver is a common set of methods for ISingleDriver, IBankDriver, and Converter. This is required for Device.driverList. IPatchDriver is a common set of methods for ISingleDriver and IBankDriver. This is required by classes which handle both Single Patch and Bank Patch, ex. LibraryFrame. (I use the name "IDriver" for this, but I could not find better name for the IDriver above.) -- Hiroo Hayashi |
From: <tt_...@gm...> - 2004-08-03 07:22:54
|
Hi Hiroo, Hiroo Hayashi wrote: > Torsten, > > Thank you for your reply. I'm trying to understand what you wrote, > but I could not yet. I'm guessing you are thinking similar thing with > me, and I could not express my intention well. Let me try again. Sorry for my bad english. > First I'm not extending functionality at all. I'm just trying to > define a minimum set of methods which a synthdriver have to provide > (in other words set of methods which core uses). OK, my proposals are definitly extensions. Maybe we come back to these in future. > Now synth driver implements 3 classes > Driver > BankdDriver extends Driver (optional) > Converter extends Driver (optional) > > Many methods in Driver class does not required for BankDriver and > Converter. Extending Driver class is overkill for BankDriver and > Converter. Even Driver class itself has many methods which does not > required by core. For example Driver.calculateChecksum(IPatch, int, > int, int) is a method only to implement > Driver.calculateChecksum(IPatch). We don't have to implement > Driver.calculateChecksum(IPatch, int, int, int). > > We need to make clear what a developer of synth driver has to > implement. I agree and start to understand your intention. You mean what a developer has to implement therewith JSynthLib.core is working properly. Which other methods the drivers need is the question of synthdrivers developer. Bye Torsten |