On Fri, Jan 14, 2011 at 4:32 PM, Reinhold Kainhofer
reinhold at kainhofer.com wrote:
Am Freitag, 7. Januar 2011, um 02:34:20 schrieb m. allan noah:
I took a quick look at the code, and only see a few minor issues at
first glance.
1. MC_AutoDetectionTimeout is not configurable at runtime
2. the code only calls sanei_usb_init during sane_init. This prevents
long-running programs like button press daemons from finding
hotplugged scanners. This was discussed recently on the list.
3. Some of your Option Groups could get their names from saneopts.h
(SANE_NAME_GEOMETRY, TITLE, DESC, etc)
Other than those things, it looks fine to me.
Okay, here are updated patches (amended, so the whole backend is in one
patch):
http://www.fam.tuwien.ac.at/~reinhold/sane/magicolor_backend_patches/
-) Patches 0001 and 0002 are the USB patches you already reviewed (with
copyright notices added as you wanted), so they are ready to be applied.
-) Patch 0003 fixes a compiler warning that macros like htobe16 (byteorder.h)
etc. are already defined. In that case, don't overwrite the existing
definitions. (Chris Bagwell okay'ed it)
-) Patch 0004 is the actual backend
-) Patch 0005 is an alternative to running autoreconf (No manual changes done
by me!). This patch should probably not be applied to git master, but someone
(Chris Bagwell already volunteered) should run autoreconf with the recommended
auto(conf|make|...) versions for sane installed.
Changes to patch 0003 are in detail (compared to the last version you looked
at):
-) All three timeouts (SNMP detection, scan data read, other scanner
responses) are configurable in the magicolor.conf file
-) Cleaned up the options, reused pre-defined values as much as possible
-) Call sanei_usb_init also in get_devices
-) get_devices does not erase the list of scanners any more, but keeps still-
existing scanners untouched, just removes removed and adds new scanners.
Thank you very much for making those updates. I'll commit the code
this weekend, and take a swing at all the autofoo. I'll ask Chris for
help if required.
There is, however, one issue that needs to be discussed in principle:
How shall network auto-detection work in a sane way, once several backends
support auto-detecting network-connected scanners? AFAICS, all backends are
loaded sequentially, and the network auto-detection of each backend needs to
wait for a certain time before timing out (even if one response was received,
you have to wait the whole timeout, as there might be another device on the
network).
So, if 10 backends support auto-detection, and each uses a 2 second timeout,
sane would have a startup time of 20 seconds... Clearly, that's not
desirable.
On Windows, that's not an issue, as the user only installs drivers for the
scanners he really owns. However, sane provides all backends by default and
runs each backend's initialization (get_devices), so here it is a problem.
Yes- the SUSE way is to comment out backends in dll.conf.
Unfortunately, we dont have a generic way to do that.
You should register for an alioth account, and request to join the
project, and I will ad you.
allan
--
The truth is an offense, but not a sin
