search

MycroftAI / fallback-wolfram-alpha

Mycroft AI official Wolfram Alpha Skill - used as a fallback if an Intent is not matched
https://mycroft.ai/skills
Apache License 2.0
8 stars 24 forks source link

Adding a event listener to respond with text without speaking #38

Open hughwilliams94 opened 4 years ago

hughwilliams94 commented 4 years ago

I was wanting to use information from WolframAlpha in another skill without mycroft speaking the results so I added a listener for wolfram.unspoken.request messages which triggers the emit_text_result method and a wolfram.unspoken.response message is then sent with the result.

I had to add the query_to_string method because the query method kept failing because wolfram.Result() did not like the output of 'BytesIO(data.content)`.

This is my first time contributing, and I was a little unsure of how the Api inheritance was working so I would love some feedback on how it can be improved.

Y0ngg4n commented 4 years ago

@hughwilliams94 Looks good 👍

krisgesling commented 4 years ago

Hey Hugh,

Thanks for the contribution, it's a great idea and would be a good use case for the Skill API however that isn't yet included in core. So this seems like a valid work around until then.

I would suggest we prepend skill to the Message names eg something like skill.wolfram.unspoken.request just to follow our namespacing convention.

However it's also not immediately apparent to me why one is "spoken" vs "unspoken". Perhaps the "spoken" method was intended to emulate the API endpoint. It seems that the differentiation is really localization, is that right? I.e. spoken requires geocoordinates and unit of measure, whilst this is a query only request.

Given this will be an externally facing interface (from this Skills perspective) I want to make sure we get the naming right.

I haven't done anything in this Skill before, does it even use that existing query method that you were having trouble with? I had a quick scan but couldn't see it being called from anywhere. It's unrelated to this PR but if it's not needed we can get rid of it.

hughwilliams94 commented 4 years ago

Sorry for a slow response, I've updated the message name adding 'skill'.

On the 'unspoken', I had used this to indicate that mycroft does not speak the response returned by Wolfram, but just sends it as a message.

I also couldn't see any usage of the query method in the skill.