mqtt-tools / mqttwarn

A highly configurable MQTT message router, where the routing targets are notification plugins, primarily written in Python.
https://mqttwarn.readthedocs.io/
Eclipse Public License 2.0
957 stars 184 forks source link

Multiple filters per topic #30

Closed jpmens closed 10 years ago

jpmens commented 10 years ago

Reported/suggested by @sumnerboy12 and I had stumbled over this problem earlier too :)

I have just noticed an issue with my mqttwarn config however. I already had a notification set up for enter/leave events using OwnTracks. This was based on your example and involves adding a custom filter to check for 'event' messages, and formatting to 'ben: enter => home' etc.

This works like a charm, however now I want to add a second notification based on the 'batt' parameter.

Because the filters are applied at the start of the processing loop, the first filter (in my case it seems to be the battery filter) is applied to all OwnTracks publishes and so the event stuff never gets hit.

Not sure of the easiest way to deal with this, but I am starting to think it might be better to have a separate set of config for each topic. This would allow multiple definitions of a given topic (or allow overlapping topics) to all be processed one-by-one with their own config, without fear of overlapping filters etc impacting on one another.

Not sure how it should be defined, but the general idea would be something like;

# topic name is the section header, followed by the config for that topic
[/owntracks/ben]
target = ['pushover', 'twitter']
format = 'Ben\'s phone battery getting low ({batt}%)!'
priority = 1
title = 'OwnTracks'
filter = owntracks_battfilter
jpmens commented 10 years ago

The proposed config file format reminds me of ini-file, but we should keep it as Python in any case, in order to allow functions etc in it.

sumnerboy12 commented 10 years ago

Yes - definitely agree it needs to be python - the custom filters/mappers work very well so don't want to lose those. I am no expert on how python config files can be structured however...unless we move the custom functions out into a separate file (mqttwarn.func?) and have the config file as a simple INI? Don't want to over complicate I know, but this allows a nice clean/simple INI style config, and then the complex custom functions live elsewhere..

matbor commented 10 years ago

I tired doing the same as Ben with the battery monitor and enter/leave function, plus a third that I'm working on, publishes to another topic the last known suburb the person was in.

I like the idea of moving the functions to a separate file, keeps the config file a bit cleaner... you could even have a functions folder, similar to the plugin, but that might be a bit over the top.

jpmens commented 10 years ago

@sumnerboy12 I've been toying with this, and I'm not particularly happy. The ini-style thing starts off looking OK, and I can extract lists, dicts, and what have you, but things get very messy quickly.

Consider this

[defaults]
logformat = '%(asctime)-15s %(levelname)-5s [%(module)s] %(message)s'
# File containing user-defined functions
functions = f1

[mqtt]
host = 'localhost'
port = 1883
username = None

[top/1]
target = ['pushover:one', 'twitter:jpmens' ]
format = '%(bar) me'
func = jpmensx

[config_file]
append_newline = true

[config_mqtt]
host = 'localhost'
port = 1883
qos = 0
retain = false

[in/a1]
module = ['mqtt:o1']
target = 'out/fruit/{fruit}'
format = 'Since when does a {fruit} cost {price}?'

[config_pushover]

[targets_pushover]
nagios = ['userkey1', 'appkey1']
alerts = ['userkey2', 'appkey2']

