Automatically decode JSON payload in on_topic decorator
koenvervloesem opened this issue · 8 comments
From the forum:
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.
This would actually help issue #12 aswell. By decoding the payload before passing to the decorated method to a dictionary for example, you could access the slots by just payload[slots] if i understood it correctly.
But then this example "@app.on_topic("hermes/+/{site_id}/playBytes/#")" would not work anymore. In some of my cases I need the payload as bytes.
Maybe with a second parameter we can force decoding from Json and putting the result into TopicData and let payload untouched or empty.
Yes, I would definitely make this optional. @H3adcra5h are you using the on_topic decorator in your projects? I haven't been using it, so these and other issues have been lingering, but I'm all for it and if someone creates a pull request I'll definitely merge it.
Yes I do. That's why I implemented this. In my app(s) I do much more than intent handling. Some of my own sensors communicating over MQTT and I use this project too. Otherwise I would have to implement it twice.
On simple example where I use the bytes is publishing messages for all satellites if an important alert comes in. My master is running complete headless, no mic no speaker. For alerting if use hermes/tts/say on the master I copy the results from hermes/audioServer/master/playBytes to all satellites. It's faster than using "say" for all satellites.
Yes, I would definitely make this optional.
I would definitely make it opt out though. I think for most usecases a standard decoded format is the way to go.
It depends on the use case of course. I'm not sure which use cases are the most popular, because frankly I have no clue who is using rhasspy-hermes-app :-)
In SnipsKit I did decode the payload as JSON by default: https://snipskit.readthedocs.io/en/latest/API.html#module-snipskit.mqtt.decorators. If I remember correctly, I did this because then you wouldn't have to do anything special to be able to address the JSON content like a dict, e.g. payload[slots] and so on.
What other use cases are you seeing, @JonahKr?
I was more thinking about the users rather than the specific appliences.
It is simpler and more straightforeward if you have all the data you could need directly available instead of going the additional step of destructing it. Its just another feature lowering the bar for new users.
Additionally it wouldn't be a breaking change for existing apps.
@app.on_topic("app") would still work and binary data could be accessible like @app.on_topic("app", bin_payload=True)
The intention for my implementation was the possibility to handle topics outside of the main scope "intent handling" in a flexible way. The decorator will definitely work with all topics.
Most of the topics in rhasspy are json based, that's true, but not all, e.g. playBytes or audioCaptured. If you want to make it as easy as possible, than all main topics should have it's own decorator.
I have no clue if this is useful for others, definitely not for me. In my eyes the flexibility is better way and I think content = json.loads(payload.decode('utf-8')) istn't too heavy to understand for beginners.