elixir-circuits / circuits_uart

Discover and use UARTs and serial ports in Elixir
Apache License 2.0
189 stars 48 forks source link

uart_comm_win: reenable polling after EV_RXCHAR #35

Closed TypedLambda closed 6 years ago

TypedLambda commented 6 years ago

a spurious EV_RXCHAR event stopps anync reading. restart the polling by calling start_async_reads.

sourcelevel-bot[bot] commented 6 years ago

Hello, @TypedLambda! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

fhunleth commented 6 years ago

Hi @TypedLambda. Thank you so much for sending this over!

I think that the debug printf about "spurious RXCHAR" is misleading and that the UART device driver is reporting something else. Could you modify the debug printf to say what port->received_event_mask is set to? If you look at https://msdn.microsoft.com/en-us/library/windows/desktop/aa363479(v=vs.85).aspx and the lpEvtMask, it will hopefully be one of those. Having said that, your call to start_async_reads is definitely needed.

Also, what is your serial port configuration and what device (brand/model) are you using for the serial port? I'd like to try reproducing the issue over here.

TypedLambda commented 6 years ago

This is a Ultimaker 2 Go 3D printer. (usb serial, arduino based) on a Windows 10. I see a EV_TXEMPTY 0x0004.

TypedLambda commented 6 years ago

It is using 250000 baud.

fhunleth commented 6 years ago

With your fix, does everything else seem to work well? I haven't had a chance to try things out, but I'm wondering if there was a hiccup in transmitting since the EV_TXEMPTY isn't triggering a send.

TypedLambda commented 6 years ago

Reception seems to have no problems after this fix. the Device complains about CRC missmatch in correlation the the TXEMPTY event. I still do'nt know if data is missing (dropped during sending) because this is a USB only interface and i can't get the data, but that would be a bug lurking somware in the sending side.

TypedLambda commented 6 years ago

I don't even understand why i get that EV_TXEMPTY at all. it should me masked out should't it?

fhunleth commented 6 years ago

I'd assume that somehow the transmit queue filled up completely at some point and Windows felt obligated to notify us that it's empty again. It might be interesting to see if you put a short pause between sends whether that makes the CRC mismatch go away.

Sorry - real life is getting in the way at the moment for me to switch to Windows to help debug this week.

TypedLambda commented 6 years ago

Interesting I'll see if slowing down things could help.

I dont't even own a windows machine, just setting things up for a colleague. Actually im going to port Nerves Uart to FreeBSD next ;-)

fhunleth commented 6 years ago

Nice! It would be great to get this working on BSD too. Hopefully the Linux-isms won't be too hard to fix.

TypedLambda commented 6 years ago

Not really that hard. Just remove all special baudrate handling and use the posix interface for all settings. Finding the usb metadata for a port or which driver created it is the hard part :) The main linuxism is that cfsetspeed() doesn’t work. For FreeBSD it actually just works.

TypedLambda commented 6 years ago

The data corruption was a unrelated synchronization bug in our code. This fix works.

fhunleth commented 6 years ago

Great!! Sorry that I haven't been able to get to this. I'll reserve some time this week to try a couple things related to the change. If nothing else, I'll merge this PR as is and make a new release. Thanks for following up!

fhunleth commented 6 years ago

I merged this manually. Thanks!