0

I have a class that handles some hardware. I want this class to check some state of the hardware every N milleseconds, and fire an event if a certain case is true.

I would like the interface on the outside to be such:

var rack = new Rack();
rack.ButtonPushed += OnButtonPushed;
rack.Initialise();

I want to avoid using "real" threads, but I don't seem to get the Task correctly (for testing, I just fire the event every second):

class Rack {
  public event EventHandler ButtonPushed;
  public void Initialise()
  {
    Task.Run(() =>
    {
        while (true)
        {
            Task.Delay(1000);
            ButtonPushed?.Invoke(this, null);
        }
    });
  }
}

But this doesn't work, the events get fired all at the same time.

Is Task the right thing to use here, or should I work with a Timer, or a thread?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Bart Friederichs
  • 33,050
  • 15
  • 95
  • 195
  • 6
    Shouldn't you be e.g. `await`ing for that `Task.Delay()` to complete..? See the Example for https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.task.delay?view=net-5.0#System_Threading_Tasks_Task_Delay_System_Int32_ – AKX Jul 29 '21 at 10:47
  • 6
    _"I want this class to check some state of the hardware every N milleseconds, and fire an event if a certain case is true"_ Maybe use [`Timer`](https://learn.microsoft.com/en-us/dotnet/api/system.threading.timer?view=net-5.0) then? – Guru Stron Jul 29 '21 at 10:55
  • When you have added `async` to the Task, you should probably provide means to cancel this Task. A default CancellationToken may do (plus a public method added to the Rack class). – Jimi Jul 29 '21 at 10:56
  • 1
    I second Guru. Task is not the tool of choice. Either a Timer or a Scheduling Framework is. – Fildor Jul 29 '21 at 10:56
  • Even with not awaited use of `Task.Delay` I'm not able [to reproduce](https://dotnetfiddle.net/WOwH8D) the issue with event handler beeing null. – Guru Stron Jul 29 '21 at 11:07
  • Microsoft's reactive framework is awesome for this: `Observable.Interval(TimeSpan.FromSeconds(1.0)).Subscribe(n => /* Do something */);` – Enigmativity Jul 29 '21 at 11:16
  • @Fabjan Not sure what exactly are you talking about. Can you explain a little bit more? Also are you sure you are not confusing `SynchronizationContext` in ASP with Thread Context and Framework ? Aslo switching to Framework still [does not reproduce the issue](https://dotnetfiddle.net/Hqz3Wx). – Guru Stron Jul 29 '21 at 11:30
  • @GuruStron Yeah, sorry I mean't `SynchronizationContext` – Fabjan Jul 29 '21 at 11:38
  • @Fabjan still not sure how it is related (though threading in theory can). – Guru Stron Jul 29 '21 at 11:49
  • 1
    @GuruStron that was my mistake. I had two instances running; one of them didn't register an event handler. – Bart Friederichs Jul 29 '21 at 11:50
  • @GuruStron I decided on the `Timer`. Simpler to implement and cleaner code (plus I couldn't get the `Task` version to run correctly) – Bart Friederichs Jul 29 '21 at 12:30

1 Answers1

3

Task delay returns a task that finishes execution after given time and needs to be awaited. In order to await it we'd need to mark the delegate as async and add await before the call:

class Rack {
  public event EventHandler ButtonPushed;
  public void Initialise()
  {
    Task.Run(async () =>
    {
        while (true)
        {
            await Task.Delay(1000);
            ButtonPushed?.Invoke(this, null);
        }
    });
  }
}

Is Task the right thing to use here, or should I work with a Timer, or a thread?

Well, your approach is just as good as using Timer. Thread could be used as well but it's a bit too low-level approach.

As others mentioned it would be better with CancellationToken, this allows us to cancel the task later so it finishes execution:

while (!cancellationToken.IsCancellationRequested)
{
   ...
}
Fabjan
  • 13,506
  • 4
  • 25
  • 52
  • Most likely it happens because of a different thread context. We attach handler in one thread but fire events in another thread – Fabjan Jul 29 '21 at 11:07
  • To answer that we need more details. Is it a console app or an app with GUI ? Which version of .NET Framework/Core did you use ? – Fabjan Jul 29 '21 at 11:11
  • I don't think there's any reason for the event handler to remain `null` if you follow the answer – asaf92 Jul 29 '21 at 11:12
  • Sorry I meant `SynchronizationContext` not `ThreadContext` – Fabjan Jul 29 '21 at 11:39
  • This works, but when I update a `TextBox.Text` property in the event handler, it seems to hang. I guess this is some kind of threading issue? It is a WinForms application BTW, .NET Framework 4.7.2 – Bart Friederichs Jul 29 '21 at 11:48
  • @BartFriederichs strange that it is hanging AFAIK it should just throw. Please see [this](https://stackoverflow.com/questions/661561/how-do-i-update-the-gui-from-another-thread). – Guru Stron Jul 29 '21 at 11:51
  • @GuruStron note the application doesn't hang, but when stepping through, it just switches back to the app and nothing is updated, so it looks like the Task hangs/blocks. Any subsequent calls aren't fired either. – Bart Friederichs Jul 29 '21 at 11:54
  • Pass a `Progress` delegate to the Task. Or `BeginInvoke()` (not `Invoke()`) in the event handler. Your event is raised in a ThreadPool Thread: you need to marshal to the UI Thread to modify properties of UI elements. – Jimi Jul 29 '21 at 12:07