poshbotio / PoshBot

Powershell-based bot framework
MIT License
540 stars 108 forks source link

ScheduledCommands fire off every IntervalMS from the time the schedule is created, not when the command is scheduled to run #85

Closed JasonNoonan closed 6 years ago

JasonNoonan commented 6 years ago

Expected Behavior

My coworker schedules the !dilbert command to fire every day at 7AM. I expect that on the next cycle after the scheduled time has passed, the command will be fired off.

Current Behavior

When the ScheduledCommand is created and initialized, a StopWatch object is created and the timer is started. Every cycle, the StopWatch is checked to see if it is greater than the defined IntervalMS. This results in (for instance) a command that is scheduled to run at 7AM every day instead firing off daily at 2:45PM because that is when I happened to create the scheduled command.

Possible Solution

Modify the Scheduler logic so that the HasExpired method checks to see if the StartAfter property is less than the current date (which is currently being done after the IntervalMS check in the module). If so, fire off the command.

Modify the RestartTimer function to modify the StartAfter property with $this.StartAfter.AddMilliseconds($IntervalMS) so that the next run triggers correctly

Steps to Reproduce (for bugs)

  1. Create a scheduled command, with a "StartAfter" value that is different from the increment amount
  2. Wait for the command to fire off
  3. Observe that the command is fired off $IntervalMS milliseconds after you created the command, not at the time you configured it to fire off

Your Environment

JasonNoonan commented 6 years ago

I have fixed this in my local module's files. I'll clean it up to match with your coding standards and send a merge request once I get some spare cycles.

devblackops commented 6 years ago

Ooh! Thanks for catching that!

devblackops commented 6 years ago

@JasonNoonan How is the PR coming?

JasonNoonan commented 6 years ago

I'll have to go double-check, but last time I looked at the code it appeared that the issue had already been resolved. Apologies for the delay, I've been pretty wrapped up in work.

JasonNoonan commented 6 years ago

Alright, non-zero chance that I'm an insane person and imagined seeing the fix. Away I go!