MycroftAI / mycroft-core

Mycroft Core, the Mycroft Artificial Intelligence platform.
https://mycroft.ai
Apache License 2.0
6.51k stars 1.27k forks source link

Feature: Allow mycroft-speak and mycroft-say-to take bash pipes #2105

Closed FruityWelsh closed 4 weeks ago

FruityWelsh commented 5 years ago

Add the ability to | into mycroft-speak and mycroft-say-to to open up scripting options.

FruityWelsh commented 5 years ago

Ran into a snag creating a unittest. Mostly I don't know where the unit-tests for the bin/ scripts are.

forslund commented 5 years ago

I do not think we have any unittests for the scripts yet.

Since the tests are of the shell scripts would they be unittests or integration tests? Basically can they be run in isolation or is a mycroft instance required?

FruityWelsh commented 5 years ago

I guess intergration would be more apt (from my understanding the scripts just send a preconfigured message onto the messagebus).

forslund commented 5 years ago

Yeah, that's exactly how they work.

I think adding a directory under the test/integrationtests folder for these would be the best place for them.

FruityWelsh commented 5 years ago

So something like: test_mycroft-speak_bash_pipe test_mycroft-speak_arg test_mycroft-say-to_bash_pipe test_mycroft-say-to_arg

To be honest I also have no idea how to write a test for a bash script that interfaces with python. Would I need to setup a mock messagebus to make sure that the messages are recieved correctly and that mycroft responds corretly?

forslund commented 5 years ago

For an integration test it would interact with a running mycroft messagebus.

I'm not certain myself what the best way to do this would be.

Iguesds the test would connect to the messagebus, listen for the speak message. Then launch the script as a subprocess wait a short interval and check that the expected message was received.

A bit complex to do. If you like to submit the change without a test that's ok.Iyt might be better investment of time if I wrote a unittestfor the underlying messagebus send function

FruityWelsh commented 5 years ago

Currently this works with discrete messages from the echo command, but does not work with cat or streams of text.

Scenarios I would like to work, but currently do not: cat testfile | mycroft-speak tail -f testfile | mycroft-speak

xargs might be a work around for this, but still not as simply as just using a |

forslund commented 5 years ago

For me this works:

$ echo "hello there" > say.txt
$ cat say.txt 
hello there
$ cat say.txt | ./bin/mycroft-speak
FruityWelsh commented 5 years ago

That works for me too. It seems to be more of an issue with the new line character and the other characters. Maybe we need to filter out special characters?

My testfiles are: Hello there General Kenobi

and the mycroft-say file which has a #, which also fails.

forslund commented 4 weeks ago

Closing Issue since we're archiving the repo