Refactor the notification module

[ghost]

The notification.py module currently supports four different messaging services, each with its own slightly different variation on how to collect and represent pokemon details, and all with various special cases scattered throughout the module. It's a bit of a tangled mess.

I want to add a new service (see #192), but I don't want to make this problem worse, so I have begun refactoring the module.

My goals:

• Consolidate similar operations / remove inconsistencies.
• Make the overall structure modular, so adding new messaging services will be easier / cleaner.
• Make rule representation more flexible, to allow new decision types (e.g. per-pokemon IVs).
• Lay groundwork for notifying about events other than sightings, such as administrative alerts and gym raids.
• Remain compatible with old config files and behaviors, so that my pull request can be accepted without breaking existing installations.

Assuming I don't run out of time or hit a roadblock, I expect I'll need help testing, to make sure I don't break the existing messaging services. (I don't use pushbullet, telegram, or twitter.) Anyone want to help with that?

[ghost]

Status update:

I have written the basic framework for my new rule system, and will soon be writing rules that implement Monocle's existing notification logic. It's looking good so far. When I'm satisfied that the rule system is working as intended, I plan to tackle the code for each messaging service.

Once those things are done, my new code should be capable of replacing the existing notification module. I'd appreciate some help testing it. My new code is currently in a module called "notify.py", partly because it's easier to type than "notification.py", but also because it gives us the option of letting the two modules coexist while fixing bugs and comparing behavior.

After that, I think it will make sense to add my SMTP implementation and some new rule types.

[ghost]

Progress continues.

