c4ev3 / ev3duder

The LEGO® EV3 Downloader/UploaDER.
http://c4ev3.github.io/ev3duder/doc/html/globals_func.html
GNU General Public License v3.0
32 stars 12 forks source link

ev3duder not correctly working for Bluetooth on Windows #40

Closed germanicianus closed 3 years ago

germanicianus commented 3 years ago

I cross-compiled ev3duder on Ubuntu 18 LTS and directly on Windows 7 - both with mingw64 32 bit.

Both compiled binaries don't work correctly for Bluetooth on Windows 7.

I am able to issue the info command successfully 2 times in a row which make the EV3 beep 2 times. After that, apparently the command cannot be sent anymore. The ls, dl and run commands don't work at all. I didn't verify the other commands.

The version compiled on/for Ubuntu 18 LTS works correctly for Bluetooth, except that the run command only succeeds once (the rbf is run on the EV3) and afterwards no commands succeed anymore.

JakubVanek commented 3 years ago

I managed to reproduce something similar earlier today, but I wasn't sure if it wasn't just a glitch of the VM that I was running Windows in. I only tried ev3duder ls and I didn't get a reply.

germanicianus commented 3 years ago

I just tested it on Windows 8.1 32 bit directly running on an Intel Atom Z3740D tablet (Version 6.3.9600 in cmd).

It has shown (nearly) the same behavior: info worked every time (executed 3 times), but an ls afterwards didn't yield any reply - ev3duder didn't return, so I had to CTRL+C.

JakubVanek commented 3 years ago

It seems that ev3duder cannot receive replies from the brick (at least the ls command cannot). I managed to capture the Bluetooth communication for the info and ls commands and this is what I got:

sent command for info (no reply required): info

sent command for ls: ls-tx

received reply for ls: ls-rx

The communication itself looks okay, so I think either ev3duder doesn't get the received data ~or the code for the ls command (and possibly others) cannot deal with replies received over Bluetooth~. Hmm, that would mean that ls wouldn't work on other platforms, but it does work on Linux...

JakubVanek commented 3 years ago

I suspect that the problem is here: https://github.com/c4ev3/ev3duder/blob/c840e0fc873a0ef589fbf739ff9f5467396e1ede/src/bt-win.c#L61 I don't have experience with WinAPI, but the ReadFile() call might be waiting for a full-sized buffer that will never arrive (the reply is shorter than the maximum packet length).

This is partially confirmed by the observation that running ev3duder --serial=COM4 ls /usr/bin does print the majority of the files and it only gets stuck at the end. This would indicate that most messages fill the reply buffer completely (and so get received properly) and only the final smaller one is not received by ev3duder.

JakubVanek commented 3 years ago

It seems that the Linux/Unix variant of the same code has to handle this too. It first tries to read the length header and only then attempts to read the rest of the buffer: https://github.com/c4ev3/ev3duder/blob/c840e0fc873a0ef589fbf739ff9f5467396e1ede/src/bt-unix.c#L111

I think the code can be adapted for Windows as well. Without timeout support it should be straightforward. Timeout support is going to be a bit more complicated as I don't expect Windows to have the same select() function, but timeouts aren't currently supported anyway.

germanicianus commented 3 years ago

@JakubVanek, the code change in attached patch fixes it for me: bt-win.patch.zip I tested info, ls, up, dl and rm.

Attention: Maybe this is not the correct place for the fix. Also, the printf should be removed.

Important code part in function _*btopen:

    COMMTIMEOUTS cto;
    GetCommTimeouts(handle,&cto);
    cto.ReadIntervalTimeout = 10;
    SetCommTimeouts(handle,&cto);

Source of information:

germanicianus commented 3 years ago

Patch with serial connection info output: bt-win_patch-02.zip Example output:

Z:\>ver

Microsoft Windows [Version 6.1.7601]

Z:\>ev3duder.exe --serial=com3 ls ../prjs/
DCB: BaudRate=115200, ByteSize=8, StopBits=0, Parity=0
COMMTIMEOUTS: ReadIntervalTimeout=10, ReadTotalTimeoutMultiplier=0, ReadTotalTim
eoutConstant=0, WriteTotalTimeoutMultiplier=0, WriteTotalTimeoutConstant=0
BrkProg_SAVE/
Projekt/
NEPOprog/
../
./
`LIST_FILES` was successful.

Z:\>

Reference: https://aticleworld.com/serial-port-programming-using-win32-api/

JakubVanek commented 3 years ago

Thank you! I have prepared PR https://github.com/c4ev3/ev3duder/pull/42 that configures COMMTIMEOUTS. I tried to add absolute timeout support as well but that broke the communication. I also increased the timeout to 100 ms as I received a truncated file from the brick when the inter-byte timeout was only 10 ms.

I think that the receive code could be improved in the future by reading the packet according to the length header (first 2 bytes). The Unix code does this and I think that is a better way of handling this. However, COMMTIMEOUTS seem to work fine so far and the code can always be improved later.