[sane-devel] Fwd: sanei_usb limitations

[Reinhold Kainhofer]

Am Mittwoch, 5. Januar 2011, um 15:25:59 schrieb m. allan noah:
> On Tue, Jan 4, 2011 at 8:55 PM, Reinhold Kainhofer
> 
>  wrote:
> > New patch is now up at:
> > http://codereview.appspot.com/2823041/
> 
> Looks good, except you forgot your copyright notice :)
[...]
> Add your copyright notice and I'll commit it.

Patch is attached (plus one patch that adds two sanity checks to 
sanei_usb_clear_halt, where a check for dn within the allowed range was 
missing).

Cheers,
Reinhold

-- 
--
Reinhold Kainhofer, reinhold at kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org
-- next part --
A non-text attachment was scrubbed...
Name: 0001-Add-function-to-set-USB-endpoints-to-use.patch
Type: text/x-patch
Size: 4155 bytes
Desc: not available
URL: 

-- next part --
A non-text attachment was scrubbed...
Name: 0002-Add-some-sanity-checks.patch
Type: text/x-patch
Size: 1783 bytes
Desc: not available
URL: 

[sane-devel] Fwd: sanei_usb limitations

[m. allan noah]

On Tue, Jan 4, 2011 at 8:55 PM, Reinhold Kainhofer
 wrote:
> New patch is now up at:
> http://codereview.appspot.com/2823041/

Looks good, except you forgot your copyright notice :)

> Actually, it is more convenient to change the default once during
> initialization and then don't have to care about passing the correct endpoint
> to each and every usb call.
>
> Anyway, since changing USB endpoints is rarely needed, I'm fine with adding
> just one function sanei_usb_set_endpoint, which sets the endpoint for a given
> type.
> If a backend needs to set a different endpoint for each call, then we might
> re-evaluate it. For now, every backend that needs to use two different
> endpoints of the same type needs to call sanei_usb_set_endpoint before each
> read/write call.

Sounds like the shortest solution.

>> > What do you have against bitmasks like (USB_DIR_IN |
>> > USB_ENDPOINT_TYPE_BULK)? Then we don't need any new constants...
>>
>> where are USB_DIR_IN and USB_ENDPOINT_TYPE_BULK defined? :)
>
> include/sane/sanei_usb.h, lines 163 and 116.

It is funny to OR zeros, but it works :)

Add your copyright notice and I'll commit it.

allan
-- 
"The truth is an offense, but not a sin"

[sane-devel] Fwd: sanei_usb limitations

[Reinhold Kainhofer]

New patch is now up at:
http://codereview.appspot.com/2823041/

Am Mittwoch, 5. Januar 2011, um 02:25:24 schrieb m. allan noah:
> On Tue, Jan 4, 2011 at 8:01 PM, Reinhold Kainhofer
>  wrote:
> > Am Dienstag, 4. Januar 2011, um 22:19:16 schrieb m. allan noah:
> >> I think your previous idea of adding a a new function to set the
> >> currently 'active' endpoint would be simpler.
> > 
> > Actually, that was never my idea for overriding the endpoints for a
> > single transactions, only for setting the global defaults. For the
> > defaults, it is definitely cleaner/easier to use one function to set the
> > default endpoints.
> 
> certainly easier than a callback, but really you don't need the
> defaults to be changed once you have the ability to override it on a
> per-call basis.

Actually, it is more convenient to change the default once during 
initialization and then don't have to care about passing the correct endpoint 
to each and every usb call.

Anyway, since changing USB endpoints is rarely needed, I'm fine with adding 
just one function sanei_usb_set_endpoint, which sets the endpoint for a given 
type. 
If a backend needs to set a different endpoint for each call, then we might 
re-evaluate it. For now, every backend that needs to use two different 
endpoints of the same type needs to call sanei_usb_set_endpoint before each 
read/write call.

> > What do you have against bitmasks like (USB_DIR_IN |
> > USB_ENDPOINT_TYPE_BULK)? Then we don't need any new constants...
> 
> where are USB_DIR_IN and USB_ENDPOINT_TYPE_BULK defined? :)

include/sane/sanei_usb.h, lines 163 and 116.

Cheers,
Reinhold

-- 
--
Reinhold Kainhofer, reinhold at kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[sane-devel] Fwd: sanei_usb limitations

[Reinhold Kainhofer]

Am Dienstag, 4. Januar 2011, um 22:19:16 schrieb m. allan noah:
> I think your previous idea of adding a a new function to set the
> currently 'active' endpoint would be simpler. 

