fraschetti / Octoslack

OctoPrint plugin for Slack, Mattermost, Pushbullet, Pushover, Rocket.Chat, Discord, Riot/Matrix, & Microsoft Teams
MIT License
74 stars 34 forks source link

add MetadataAnalysisFinished to possible events #48

Closed tedder closed 5 years ago

tedder commented 5 years ago

I want to get pinged when MetadataAnalysisFinished is done, with some specifics:

Creating this though I intend on doing it and submitting a PR. Still, putting it out here.

fraschetti commented 5 years ago

Hi @tedder

I've started work on this request. Some of the properties you mentioned aren't in the stock analysis logic so I'm assuming you're using OctoPrint-PrintTimeGenius. It'll only take a few extra lines of code to handle both scenarios. Stay tuned.

fraschetti commented 5 years ago

Good news and bad news.

Good news - implementing the new event type and adding extra support for PrintTimeGenius wasn't much extra work.

Bad news - When PrintTimeGenius is in play, the MetadataAnalysisFinished event can fire multiple times with a variety of variable combinations. I've only ever seen this happen with larger files/longer prints but I've seen the event fire twice, one of the two missing all parameters (even though analysisPending == false). analysisPrintTime but not compensatedPrintTime, etc. I'll keep the support in place but wanted to set expectations. I had considered using a short ttl cache to eliminate the duplicates (use the first, ignore the second) but then observed the inconsitent argument scenario where the first emitted event was missing arguments but the second was not (I suspect it could easily be the other way around).

I'll commit what I have in the near-ish future as it works just fine for the stock analyzer and has a quirk for PrintTimeGenius but is otherwise functional.

fraschetti commented 5 years ago

Hi @tedder

I've added what support I can for these events. I suspect I'd been to submit changes to PrintTimeGenius if we wanted to clean up some of the noise it produces. Aside from that, Octoslack 1.9.0 should have these new events for you to play around with.

Good luck!