Dont throw exception in SerialStream constructor if dtr or rts is not available#48577
Dont throw exception in SerialStream constructor if dtr or rts is not available#48577krwq merged 3 commits intodotnet:mainfrom sleeuwen:serialstream-virtualport-ctor-exception
Conversation
Dont throw IOException in SerialStream constructor when using a virtual port on linux. Setting DtrEnable or RtsEnable are not possible due to the termios commands not implemented by socat sockets. This fix ensures no exception is thrown in the constructor when we didn't even set the dtrEnable or rtsEnable parameters explicitly and allows the usage of SerialStream with those virtual ports.
|
|
| { | ||
| DtrEnable = dtrEnable; | ||
| } | ||
| catch (IOException) |
There was a problem hiding this comment.
This constructor is internal. It might be a good idea to document the new behavior in the APIs that consume it.
There was a problem hiding this comment.
I'm wondering if we should swallow the exception only when dtrEnable=false. I see no reason to fail in that case. However when somebody as asking for DTR I'm not sure if it is ok to silently ignore the request.
As far as the testing @carlossanlop : we currently do not have permanent setup with serial hardware and this is also HW specific e.g. difficult to test in general environment.
There was a problem hiding this comment.
@carlossanlop I'm not sure I understand what you're asking with documenting the new behavior. There is only one consumer as far as I can see, and that is the SerialPort in it's Open() method.
@wfurt I'm fine with only swallowing the exception if dtrEnable=false, but before I do I'd like confirmation that's what needs to be done.
There was a problem hiding this comment.
@wfurt Can you confirm that we should only swallow the exception when dtrEnable=false?
There was a problem hiding this comment.
I agree with @wfurt swallowing an exception is only acceptable when dtrEnable=false and similarly for rts. Even better solution could be to try to see what native call returns and avoid throwing/catching completely
|
I've updated the PR to only swallow the exception when the value it's trying to set is |
krwq
left a comment
krwq
left a comment
There was a problem hiding this comment.
Looks good to me as it's better than it was before. I'd prefer we didn't throw and catch but that can be improved in the future.
Thanks for the contribution!
|
Is it possible to get this commit into a build sooner than November? |
|
Is it possible to fix the same issue when using a virtual port on Windows? |
|
@IFoundTheLight I think that nuget package might already be available: https://www.nuget.org/packages/System.IO.Ports/6.0.0-preview.5.21301.5 @LeonidFirdman possibly yes - note that we are currently not investing in this area so we will be relying solely on contributions... |
|
@krwq Currently, this issue affects all new Intel platforms because starting from the SkyLake in Core-i platforms and the ApolloLake in the Atom platforms, the Serial IO UART interface is implemented with TX, RX, CTS, RTS signals only. |
|
@LeonidFirdman I'd recommend to start with creating a new Windows specific issue and provide link to this PR and all info like the one you shared above |
ghost
locked as resolved and limited conversation to collaborators
9 participants
Dont throw IOException in SerialStream constructor when setting dtr and/or rts is not possible (for example virtual ports on Linux cannot set this and currently crash in the constructor of SerialStream making it impossible to use on those ports)
This fixes #31523 using the suggestion in #37841. This only fixes the Linux version, not the windows one, so I don't think it completely fixes #37841 (I could probably add the try/catch in the windows implementation as well if you want me to).