Actually, that was never my idea for overriding the endpoints for a single 
transactions, only for setting the global defaults. For the defaults, it is 
definitely cleaner/easier to use one function to set the default endpoints.

For overriding the ep of a single transaction I don't like this approach too 
much. Having to call a setter function first to set a global variable, before 
you do the actual function call is highly thread-unsafe, and I would prefer to 
pass the ep right to the function itself.

> something like:
> 
> /*endpoint types*/
> #define SANEI_USB_BULK_OUT 0
> #define SANEI_USB_BULK_IN  1
> ...

What do you have against bitmasks like (USB_DIR_IN | USB_ENDPOINT_TYPE_BULK)? 
Then we don't need any new constants...

> sanei_usb_set_endpoint(SANE_Int dn, SANE_Int endpoint_type, SANE_Int
> endpoint_number){
>   switch/case setting devices[devcount].
> }
> 
> Then you've only got one new function, and no other code changes?


Cheers,
Reinhold
-- 
--
Reinhold Kainhofer, reinhold at kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[sane-devel] Fwd: sanei_usb limitations

[Reinhold Kainhofer]

Am Samstag, 27. November 2010, um 00:10:54 schrieb m. allan noah:
> Reinhold Kainhofer has recently worked up a patch with an alternative
> implementation of your idea- and there was some discussion about yet a
> third mechanism, which relies on a 'setting' function to be called
> prior to transferring the data, much as the existing timeout control
> code works. I think that would be the best choice, not sure if
> Reinhold has gotten a chance to work on it.

I have now updated my patch to also include a callback function to 
sanei_usb_open_extended, which is called before all available endpoints are 
listed. 
Patch is here:
http://codereview.appspot.com/2823041/

The callback function (if given) is called for each combination of 
USB_DIR_IN/USB_DIR_OUT and 
USB_ENDPOINT_TYPE_CONTROL/USB_ENDPOINT_TYPE_ISOCHRONOUS/USB_ENDPOINT_TYPE_BULK/USB_ENDPOINT_TYPE_INTERRUPT.
 
If a value >0 is returned, it is used as the default endpoint of that type 
(instead of the first encountered endpoint of that type).

The old functionality of an additional argument to each usb read/write 
function to explicitly override the endpoint for a single transaction is still 
there, so one can now override the default used for all transfers as well as 
the endpoint for a single transfer.

Any comments? Of course, I would love to get this into the 1.0.22 release, so 
other backends that need different endpoints for different communication types 
finally have this functionality available.

Cheers,
Reinhold
-- 
--
Reinhold Kainhofer, reinhold at kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[sane-devel] Fwd: sanei_usb limitations

[m. allan noah]

On Tue, Jan 4, 2011 at 8:01 PM, Reinhold Kainhofer
 wrote:
> Am Dienstag, 4. Januar 2011, um 22:19:16 schrieb m. allan noah:
>> I think your previous idea of adding a a new function to set the
>> currently 'active' endpoint would be simpler.
>
> Actually, that was never my idea for overriding the endpoints for a single
> transactions, only for setting the global defaults. For the defaults, it is
> definitely cleaner/easier to use one function to set the default endpoints.

certainly easier than a callback, but really you don't need the
defaults to be changed once you have the ability to override it on a
per-call basis.

> For overriding the ep of a single transaction I don't like this approach too
> much. Having to call a setter function first to set a global variable, before
> you do the actual function call is highly thread-unsafe, and I would prefer to
> pass the ep right to the function itself.

Yes- it does seem cleaner to pass the ep to the function, but I'm not
sure about your threading argument. You are pretty sure to break
something if you've got two threads talking to the scanner at the same
time.

>
>> something like:
>>
>> /*endpoint types*/
>> #define SANEI_USB_BULK_OUT 0
>> #define SANEI_USB_BULK_IN ?1
>> ...
>
> What do you have against bitmasks like (USB_DIR_IN | USB_ENDPOINT_TYPE_BULK)?
> Then we don't need any new constants...

where are USB_DIR_IN and USB_ENDPOINT_TYPE_BULK defined? :)

allan
-- 
"The truth is an offense, but not a sin"

[sane-devel] Fwd: sanei_usb limitations

[m. allan noah]

I think your previous idea of adding a a new function to set the
currently 'active' endpoint would be simpler. something like:

/*endpoint types*/
#define SANEI_USB_BULK_OUT 0
#define SANEI_USB_BULK_IN  1
...

sanei_usb_set_endpoint(SANE_Int dn, SANE_Int endpoint_type, SANE_Int
endpoint_number){
  switch/case setting devices[devcount].
}

