gdombiak / OctoPrint-OctoPod

OctoPod plugin for OctoPrint. Used for sending real time push notifications
Apache License 2.0
70 stars 12 forks source link

"Bed warming target reached" notifications sending every x seconds #145

Open jackdalton2 opened 2 years ago

jackdalton2 commented 2 years ago

Expected behavior:

Actual behavior:

I'm fairly certain the actual behavior isn't the intended behavior, but please set me straight if it is.

Will follow up with a PR soon.

gdombiak commented 2 years ago

Hi Jack,

Thanks for bringing this up. I see that the UI is not clear about this intended behavior but I also see how you may want for this to work like you intended. I find it useful to get notifications starting at the specified minutes and then at that interval of minutes. I can also see how you may want to get the notification only once.

How about improving the UI so that:

  1. Text is clear that notification will be sent also at the specified interval (same minutes for first notification)
  2. Add an option to only be alerted once.

The logic to be alerted once should reset the flag once target temp changes so that user can get future notifications.

How does this sound to you? Let me know if you have other ideas. Thanks again, Gaston

jackdalton2 commented 2 years ago

That's a great idea, I'd be more than happy to work on that tomorrow and add this to my PR if this is a feature you're interested in.

I've always left this delay set to a minute, using it to let me know when to clean off oozing filament and start the print. Of course, today was the day I didn't make it to the printer within a minute and after 8 notifications I was scratching my head a little.

I'd never considered alternate use cases but that certainly makes sense, I can understand why you'd want them to repeat when the delay is set to a longer interval.

I think an additional option for the notification to be sent with zero delay and only once would also be beneficial, but I don't think it'd be worth the tradeoff of making the settings too cluttered. I think your idea for an option to only be alerted once and clarifying the UI is the way to go.

Let me know your thoughts on this,

Thanks!

jackdalton2 commented 2 years ago

Added c71a81e to implement this.