You prefer having placeholder in the topic instead of the wildcard. The implementation should parse and create the regex for matching the topic. On an incoming event it extract the the data, map it to the right placeholder and provide this as properties or dictionary.
Then the topic as argument is unnecessary. Like this:
Your code should just work like this. Because in on_raw_message the concrete topic that matches the wildcard (like "hermes/hotword/foobar/detected") is passed, and you pass this to the decorated function with function(topic, payload). So in the function the topic name is in the topic argument.
Right if the app creates a regex to match the topic to the right decorated function.
I have to look how complex wildcards can be and if I’m able to create the right regex on the fly. I’m not a specialist for regex.
It seems there are only + and #. So it should be possible for me to create a regex on the fly.
But the more I think about it, with placeholders could also be a nice solution, or a mix of both.
With placeholders would look nicer (and more easy to read for ppl without programming background) but I want to note that the placeholder should not be fixed in one place only.
For the example of printer/{name}/state it works but someone might come along that wants to filter by state also, or has more abstruse hermes topics because one IoT device sets a strange topic that switches the order around.
I think in this case a bit to far might be better. I have only started playing around with mqtt when I turned up in this forum but I have found some strange mqtt topics around the internet. I even came across one example where someone somehow had the state in the middle.
Most ppl would never do this but since the library is a way to make it easier for less experienced users some strange cases might turn up and depending on how long ago they formed the habit they might be unwilling to change. And like I said, some IoT devices set pretty strange topics, too.
I was mistaken here. I overlooked that the code checks for equality between the key and the topic. But I think with some minor changes this could be made to work.
So there are two separate ideas in the current discussion:
I think we should drop the regex pattern, so the on_topic decorator only needs one argument: a string. With some modifications the code could interpret this string as a topic wildcard and create a regular expression that matches all topics matching this wildcard. In the on_raw_message function the incoming topic would then be matched with the regular expression to see which decorated function should be called. This way you can decorate a function with @app.on_topic("printer/+/+/state") and the function would be called for topics printer/office/brother/state, printer/livingroom/hp/state and so on. The same could be done for the # wildcard.
I like @fastjack’s idea of the templates and named properties. But because there are very few restrictions on MQTT topic names and printer/{room}/{name}/state is a valid MQTT topic, I think we shouldn’t use a string for this argument, but define (or re-use? this looks like very general functionality, so maybe we could re-use some existing minimal templating language module for his) a specific class for this. Or maybe just define another decorator for this, such as on_topic_template.
I think having 1. first and later extending the code to have 2. would be good. 1. would be a nice feature for the first published version of rhasspy-hermes-app on PyPI, so people can already react to Hermes messages that don’t have their own decorator yet in the library.
This is very nice. If you’re ambitious, you could try adding @fastjack’s suggestion to add multiple topics. I think this is doable by changing the decorator’s signature to:
def on_topic(self, *topic_names: str):
And then put almost all the code inside the wrapper function in a loop for topic_name in topic_names: and then probably some minor changes elsewhere.
I’ll have a closer look tomorrow and test it before merging the PR.
The examples are purely fictional, of course. The goal is to have specific decorators for Hermes messages that work on Rhasspy Hermes classes, but in the mean time, you can use this on_topic decorator for Hermes messages that don’t have their own decorator yet. This also makes it easy to react to both Hermes messages and non-Hermes MQTT messages.
I just created a first rough API documentation. You can generate it with:
make docs
Afterwards you can find the generated HTML pages in docs/build/html. The plan is to extend this documentation and publish it for easier discovery of the library’s functionality.
With the new “on_topic” decorator I am trying to handle the “intent not recognized” event like this:
@app.on_topic("hermes/dialogueManager/intentNotRecognized")
def intent_not_recognized(data: TopicData, payload: bytes):
_LOGGER.debug("not recognized: " + str(data) + str(payload))
payload_json = json.loads(payload.decode("utf-8"))
session_id = payload_json.get("sessionId")
site_id = payload_json.get("siteId")
if session_id in akinator_sessions:
text = 'Das habe ich nicht verstanden'
app.publish(
DialogueContinueSession(
session_id=session_id,
site_id=site_id,
text=text,
intent_filter=intent_filter,
custom_data=None,
send_intent_not_recognized=True,
slot=None
)
)
The problem is: It only kind of works. Rhasspy seems to push some or most of the TTS message into the STT system. Causing an infinite loop of intent_not_recognized events. It should wait until the TTS output is finished before recording. It does that normally, why not when the ContinueSession is published from the intent not recognized event. Is that a Rhasspy bug?
Regarding the api:
It would be great if there were a parameter in the decorator to say the system “payload is json” and it doing the json-load and decode for you. Even better in my case a on_intent_not_recognized decorator´
I think hermes/dialogueManager/intentNotRecognized is the correct one to use. It’s behavior is depending on the send_intent_not_recognized parameter of the DialogueContinueSession message. (True will issue the intentNotRecognized event False will end the session. I looked at the Rhasspy source).
Sadly with both it will capture part of the TTS response as input.
Thank you. I would need the other one. But there are probably use case to have both. The nlu one will react to every not recognized intent the dialogueManager only in an active dialog session (at least that’s my understanding)
