On Thu, Nov 15, 2012 at 11:38 PM, Markus Grabner <[email protected]> wrote: > Am Donnerstag, 15. November 2012, 08:16:25 schrieb Stefan Hajnoczi: >> On Thu, Nov 15, 2012 at 1:11 AM, Markus Grabner <[email protected]> > wrote: >> > On Wednesday 14 November 2012 08:45:24 Stefan Hajnoczi wrote: >> >> The sysfs attributes involve MIDI commands and some state. If we >> >> leave this to userspace then the kernel driver can focus on PCM and >> >> MIDI I/O. The code will become much smaller and simpler because we >> >> can drop all the peeking into MIDI buffers and the housekeeping that >> >> goes along with that. >> >> >> >> Letting userspace handle MIDI means that line6linux development >> >> becomes accessible to a wider group of developers - people not >> >> comfortable with C or driver hacking. It encourages people to explore >> >> their devices and contribute code. >> > >> > I agree with all of that. Let me point to a document which I started to >> > write a year ago when we first briefly discussed this: >> > >> > https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qt >> > based/doc/design.tex >> > >> > The sysfs attributes are ugly, but very convenient for development and >> > testing. I don't want to remove them without having a similarly convenient >> > replacement, that's why I started to write some code according to the >> > ideas >> > discussed in the abovementioned document. Feel free to comment on this! >> > >> > One particular question is how to best control the driver's behaviour >> > (e.g., the midi masks) from user mode. I can imagine at least two >> > methods: *) using ioctl() calls >> > *) defining a virtual MIDI device for communication with the driver >> > >> > Are there more options, and what would be considered best practise in this >> > case? >> >> Either ioctl() or sysfs is okay. ioctl() is very easy for C/C++ >> applications to use if they already have a file descriptor. For >> human-readable data sysfs is nicer since it can be accessed easily >> from a shell session. The challenge with sysfs is locating the device >> directory, but this could be done given the ALSA device numbers. > As far as I remember, it was Linus himself who recommended the special files > in the Line6 driver to be writable only by root :-), so this would be too > restrictive for most use cases. If it is accepted practise to define ioctl() > calls for this purpose, this is probably the best solution once we have user- > space tools to access Line6 devices. This would replace a command like > > echo 4 > /sys/bus/usb/drivers/line6usb/7-2:1.0/midi_mask_transmit > > by something like > > /usr/bin/line6 -d PODxtPro set midi_mask_transmit 4
I'm not sure what the best practice is, I guess we'll see when submitting the patches to move the driver out of staging. The only problem I see with sysfs is that it's not clear to me how to control file ownership. Typically we need the 'audio' group to own the attrs so regular users can read/write them. sysfs supports chmod(2)/chown(2) but I don't know whether udev(7) rules can apply ownership on sysfs attributes. >> The design document you posted looks sane and well thought out. I >> think the main "meat" of the library will be the data tables for each >> supported device. Ideally the library code would not hard-code names >> (e.g. amp1_gain, wah_position) but instead use self-describing data >> tables. The advantage here is that developers can easily add new >> tweak parameters by modifying the data tables. The only piece of >> software that needs to understand the parameter names is the GUI, and >> even there we could try to use a data-driven approach. >> >> In other words, the library doesn't need enums for all possible >> parameters. Instead you could say line6_set_param(dev, >> "master_volume", 0x40), which looks up "master_volume" in the device's >> data table. That produces a MIDI message (PC, CC, or SysEx). > I actually prefer symbols over strings for this purpose for several reasons: > *) a typo in a string doesn't get noticed at compile time > *) symbols can be used in switch/case statements > *) comparing an integer variable with a symbolic constant gives cleaner code > than a string comparison (using the "==" operator instead of calling "strcmp") > *) an integer is lightweight compared to a string, both in terms of memory > consumption and performance Since you have code working today let's go with the enum approach. > > The CSV table approach discussed in Section 4.4 of the design document comes > close to the self-describing data tables you mention above: all relevant > relations (e.g., the mapping between USB product ids and device names, or > between device parameters and corresponding MIDI byte codes) are defined and > edited in CSV files, which are automatically converted to corresponding C++ > code (enums, tables, maps, etc.) during the build process. The application can > then use these symbols as desired, and consistently adding a new parameter to > all relevant enum lists, tables, maps, etc., is as easy as adding a single row > to a CSV table. The created files should contain a prominent notice that they > were created automatically, with a reference to the input file (this is not > yet implemented). > > The perl script that does all this magic (parse_data.pl) is actually the most > complex piece of software in the user space code repository at this time :-) Cool :). I should try adding POD HD300 support. >> When the device sends MIDI messages to the host in response to the wah >> pedal, for example, the library uses the data table to translate from >> the MIDI message to event_param_changed(dev, "wah_position", 0x7f). > That's basically how it is (partially) implemented, except that a symbolic > constant is used instead of a string, and that the parameter id and the value > are collected in an "Action" class. This is yet missing from the design > document, the basic idea is to have a (small) set of action types (such as > "set a continuous parameter value", "set a boolean parameter value", etc.). > These are device-independent (though certainly not all devices support all > parameters), and it's the library's responsibility to translate between these > and the corrsponding device-dependent MIDI byte codes when transmitting or > receiving data. > >> The design document doesn't cover whether to make the library stateful >> or not. A stateless library simply transmits and receives events on >> behalf of the application. A stateful library keeps a "current value" >> stored for every parameter in the data table so that the application >> can retrieve it quickly. My feeling is that a stateless library is a >> good thing but there is a risk that applications will duplicate code, >> so we'd have to watch out if we find ourselves reimplementing the same >> state code in multiple applications. > As far as I know it is not possible to query a single parameter value from the > device (e.g., the current volume), and even if it were, such a query would > require a host-device-host roundtrip and suffer from a (small) delay. On the > other hand, whenever changing to a different tone on a POD device, the entire > settings for the new tone are transmitted to the host (~140 bytes). I > therefore think that maintaining the current state is useful here (that's > actually how the driver currently responds to read requests on special files > representing device parameters, which would also become obsolete in the driver > when done by a user space library). In a Qt GUI the widgets already keep state - you can ask a slider for its current value. So the application doesn't need the library to duplicate that state. BTW I have begun removing the MIDI sysfs attributes from pod.c. I'll send patches next week. >> A small detail that came to mind: transmitting MIDI should be >> asynchronous just like receiving MIDI is. The reason for this is that >> sending large commands (e.g. restoring saved presets) should not block >> the GUI. > After the headaches we had with a "true" asynchronous approach in our first > attempt, I think it's less error-prone to use the select() mechanism provided > by most UI libraries. In Qt, you can request notification when data are ready > for reading (or writing data has completed), by connecting signals from the Qt > device classes with corresponding slots in your application code. It is > probably sufficient to use non-blocking I/O and react on the completion > notification (e.g., by displaying a "process completed" dialog). If a progress > bar should be displayed, a large message should be split into smaller pieces > (e.g., one per preset) to allow for proper upgrade of the progress bar, but > the Qt main loop frequently get the opportunity to process user interface > events such that the GUI remains responsive. I'm not familiar with GTK, but I > think it's similar there. I think we're talking about the same thing here. I mean that the ALSA MIDI write call should be non-blocking so that we don't block the main loop. > >> Is there code to implement this design? > Yes, it's under > > https://line6linux.svn.sourceforge.net/svnroot/line6linux/apps/branches/qtbased > > The name "qtbased" is actually a misnomer, but the idea to clearly separate > the code related to a particular user interface library (Section 3.3.2 of the > design document) only gradually evolved while working on this branch, and I > simply didn't want to rename it again. > > Be aware that the code is very experimental, poorly documented, and quite a > mess. Nevertheless, the line6shell at least displays clear text descriptions > of events received from the device by making use of the auto-generated tables, > i.e., without any hard-coded parameter name in the code. Have fun exploring > the code :-) Thanks! Stefan ------------------------------------------------------------------------------ Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov _______________________________________________ Line6linux-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/line6linux-devel
