[sane-devel] RFC: proposal for an improved sanei_scsi library

[Henning Meier-Geinitz]

Hi,

On Sat, Dec 07, 2002 at 11:29:20PM +0100, abel deuring wrote:
> Some time ago, I made some remarks on this list about things I don't 
> like in the sanei_scsi interface: 
> http://www.mostang.com/pipermail/sane-devel/2002-January/001463.html
> Here is a proposal for a modified interface. The main points are:

Looks basically ok for me but I don't know much about SCSI :-)

I would just switch to the new API in SANE2 and use e.g.
sanei_scsi_open (not sanei2_). All backends must be rewritten anyway.
So you avoid confusion if it's necessary to add a sanei_scsi_open2 in
future.

> Instead, the sense buffer and the device status byte are passed to the 
> caller by sanei_scsi_req_wait / sanei_scsi_cmd.

What is the "device status byte"? Is this defined in the SCSI spec?
With the old interface, there was no way to get it?

> - sanei2_scsi_req_flush_all now only flushes the command for a single 
> device file. This is perhaps not very important, since the frontends I 
> know do not run more than one scan at a time, and it is not very likely 
> that we'll ever have a frontend that tries to control 10 scanners 
> simultaneously -- but a clean implementation would allow this ;) (And we 
> are not that far away from such an implementation.)

The SANE sttandard requires, that backends can open several devices
simultaneously. So I think it is important. And even if there is no
frontend yet that supports opening multiple devices it wouldn't be
that unusual to just keep the device selection window open to select
more scanners.

By the way: simultaneous scanning with some SCSI and some USB scanners
and multiple copies of XSane does work :-)

> /** Find SCSI devices.
>  *
>  * Find each SCSI device that matches the pattern specified by the
>  * arguments.  String arguments can be "omitted" by passing NULL,
>  * integer arguments can be "omitted" by passing -1.
>  *
>  *   Example: vendor="HP" model=NULL, type=NULL, bus=3, id=-1, lun=-1 would
>  *          attach all HP devices on SCSI bus 3.
>  *
>  * @param vendor
>  * @param model
>  * @param type
>  * @param bus
>  * @param channel
>  * @param id
>  * @param lun
>  * @param attach callback invoked once for each device
>  * @param dev real devicename (passed to attach callback)
>  *
>  */ 

While you are at it you could add descriptions for params vendor to
lun :-)

> /** Open a SCSI device
>  *
>  * Opens a SCSI device by its device filename and returns a file descriptor.
>  * The function tries to allocate a buffer of the size given by *buffersize.
>  * If sanei2_scsi_open returns successfully, *buffersize contains the
>  * available buffer size. This value may be both smaller or larger than the
>  * value requested by the backend; it can even be zero. The backend must not
>  * try to issue SCSI command with data blocks larger than given by the
>  * value of *buffersize. The backend must decide, if it got enough buffer
>  * memory to work.

What about timeout? Can it be changed by sanei_scsi, too? If yes, I
would add a sentence about this. Otherwise it could be just int
timeout instead of int * timeout.

>  * @param devicename name of the devicefile, e.g. "/dev/sg0"
>  * @param fd file descriptor
>  * @param is the timeout in seconds for SCSI commands

@param timeout is the timeout in seconds for SCSI commands

>  * @param sense_arg pointer to data for the sense handler

Which sense handler?

>  * @sa sanei2_scsi_open(), HAVE_SANEI_SCSI_OPEN_EXTENDED

@sa is "see also", you can remove this line in this case.

> /** Enqueue SCSI command
>  *
>  * One or more scsi commands can be enqueued by calling sanei2_scsi_req_enter().
>  *
>  * NOTE: Some systems may not support multiple outstanding commands.  On such
>  * systems, sanei2_scsi_req_enter() may block.  In other words, it is not proper
>  * to assume that enter() is a non-blocking routine.
>  *
>  * @param fd file descriptor
>  * @param cmd pointer to SCSI command
>  * @param cmd_size size of the command
>  * @param buffer pointer to the buffer with data to be sent to / received from
>           the scanner

   *        the scanner
   
>  * @param buffer_size size of the data buffer
>  * @param direction direction of the data transfer. Allowed values:
>  *	  - SANE_SCSI_DXFER_NONE  no data transfer
>  *        - SANE_SCSI_DXFER_TO_DEVICE data is sent to the device
>  *	  - SANE_SCSI__DXFER_FROM_DEVICE data is received from the device

Use SANEI_, as this is not a SANE API definition. The #defines for
these constants should be in the header, too. In this case, there
would be a nice cross-reference to their descriptions if you use the
doxygen HTML pages.

Minor formatting issue:
The formatting seems to be wrong here. Looks like the "-" don't work
in @param sections. When you explain the SANEI_SCSI_ stuff at the
point where they are #defined, you could just list them here without
the "-".

>  * @sa sanei2_scsi_req_enter()

Remove.

> /** Wait for SCSI command
>  *
>  * Wait for the completion of the SCSI command with id ID.  
>  *
>  * @param id id used in sanei2_scsi_req_enter()
>  * @param sb pointer to a SANE_Byte array, where the SCSI sense data may be
>  *           stored. The caller must allocate the necessary memory.
>  *	     The parameter may be NULL.

Why is SANE_Byte used here while all the other types are non-SANE
types? If you want to make sure that it's a byte (as in 8 bits)
buffer, maybe it's better to use u_int8_t or something like that.
SANE_Byte must be able to hold 8 bits, but it may be bigger than that.

>  * @param scsi_status the status returned by the device. 
>  *           xxx what about the situation, that the command has not been
>  *           issued, or other situations, where no status information
>  *           is available? Can this situation be detected for all supported
>  *           OSes? If that is possible, we should add another parameter which
>  *	     signals "SCSI status available"

Maybe use the return value to flag this. I.e. i/o error: scsi_status
has some meaning, invalid arg: something different went wrong.

> /** Send SCSI command
>  *
>  * This is a convenience function that is equivalent to a pair of
>  * sanei2_scsi_req_enter()/sanei2_scsi_req_wait() calls.

[...]

>  *
>  * @return
>  * - SANE_STATUS_GOOD - on success
>  * - SANE_STATUS_IO_ERROR - if an error was received from the SCSI driver
>  * - SANE_STATUS_NO_MEM - if malloc failed (not enough memory)
>  * - SANE_STATUS_INVAL - if a locking or an unknown error occured
>  * - SANE_STATUS_DEVICE_BUSY  - the SCSI command could not be issued; try
>  *                       again later
>  * @sa sanei2_scsi_cmd2(), sanei2_scsi_req_enter(), sanei2_scsi_req_wait()

Remove the first reference.

>  */
> extern SANE_Status sanei2_scsi_cmd2 (int fd,
> 				   const void * cmd, size_t cmd_size,
> 				   const void * buffer, size_t buffer_size,
> 				   int direction, SANE_Byte *sb, size_t *sblen,
> 				   SANE_Byte *scsi_status);


why scsi_cmd2?

Bye,
  Henning