Then you've only got one new function, and no other code changes?

allan

On Tue, Jan 4, 2011 at 4:01 PM, Reinhold Kainhofer
 wrote:
> Am Samstag, 27. November 2010, um 00:10:54 schrieb m. allan noah:
>> Reinhold Kainhofer has recently worked up a patch with an alternative
>> implementation of your idea- and there was some discussion about yet a
>> third mechanism, which relies on a 'setting' function to be called
>> prior to transferring the data, much as the existing timeout control
>> code works. I think that would be the best choice, not sure if
>> Reinhold has gotten a chance to work on it.
>
> I have now updated my patch to also include a callback function to
> sanei_usb_open_extended, which is called before all available endpoints are
> listed.
> Patch is here:
> http://codereview.appspot.com/2823041/
>
> The callback function (if given) is called for each combination of
> USB_DIR_IN/USB_DIR_OUT and
> USB_ENDPOINT_TYPE_CONTROL/USB_ENDPOINT_TYPE_ISOCHRONOUS/USB_ENDPOINT_TYPE_BULK/USB_ENDPOINT_TYPE_INTERRUPT.
> If a value >0 is returned, it is used as the default endpoint of that type
> (instead of the first encountered endpoint of that type).
>
> The old functionality of an additional argument to each usb read/write
> function to explicitly override the endpoint for a single transaction is still
> there, so one can now override the default used for all transfers as well as
> the endpoint for a single transfer.
>
> Any comments? Of course, I would love to get this into the 1.0.22 release, so
> other backends that need different endpoints for different communication types
> finally have this functionality available.
>
> Cheers,
> Reinhold
> --
> --
> Reinhold Kainhofer, reinhold at kainhofer.com, http://reinhold.kainhofer.com/
> ?* Financial & Actuarial Math., Vienna Univ. of Technology, Austria
> ?* http://www.fam.tuwien.ac.at/, DVR: 0005886
> ?* LilyPond, Music typesetting, http://www.lilypond.org
>



-- 
"The truth is an offense, but not a sin"

[sane-devel] Fwd: sanei_usb limitations

[Reinhold Kainhofer]

Am Samstag, 27. November 2010, um 00:10:54 schrieb m. allan noah:
> Reinhold Kainhofer has recently worked up a patch with an alternative
> implementation of your idea- and there was some discussion about yet a
> third mechanism, which relies on a 'setting' function to be called
> prior to transferring the data, much as the existing timeout control
> code works. I think that would be the best choice, not sure if
> Reinhold has gotten a chance to work on it.

Not yet, I currently have so many other things to do that I don't find the 
time to work on sane. It's on my list, though. 

Cheers,
Reinhold

-- 
--
Reinhold Kainhofer, reinhold at kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[sane-devel] Fwd: sanei_usb limitations

[m. allan noah]

resend to entire list...
---
Reinhold Kainhofer has recently worked up a patch with an alternative
implementation of your idea- and there was some discussion about yet a
third mechanism, which relies on a 'setting' function to be called
prior to transferring the data, much as the existing timeout control
code works. I think that would be the best choice, not sure if
Reinhold has gotten a chance to work on it.

allan

On Fri, Nov 26, 2010 at 5:00 PM, Hugo "Bonstra" Grostabussiat
 wrote:
> Hi,
> as I said in my last post, I am writing a backend for Hercules Scan at home
> 48 USB scanner.
> This device uses an unusual USB interface which has 3 bulk endpoints:
> * EP 0x01 is used to send commands to device
> * EP 0x82 is used to get the result of some commands
> * EP 0x83 is used to get scan data
> In its current state however, sanei_usb API does not allow us to choose
> what endpoint to use for bulk transfers. It will always use the first IN
> and OUT endpoints it finds for functions sanei_usb_read_bulk() ans
> sanei_usb_write_bulk() respectively.
>
> To overcome that limitation, I thought of adding a few functions to
> sanei_usb API to allow specifying an alternative endpoint (for instance:
> sanei_usb_read_bulk_ep() which would take an additional ep argument of
> type SANE_Byte). It wouldn't break any existing backend.
> Would a change of that kind be accepted ? Or should I use libusb directly ?
>
> Thanks in advance.
> ---
> Hugo Grostabussiat
>
> --
> sane-devel mailing list: sane-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/sane-devel
> Unsubscribe: Send mail with subject "unsubscribe your_password"
> ? ? ? ? ? ? to sane-devel-request at lists.alioth.debian.org
>



--
"The truth is an offense, but not a sin"



-- 
"The truth is an offense, but not a sin"