ChrisPulman/SerialPortRx

Add EventLoopScheduler in timeout handling

Closed this issue · 6 comments

djonasdev commented

When using your code https://github.com/ChrisPulman/SerialPortRx/blob/master/SerialPortRx/SerialPortRxMixins.cs#L125, you will see that (for example 100ms) the delay is not working as expected:

            Observable.Interval(TimeSpan.FromMilliseconds(1)).Subscribe(_ => {
                elapsedTime++;
                if (elapsedTime > timeOut)
                {
                    Trace.WriteLine(DateTime.Now.ToString("HH:mm:ss:FFF"));

If you additional add an EventLoopScheduler, everything works as expected

            Observable.Interval(TimeSpan.FromMilliseconds(1), new EventLoopScheduler()).Subscribe(_ => {
                elapsedTime++;
                if (elapsedTime > timeOut)
                {
                    Trace.WriteLine(DateTime.Now.ToString("HH:mm:ss:FFF"));
ChrisPulman commented

Thank you for this feedback, is there a particular platform that this is an issue with?
I should expose a Scheduler option for the all the Functions, which should overcome this issue.

djonasdev commented

I'm currently working with WinUI3 in combination with NET6.

I should expose a Scheduler option for the all the Functions, which should overcome this issue.
That's definitely not a bad idea 👍

ChrisPulman commented

Released SerialPortRx V2.1.2 which should fix this

ChrisPulman commented

Can you confirm this is now resolved.

djonasdev commented

Hello Chris,

thanks for the quick implementation.
Since I implemented your source code directly into my project, I can't test it. I think you have already tested it locally and were able to reproduce the error?

Another tip on my own behalf. If you do refactoring, push that in a separate commit. This makes it much easier to read the relevant code changes. 😉

ChrisPulman commented

Sorry about that, I realised that I was following some old coding standards and wanted to standardise on something more up to date.
Again thank you for your feedback, hopefully this will assist others who may have been experiencing the same issue but not spent the time to report it.

  • 👍1