I got sidetracked restructuring my code to ease future support of events other than pokemon sightings. For example, we might want to send administrative alerts (#146) or gym status notices (#158). This seems like a good time to lay some of the groundwork for that functionality, since I'm already rewriting most of the notification system.

I also found some threading and blocking i/o bugs in the current notification module, which my new code avoids.

[ghost]

Got a little busy last week, but I haven't abandoned this. Currently working out how to expose the flexibility I want without making the config file settings overly complex. Depending on what obligations come up in the next few days, I might be able to start early rule system tests as early as this week.

[DavePlater]

If you are worried about making the config complex, have you considered a secondary config? The regular config would just have like a reference to it that you could "turn on"? (I don't know much about how python works)

[ghost]

I don't think a secondary config file will be necessary; I have it mostly worked out.

I think I can use the existing NAME = 'value' style for basic messaging service configuration, and something like we see in pickle_landmarks.py for notification rules.

For advanced configurations, I also want to allow multiple notification channels, each with different rules. For example, a public channel that sends pokemon sightings to webhooks and twitter, a team channel that sends gym changes to a private discord server, and an admin channel that sends system errors to my phone. I'm not implementing all of this right away, but I want my code and config structure to allow adding these features later without forcing new config syntax on people who just want a simple upgrade.

[ghost]

I'm getting close, folks.

Thinking about what I want from a notification system in terms of functionality, usability, and backward compatibility, has led to a lot of questions, experiments, and redesigns since I first submitted this issue. I've invested more time than expected, but I think the result will be satisfying.

With most of the questions answered and new code written, I have finally switched it on for the first time and started looking for breakage. Bugs I've found so far have been minor, so this seems like a good time to share some highlights of what I've been developing:

• Each messaging service is encapsulated in its own class (rather than implemented by bits of conditional code scattered throughout the module), and pokemon sighting details are represented by a common set of variables shared by all these classes. Supporting new services will therefore be easier and less error-prone.
• Email notification is implemented. (Actually, it's the only messaging service implemented at the moment, but I'll convert the others to the new system soon.) This includes SMTP server authentication, TLS certificate validation, and multiple recipients. It can also attach the same pokemon images that get sent to Twitter.
• Pokemon sighting logic and data are encapsulated in such a way that other event types can be added in the future. For example, gym status notices or server error notices.
• A new rule system allows much more flexibility in configuring which pokemon trigger notifications. The old score/always/never settings are still supported for backward compatibility, but they could be deprecated in favor of the new system.
• It is possible to define multiple notification "channels", each with its own set of rules, messaging services, and recipients. Initially, this will allow (for example) separate twitter feeds for ultra-rare pokemon, good prestigers, and good defenders, along with email notices for your favorite pokemon. If/when additional event types are added, it could also support gym event notices sent only to members of a team, or private admin alerts when error conditions arise. All of this from a single Monocle instance.

Rule configuration is especially interesting, because it uses variable expressions instead of the awkwardly-encoded minimum/maximum value lists typical of other projects (like PokeAlarm's JSON-based filters). Here are a few examples:

NOTIFY.spawn(['dratini'], level > 20, iv > 80)
NOTIFY.spawn(..., rank < 20)
NOTIFY.spawn(('Dragonite', 'Blissey'), True, channel='friends')

The first line says we want Dratini above level 20 with IVs above 80%.

The second line says we want all pokemon with no rules of their own, if they are among the 20 rarest species.

The third line says the "friends" channel wants all Dragonite and Blissey sightings, regardless of their stats.

I am implementing the variables and comparisons necessary to mimic the old config settings, plus several more, in my first draft. More can be added over time. I have ideas for some pretty cool rule constructs that don't exist in any other scanner, but I don't want them to delay the release of this foundation work or consume more of my time right now. :)

Stay tuned. I'll have more to report soon. For now, back to initial testing and debugging.

[ghost]

At this moment, I have a Monocle instance running the old and new notification modules at the same time, with a lot of logging, to test the backward compatibility of my new module.

Almost every sighting yields two log messages (one from each notifier) indicating that the same decision is being made by both notifiers. When the decision is to notify, two identical messages arrive in my inbox. Every once in a while, the decisions diverge, and I investigate. It's kind of exciting to find that every one of the divergences I've seen so far has been due to the time-based "score" changing slightly over the few milliseconds that pass between each notifier running.

In other words, my new module's backward compatibility code is behaving exactly as if it were another instance of the old notification module. Sweet.

[ghost]

Further testing is on hold while awaiting Monocle updates for the latest game changes.

In the mean time, I'm adding twitter support to the new module. I noticed that the old module is monkey-patching peony in order to upload images without a dependency on the magic package. This is convenient for users, but it makes for fragile code, so I spent some time figuring out an alternative approach that I think will work without monkey-patching. I filed a bug report against peony while I was at it, in hopes that such workarounds can be avoided in the future.

[ghost]

I have resumed testing now that the API is up to date.

Side-by-side comparison of my code with the old notification module highlights some bad assumptions made by the old code, resulting in buggy behavior around pokemon rareness calculations with the ALWAYS_NOTIFY and ALWAYS_NOTIFY_IDS settings. I want to remain compatible with existing config files, but it looks like doing so will require emulating legacy bugs. I think I'm going to make the bug emulation configurable for now. It should be easy to remove at some point in the future, perhaps after a deprecation period.

[DavePlater]

I really don't know why you want to be backwards compatible with old configs. Save yourself a lot of hassle is you just drop it. Or at best, "use old system" only without your changes/fixes or "use new system" to use your changes. At some point you gotta draw a line in the sand.

[ghost]

I really don't know why you want to be backwards compatible with old configs. Save yourself a lot of hassle is you just drop it.

I appreciate your enthusiasm for the new stuff, but I believe the hassle is worth it. Here's why:

As someone who has spent a lot of time using and administering software, experience has shown that there are few things more frustrating than an "upgrade" that breaks things. It tends to happen when a developer either doesn't understand the code they're meddling with, or decides that saving a little time for themselves is worth wasting a lot of time for dozens, hundreds, or even millions of other people. When they needlessly break working systems or workflows because they couldn't be bothered with backward compatibility, phased changes, deprecation periods, or (at the very least) migration guides, I start looking for alternatives to their software, because they have already betrayed their users once and are likely to do it again in the future. I and my users have better things to do with our time than grapple with upstream blunders like that.

As someone who has spent a lot of time developing software, part of my motivation is to help people, not make things harder for them. Quality has always been important to me, both in the code itself and in the user experience. I have seen it proven time and time again that it is far worse to break working systems than it is to delay (or forgo) new features. While it is sometimes frustrating to wait for development that takes a bit longer this way, the additional time turns out to be insignificant in the long run, and it nearly always yields a net win due to happy users, fewer bugs, more flexible code, and less time spent later on revising or hacking around hasty early decisions.

On top of all that lie the facts that Monocle is not my project, and I am not interested in maintaining a fork. All this work I'm doing will barely see the light of day if my pull request is not accepted, so making my changes easy for @Noctem to approve would be wise. Backward compatibility will almost certainly be a factor in his decision.

[ghost]

After tracking down three apparent bugs in the old rareness-related code, and reworking my own code to (optionally) emulate them, I believe my module can now report exactly the same results as the old notification module. People with existing installations will be able to upgrade with no config changes and get results very close to those that came from the old module, or add a single emulate-legacy-bugs setting and get results that match the old decisions perfectly.

My work-in-progress is now functional enough that it is more useful to me than the old module, so I have made the switch in my personal Monocle instance. Eating my own dog food for a while helps me find things that could use improvement before I ask others to try it.

Next on my list: Finish twitter/pushbullet/webhook support, refine some rough spots in my code, and make a pull request out of all this.

[DavePlater]

Is there a way I can use your version? I've missed 2 snorlaxs today so far because i am not able to get any notifications to work

[ghost]

I haven't yet pushed my working code to anyplace public, but I am preparing it for that, and will post here when the time comes. Please be patient. Monocle's job is a moving target, the changes I'm making are nontrivial, and we're all working in our free time. Delays are to be expected here. This project is not abandoned, though.

I have been busy with other commitments for the past couple of weeks, but I did manage a basic test of multi-channel notifications, each channel with different rules. They work. :)

In case you haven't heard yet, the current round of account blinding / banning is still unresolved, and makes scanning unwise for now anyway. Probably best not to run Monocle today unless you're researching a fix.

[DavePlater]

Yeah I switched offline a little too late and caught a lot of BAN bans. I had a stop-gap in place with my reverse-proxy where i was able to auto-poll (outside of the map) and sift through pokes and get web(browser) notifications....but i only had a day or two with it before the ban hammer started :-(

[ghost]

I just did some initial testing of my Twitter sender. Looks good so far. It took longer than I expected because I was pondering how to imitate the adaptive formatting used by the old code without recreating its internal flaws. Some of the problems I solved:

• Hard-coded assumptions about navigation URL length
• Hard-coded tweet text
• Repeatedly searching/replacing various substrings in the hard-coded text, in attempts to fit tweets into 140 characters
• Failing to fit tweets into 140 characters and posting them anyway
• No way to configure a tweet format that remains consistent regardless of sighting details
• Alternate code paths in multiple places to accommodate sightings with & without exact despawn times

Somewhere along the way, I also got some help from the peony-twitter developer, in the form of a change to that library allowing us to use it without monkey patching it or writing images to disk.

I'll probably make some minor tweaks to this sender before calling it done, but I'm mostly happy with it already, so that shouldn't take long.

[ghost]

All the goals in my first post have already been met. The twitter and smtp senders are working quite well. The remaining senders for my first milestone (webhook/pushbullet/telegram) should go pretty quickly, once I start on them. What remains beyond that is mostly cleanup and tweaks based on what I've learned by using my new module instead of the official one.

I want to wrap up this refactor soon, but my progress has been slowed to a crawl due to a free time shortage and recent blinding, banning, and API changes (uk27). Nevertheless, I'm still hopeful that I'll have something worth pushing in the next couple weeks. Stay tuned.

[ghost]

I still haven't yet implemented webhook, pushbullet, or telegram senders, but since everything else works (including twitter and smtp) and several people have expressed interest in getting their hands on this stuff ASAP, I have pushed it to my "notfy" branch, here:

https://github.com/mewio/Monocle

As of this moment, the notify branch is a single commit. It is based on my "interim" branch, which contains protocol / version / gen3 updates to keep Monocle working since Noctem's last commit. In other words, my interim branch is like Chrales' fork without the new features that Chrales added.

The vast majority of the work is in two new files: notify.py and notifyconfig.py, the latter of which contains some introductory documentation. There are also new entries in config.example.py, and several new config vars in sanitized.py. To start getting familiar with the user-visible changes, I suggest reading the doc string at the top of notifyconfig.py. Those docs aren't exhaustive, but should be a good start for folks who just want to try the new features.

I believe this branch is completely compatible with existing vanilla Monocle config files (Noctem, not Chrales) that do not depend on webhook, pushbullet, or telegram. I plan to add those three senders next.

[ghost]

I just added a bunch of small changes, plus a Webhook sender. Haven't fully tested it yet; I might have to change some of the numbers from floating point to integer in order to match the old notification module's json output.

[ghost]

I just pushed an update, mostly for webhook tweaks and other fixes. Verified that webhook data types match those in vanilla monocle, so it should be compatible with existing installations. I rebased & squashed the notify branch while I was at it, so don't be surprised if git pull complains. Next up: pushbullet, telegram, discord.

[ghost]

My notify branch now supports pushbullet, as well as some new time-related message template variables. (All my senders use configurable text templates, which should help a lot with localization and personal style preferences.)

Once again, I rebased & squashed all commits on that branch while I was at it, so don't be surprised if git pull complains.

My "interim" branch (upon which "notify" is based) also now has the latest API version numbers, so it works with Niantic's servers as long as you have Chrales' latest aiopogo installed.

Next up: telegram, discord.

[ghost]

I just pushed another update to my repo. Telegram support is included, which means that my branch is now fully backward compatible with Noctem's development branch (and config files that worked with that branch).

Once again, I squashed all the new commits and force-pushed, so don't be surprised if git pull complains.

Next steps: Discord support, and then looking at what it will take to migrate my code from Noctem's Monocle to Chrales' fork.

[ghost]

I just pushed an update to my notify branch (squashed & rebased as usual), plus protocol version updates to my interim branch, so the whole thing is compatible with the current game servers.

This update includes Discord support, and with that, the code is now feature-complete.

The next step will be integration with Chrales' fork, so that my work can be used with the new de-facto standard Monocle (and all the features that it introduced since Noctem disappeared). I'll be coordinating with Chrales on this.