Scubapro Aladin Square

[Jef Driesen]

On 14-11-17 21:04, Linus Torvalds wrote:
> On Tue, Nov 14, 2017 at 11:57 AM, Jef Driesen <jef at libdivecomputer.org> wrote:
>>
>> I suspect this is not caused by a difference in the libusb version, but a
>> difference in the subsurface branch of libdivecomputer. If you look at the
>> g2.log of the successful download on Windows, then you can clearly see
>> libdivecomputer is sending out 33 byte packets (1 byte report id and 32
>> bytes payload).
> 
> .. but that's _exactly_ the usb library difference. On Windows, it's
> using hidapi instead. Just a library difference.

Ah ok, you are referring to libusb vs hidapi. I thought you were talking about a 
difference in the libusb version.

Give me a couple of minutes to build a libusb based windows version.

>> So somehow the subsurface branch doesn't use the right size.
>> It's probably the csize+1 in this code:
> 
> That "+1" should only trigger for GATT, since packet_size is 64 for usbhid, see
> 
>          static dc_custom_io_t custom = {
>                  .packet_size = 64,
>                  .packet_close = usbhid_packet_close,
>                  .packet_read  = usbhid_packet_read,
>                  .packet_write = usbhid_packet_write,
>          };
> 
> in dc_usbhid_custom_io().
> 
> That said, the part that *should* trigger:
> 
>>                  status = io->packet_write(io, buf, sizeof(buf), &transferred);

In of you previous emails, you asked Vincent to patch the

         if (io->packet_size < 64) {

to a

         if (1) {

So I assumed he did exactly that and in that case the code always takes that 
branch and thus also the csize+1!

> Already has that "sizeof(buf)", which is TX_PACKET_SIZE.
> 
> The reason it is then turned into 31 is because of this in src/usbhid.c:
> 
>          // Skip a report id of zero.
>          unsigned char report = buffer[0];
>          if (report == 0) {
>                  buffer++;
>                  length--;
>          }
> 
> and that's because at least _my_ usblib doesn't need/want that extra
> report byte.
> 
> I'm not sure if it hurts, though. So maybe that code could be disabled.

Ah that explains the difference. In upstream libdivecomputer the buffer is one 
byte larger:

unsigned char buf[TX_PACKET_SIZE + 1];

Jef