[openhab/warnings/#]
target = ['pushover', 'alerts']

[monitoring/+]
target = ['pushover', 'nagios']

[owntracks/+/+]

[hw/#]

[spa ce/in/top/+/ic/#]

[um/+/laü/+]

Configuring the individual service is easy. Then, all sections not called default, mqtt config_ etc could be read in as topics, but I'm not seeing how to cleanly specify topic to targets, etc.

sumnerboy12 commented 10 years ago

Yep I see your point. How about this, in my view we support a great number of services, but in reality each user will only use 2-3, maybe 4 of these in most scenarios. In my case it is Pushover, XBMC, SMTP and Twitter.

So what about having a config file for each service, i.e. pushover.conf, xbmc.conf, etc. Inside these we have the config and target definitions. These could live in either the individual service folder or the core folder - up to you. Either way it would be nice and easy to find config for a particular service, rather than scrolling through a huge config file.

Then, assuming we have a separate file for custom functions, mqttwarn.func, then all we are left with is the INI file config for each topic.

Thoughts?

jpmens commented 10 years ago

Show me what you think the main ini file would look like (i.e. the association between topic and target.

sumnerboy12 commented 10 years ago

[/owntracks/+] target = ['pushover', 'xbmc'] title = 'OwnTracks' priority = 0 format = u'{username}: {event} => {desc}' datamap = owntracks_data filter = owntracks_eventfilter

[/owntracks/ben] target = ['pushover:ben', 'twitter'] title = 'OwnTracks' priority = 1 format = u'Ben battery is getting low ({batt}%)!!' filter = owntracks_battfilter

sumnerboy12 commented 10 years ago

What are your thoughts JP? Am I missing anything here? Would you be happy to move the service config/targets into separate files? I think this has merit, as it will make maintaining the service config easier. You would only need config files for the services you are using. It might make sense for the config files to live in the /services sub-directory tho?

Then all you would have is the optional 'functions' file (which would be python) and the INI file in the root path. Very easy to add new topics in this manner, as long as you know what targets are available for each service type.

This would allow multiple processing loops for a single received message - i.e. for an OwnTracks location publish you could generate enter/leave notifications, as well as low battery notifications, as well as reverse geo-location lookups (as per @matbor).

One downside is that handling of the 'all' target - i.e. 'pushover' instead of 'pushover:ben' - would have to move into the individual service scripts - since the calling script doesn't know anything about the targets available. But I don't think that is a big deal to implement.

Happy to assist you with these changes if you think we are on the right track!

jpmens commented 10 years ago

Haven't forgotten you, mate! a) still thinking about this. b) new wall-to-wall in office so move out, move in. c) effing DSL router burned out this morning; several hours offline, awaiting replacement.

jpmens commented 10 years ago

@sumnerboy12 I just can't dig into this at the moment, and not before mid next-week probably.

I've created a new branch (ng-config) with a tests/ directory. The file c.py reads c.cfg and contains some of the experiments in reading ini files etc.

You're welcome to dig into this if you feel like it. Keep me posted.

sumnerboy12 commented 10 years ago

Hey JP,

Just pushed some changes to a separate branch - new_config_ben.

Only had brief testing but I think it is starting to come together. Attached are my config files, the two service ones live in the /services folder. Hopefully this gives you an idea of the structure...

One thing that definitely isn't working is the importing of the 'functions'. The file name is loaded but I am not sure how to actually load the code inside that file for use in the main loop, with the functions being able to be called by name (from the main config).

Everything else seems to be working so far, title, priority, etc.

So far this has meant no changes to the plugin scripts...which is great news!

Let me know what you think. I am away on holiday from Wed - Sun but will touch base upon my return.

jpmens commented 10 years ago

I've removed some stuff from your last comment which you, cough, probably didn't want here ... ;-)

Will look, thanks, & have a good rest.

sumnerboy12 commented 10 years ago

Whoops - thought that was a personal email...

JP Mens mailto:notifications@github.com Monday, 10 March 2014 10:23 a.m.

I've removed some stuff from your last comment which you, cough, probably didn't want here ... ;-)

Will look, thanks, & have a good rest.

— Reply to this email directly or view it on GitHub https://github.com/jpmens/mqttwarn/issues/30#issuecomment-37140132.

JP Mens mailto:notifications@github.com Thursday, 6 March 2014 6:05 a.m.

Reported/suggested by @sumnerboy12 https://github.com/sumnerboy12 and I had stumbled over this problem earlier too :)

# topic name is the section header, followed by the config for that topic [/owntracks/ben] target = ['pushover', 'twitter'] format = 'Ben\'s phone battery getting low ({batt}%)!' priority = 1 title = 'OwnTracks' filter = owntracks_battfilter

— Reply to this email directly or view it on GitHub https://github.com/jpmens/mqttwarn/issues/30.

jpmens commented 10 years ago

I've looked at this, Ben, and it's good stuff! I'll pick up where you left off and see if we can get this show on the road!

jpmens commented 10 years ago

To whoever is listening, this is now almost ready! A lot of stuff has changed, including a rename of default config to .ini to reflect the configuration file format. This is what we currently support:

[defaults]
hostname  = 'localhost'  ; default
port      = 1883
username  = None
password  = None
lwt       = 'clients/mqttwarn'

; logging
logformat = '%(asctime)-15s %(levelname)-5s [%(module)s] %(message)s'
logfile   = 'mqttwarn.log'

; one of: CRITICAL, DEBUG, ERROR, INFO, WARN
loglevel     = DEBUG

; path to file containing self-defined functions for formatmap and datamap
; omit the '.py' extension
functions = 'myfuncs'

; name the service providers you will be using.
launch   = file, log, osxnotify

[config:file]
append_newline = True
targets = {
    'f01'       : ['/tmp/f.01'],
    'log-me'    : ['/tmp/log.me'],
    }

[owntracks/+/+]
targets = log:info, file:f01
datamap = OwnTracksTopic2Data()
format  = OwnTracksConvert()
  1. loglevel is configurable
  2. all user-defined function are in a separate Python file configured as functions
  3. Each service has its individual configuration section called [config:<servicename>]
  4. datamap and format functions are invoked only if the names are alphanumeric and are followed by (). (Makes it look like a function name and is clearer.)

Code is currently being pushed to the newconfig branch here. Kindly look and feel ;-)

jpmens commented 10 years ago

Oh, and there's a new service called log which is configured thusly:

[config:log]
targets = {
    'info'   : [ 'info' ],
    'warn'   : [ 'warn' ],
    'crit'   : [ 'crit' ],
    'error'  : [ 'error' ]
  }

and the docs need a full rewrite!

jpmens commented 10 years ago

@sumnerboy12 As soon as you're back, I'd appreciate help in checking whether README.md makes sense, and in testing this branch.

If it's all good, I'll merge, and we'll roll with this.

sumnerboy12 commented 10 years ago

Looking good - made a couple of minor changes. One thing - I am still unable to setup two sets of config for the same topic - i.e. match '/owntracks/#' and handle enter/leave events as well as low battery events. This is because the config key is the topic match pattern. What if we just had some arbitrary value as the topic key and the topic was a parameter in the config?

[owntracks-events] topic = /owntracks/# targets = pushover:ben_ot title = OwnTracks priority = -1 datamap = owntracks_data() filter = owntracks_eventfilter() format = {username}: {event} => {desc}

[owntracks-battery] topic = /owntracks/# targets = pushover:ben_oh title = OwnTracks priority = -1 datamap = owntracks_data() filter = owntracks_battfilter() format = {username}'s phone battery is getting low ({batt}%)

jpmens commented 10 years ago

This particular case can be solved now with

[owntracks/+/+] [owntracks/Ben/+]

But I see what you mean.

Not specially fond of your suggestion, b/c I like look of topic names in section, but:

If you make it happen in such a way that optional topic= overrides section name, I'll take it.

Eg: both

[owntracks/+/+]

and

[Blurb] topic = owntracks/+/+

are equivalent. On Mar 16, 2014 8:47 AM, "Ben Jones" notifications@github.com wrote:

Looking good - made a couple of minor changes. One thing - I am still unable to setup two sets of config for the same topic - i.e. match '/owntracks/#' and handle enter/leave events as well as low battery events. This is because the config key is the topic match pattern. What if we just had some arbitrary value as the topic key and the topic was a parameter in the config?

[owntracks-events] topic = /owntracks/# targets = pushover:ben_ot title = OwnTracks priority = -1 datamap = owntracks_data() filter = owntracks_eventfilter() format = {username}: {event} => {desc}

[owntracks-battery] topic = /owntracks/# targets = pushover:ben_oh title = OwnTracks priority = -1 datamap = owntracks_data() filter = owntracks_battfilter() format = {username}'s phone battery is getting low ({batt}%)

Reply to this email directly or view it on GitHubhttps://github.com/jpmens/mqttwarn/issues/30#issuecomment-37751395 .

sumnerboy12 commented 10 years ago

Yep I had a feeling you would suggest that. I have implemented as you suggested and everything looks to be working quite nicely now. I am going to start running this new version in 'production' on my server.

jpmens commented 10 years ago

Thanks, Ben! Merged into master

jpmens commented 10 years ago

@sumnerboy12 I may have borked the README; would appreciate you checking (IIRC you had an example with topic = in the section, but I think I lost it ..... #terriblysorry

sumnerboy12 commented 10 years ago

No I think it looks ok. I didn't add an example with a topic option, as I figured it was well enough documented and is probably not going to be a highly used feature.

BTW - just pushed a fix to the master branch which stops topics being subscribed to multiple times if specified in multiple sections.

sumnerboy12 commented 10 years ago

I did however add an example to the mqttwarn.ini.sample...

jpmens commented 10 years ago

Pushed last change for this week (promise!) which consolidates the formatting code for format=, title= and what have you. I'd appreciate you testing this wether anything has broken.

sumnerboy12 commented 10 years ago

Looks to be fine for my config. Great work.