Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SerialPort ReadByte() does not longer raise the documented TimeoutException (after timeout reached) #78319

Closed
RobertK66 opened this issue Nov 14, 2022 · 10 comments

Comments

@RobertK66
Copy link

RobertK66 commented Nov 14, 2022

Description

After upgrading my code from 6.0.0 to 7.0.0 the System.IO.Ports.SerialPort.ReadByte() function no longer raises a TimeoutException as documented and expected. Iso it raises a System.IO.IOException with the (internationalized(?)) text: {"Dieser Vorgang wurde wegen Zeitüberschreitung zurückgegeben. : 'COM30'"} which then breaks the Read() loop.

Reproduction Steps

Use code from your example here: https://learn.microsoft.com/en-us/dotnet/api/system.io.ports.serialport.readtimeout?view=dotnet-plat-ext-7.0

Use a com port where a FTDI USB-UART adapter is connected.
Send char to the FTDI from external hardware -> read loop works and received chars are output. to screen.
Stop sending chars -> wait the configured timeout -> Read loop stops with exception.

Expected behavior

The Read() function should run forever because TimeoutException is catched with empty handler within the while loop.

Actual behavior

Iso TimeoutException a IOException with some text to explain the timeout occurrence is raised.

Regression?

Yes it worked in net6.0

Its broken with net7.0

Known Workarounds

check for generic IOException iso specific TimeoutException. Not easy because of internationalization of the message !?

Configuration

  • net7.0
  • Windows 11 Pro 21H2
  • AnyCPU
  • No, i haven't tested other platforms

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 14, 2022
@ghost
Copy link

ghost commented Nov 14, 2022

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

After upgrading my code from 6.0.0 to 7.0.0 the System.IO.Ports.SerialPort.ReadByte() function no longer raises a TimeoutException as documented and expected. Iso it raises a System.IO.IOException with the (internationalized(?)) text: {"Dieser Vorgang wurde wegen Zeitüberschreitung zurückgegeben. : 'COM30'"} which then breaks the Read() loop.

Reproduction Steps

Use code from your example here: https://learn.microsoft.com/en-us/dotnet/api/system.io.ports.serialport.readtimeout?view=dotnet-plat-ext-7.0

connect a terminal program to configured COM Port. Send some lines of character. (if bytes are sent fast enough it works normal)
wait the configured timout -> Read loop stops with exception.

Expected behavior

The Read() function should run forever because TimeoutException is catched with empty handler within the while loop.

Actual behavior

Iso TimeoutException a IOException with some text to explain the timeout occurrence is raised.

Regression?

Yes it worked in net6.0

Its broken with net7.0

Known Workarounds

check for generic IOException iso specific TimeoutException. Not easy because of internationalization of the message !?

Configuration

net7.0
Windows 11 Pro 21H2
AnyCPU
No, i haven't tested other platforms

Other information

No response

Author: RobertK66
Assignees: -
Labels:

area-System.IO

Milestone: -

RobertK66 added a commit to RobertK66/embedded-toolbox that referenced this issue Nov 14, 2022
@RobertK66
Copy link
Author

RobertK66 commented Nov 15, 2022

original description had an error for step to reproduce: "connect a terminal program to configured COM Port." is not the correct description here -> updated the original comment.

(I am using a FTDI UART to USB adapter and have some real embedded hardware (sending char packages) connected to this FTDI - COM Port!)

@iq2luc
Copy link

iq2luc commented Nov 16, 2022

@RobertK66 I confirm this issue on Windows, it broke all my programs using SerialPort.
No problems on Linux (x86_64 / 6.0.8).

@RobertK66
Copy link
Author

RobertK66 commented Nov 17, 2022

@iq2luc: I changed my read loop to use the BytesToRead property to check if a call to ReadByte() will block. This seems to be the cleaner workaround/implementation then using an exception for 'normal' functionality...

while(Continue) {
     while (sp.BytesToRead > 0) {
            var b = sp.ReadByte();
            debug.ProcessByte(b);
      }
      ...
}

But when I first coded this I only copied the example from the MS-Documentation and there the 'trick' with the TimeoutException was proposed.

@iq2luc
Copy link

iq2luc commented Nov 17, 2022

@RobertK66: My workaround is to completely ignore ReadTimeout (leave it to default value of InfiniteTimeout = -1) and manage timeouts at higher levels in the actual communication protocol (which - in my opinion - is a better approach anyway). So there is no need for two while loops, I simply do:

while(running) {
   int count = sp.Read(buffer, 0, buffer.Length)
   GotSomeBytes(buffer, count)
}

No more catching TimeoutException.

Still, the concept of throwing TimeoutException (as documented) is broken only in Windows. In Linux everything works as before.

Overall, while I consider it a bug anyway (due to inconsistencies and impossibility of discerning between Timeout related exceptions and IO exceptions), the code change I had to do makes things cleaner in my opinion (handling actual timeouts at higher levels makes more sense to me).

Thanks for the feedback and filling the bug in the first place.

@RobertK66
Copy link
Author

I tried to make a 'non blocking' version here. For strings with encoded chars this would be available with ReadExisting(). But for pure bytes this seems not to be available!?

As I understand MS doku, your version of the loop will never terminate if your RX line is quiet and no data gets received at all. Read will block (for ever when Timeout is set to -1) and the while never gets a chance to recheck the running flag....

@iq2luc
Copy link

iq2luc commented Nov 17, 2022

Yes, it is blocking and personally I prefer it that way. There is no need to "recheck" the running condition again unless I'm closing the port. When I want to terminate the loop and close the port: first set running = true, then sp.Close() and wait on reading thread to complete.

Something like this (simplified, no conditions / errors checking):

public void Connect() {
   _sp = new SerialPort() {...};
   _sp.Open();
   _rt = new Thread(ReadFromSerialPort) { IsBackground = true; }
   _rt.Start();
}

public void Disconnect() {
   _running = false;
   _sp.Close();
   if(!_rt.Join(timeout)) {
      // This should not happen normally, still...
      _rt.Interrupt();
   }
   _sp = null;
}

private void ReadFromSerialPort() {
   int count;
   _running = true;
   while(_running) {
      try {
         count = _sp.Read(_buffer, 0, _buffer.Length)
         // string line = _sp.ReadLine(); // Or just read string lines
      }
      catch(ThreadInterruptedException) {
         // If I'm not wrong this is thrown when thread interrupted
         break;
      }
      catch(Exception) { // Or more granular, catch IOException too etc.
         // Something went wrong, maybe port closed behind our back etc.
         break;
      }
      GotSomeBytes(_buffer, _count);
   }
   if(_running) { // Port closed behind our back, we didn't actually request to close it
      _running = false;
      // Treat the case accordingly
   }  
}

privat void GotSomeBytes(buf, count) {
   // Do something with the current buffer of "count" bytes
}

@RobertK66
Copy link
Author

Ahh I see,
Thanks for that feedback. Yes as long as we start our own thread for reading there should not be a problem with this blocking Read. I have not had the idea to kill the thread by other means with Closing or interrupting. I will give this a try. Thx again!

@krwq
Copy link
Member

krwq commented Jan 13, 2023

I'll close this issue in favor of #80079 which has some investigation done.

@krwq krwq closed this as completed Jan 13, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 13, 2023
@krwq
Copy link
Member

krwq commented Jan 13, 2023

Duplicate of #80079

@krwq krwq marked this as a duplicate of #80079 Jan 13, 2023
@krwq krwq closed this as completed Jan 13, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants