Command strict ordering

[OldLodgeSkins]

Hi again! Now that I have my first two routines working, I have found something odd... Nothing major but I figured I'd notify you. In my morning routine I had the instruction "say good morning!" (so, with the word "morning" in it...). I ended up with an infinite loop, Mycroft was repeating the instructions over and over again. I have to re-start the skills service for it to stop. When I changed the phrase to "say good day!" instead it all stopped. That routine is working normally now. Also, there seems to be no specific order in which the instructions are executed - here again nothing major, I love this skill and I am definitely going to use it. Maybe just something to consider for a future release :)

[ChristopherRogers1991]

For the infinite loop - I'm wondering if the wake word triggers we noted in the logs on #3 are the culprit - if it is consistently hearing the wake word and listening for commands, it could hear itself saying 'morning' and re-trigger the routine. The word "morning" on it's own shouldn't have any meaning without the wake word being said first (I have not personally run into the loop issue, even with a similar routine defined).

As for the order - the events should be sent to Mycroft in order, but it's possible the processing takes longer for one than another. E.g. if you ask for the weather and the time, it may take longer to get the weather information than the time, so the time could potentially be reported by Mycroft before the weather. Can you check the skills log and verify the order in which the commands are actually being sent to Mycroft?

[ChristopherRogers1991]

Hey @OldLodgeSkins,

Just wanted to check in and see if you'd been able to check the logs to confirm the order of events/check if false wakewords are triggering the infinite loop. Let me know if you have any updates.

-Chris

[OldLodgeSkins]

Hi Christopher

Sorry for the delayed response... It's been a crazy 10 days for me. Anyway, concerning the loop I guess you're right although I don't see why Mycroft would re-start recording at that time... It seems to be a similar issue to the one I had when creating a new routine, and it is apparently not related to your skill. Anyway just by not having the word "morning" in the name of the routine I've been using it with no problem since the other day.

Concerning the order, I thought the same thing, it makes sense, however it might be interesting to force the order - i.e. don't start the next task until the current one is finished. I can live without it though, it's just food for thoughts.

[ChristopherRogers1991]

Updated the title to reflect the ordering feature request. I'm not sure there's anyway to enforce the order - not all commands give responses, so if I tried to watch the messagebus for indications the previous action completed, I could end up waiting forever. I could add a timeout, but it'd just be a guess, and the commands could still appear to run out of order. I'll look into what's available though, and see if there's any workable solution.

Something I've been considering that might help alleviate this is a "wait" command, that would simply sleep for for however long the user specifies. Then you could do something like:

1) tell me the weather 2) wait 5 seconds 3) tell me the time

Then it would be up to you as the user to guess roughly how long you need Mycroft to wait in order to get the commands to execute properly. It's a little manual/hacky, and it would still ultimately be guess work, but as the user you should be able to make more educated guesses than I can. I think a wait command will make it in regardless of whether I can enforce the order, as I think it will have other uses, but it may also be the best option for this as well.

[ChristopherRogers1991]

With some further investigation, it looks like I can add a source to the context of the message, and there are start and complete messages on the messagebus. I think I can set a unique source, and watch for completion messages before issuing the next command.

[ChristopherRogers1991]

Here's some sample code showing roughly how that would need to work:

chris@compy:/home/chris/Code/mycroft-core (dev) (02:13:46)
$ ipython
Python 3.6.7 (default, Oct 22 2018, 11:32:17)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.2.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import json

In [2]: from threading import Thread

In [3]: import websocket

In [4]: def on_message(ws, message):
   ...:     print(message)
   ...: 

In [5]: ws = websocket.WebSocketApp("ws://localhost:8181/core", on_message=on_message)

In [6]: t = Thread(target=ws.run_forever)

In [7]: data = {
   ...:             "type": "recognizer_loop:utterance",
   ...:             "context" : {"source": "my websocket 1234"},
   ...:             "data": {
   ...:                 "utterances": ["turn off the lamp"]
   ...: 
   ...:             }
   ...: }

In [8]: t.start()

{"type": "connected", "data": {}, "context": null}
{"type": "skill-reminder:reminder", "data": {}, "context": null}
{"type": "skill-date-time:TimeSkillupdate_display", "data": {}, "context": null}
In [9]: ws.send(json.dumps(data))

{"type": "recognizer_loop:utterance", "context": {"source": "my websocket 1234"}, "data": {"utterances": ["turn off the lamp"]}}
{"type": "skill.converse.request", "data": {"skill_id": "mycroft-hue", "utterances": ["turn off the lamp"], "lang": "en-us"}, "context": null}
{"type": "skill.converse.response", "data": {"skill_id": "mycroft-hue", "result": false}, "context": {}}
{"type": "mycroft-hue:ToggleIntent", "data": {"intent_type": "mycroft-hue:ToggleIntent", "mycroft_hueOffKeyword": "off", "mycroft_hueGroup": "lamp", "target": null, "confidence": 0.5384615384615385, "__tags__": [{"mycroft_hueOffKeyword": [{"start_token": 1, "entities": [{"key": "off", "match": "off", "data": [["off", "mycroft_hueOffKeyword"]], "confidence": 1.0}], "confidence": 1.0, "end_token": 1, "match": "off", "key": "off", "from_context": false}], "mycroft_hueGroup": [{"match": "lamp", "key": "lamp", "start_token": 2, "entities": [{"key": "lamp", "match": "lamp", "data": [["lamp", "mycroft_hueGroup"]], "confidence": 1.0}], "end_token": 2, "from_context": false}]}], "utterance": "turn off the lamp"}, "context": {"source": "my websocket 1234", "target": null}}                                          
{"type": "mycroft.skill.handler.start", "data": {"name": "PhillipsHueSkill.handler"}, "context": {"source": "my websocket 1234", "target": null}}
{"type": "mycroft.skill.handler.complete", "data": {"name": "PhillipsHueSkill.handler"}, "context": {"source": "my websocket 1234", "target": null}}
In [10]:                                 

[OldLodgeSkins]

Interesting :)

[ChristopherRogers1991]

@nonamewastaken flagged this in #7.

I've made an attempt at a fix. If you want to test it, you can try out the improvment/strict_task_order branch.

On that branch, I added logic to add a task_id to the Message context. The skill tracks that task_id, and waits for a mycroft.skill.handler.complete message on the bus with the same task_id. Once the skill gets that back (or after 30s), the next Message will be sent. In theory, this should resolve the issue of multiple things running at the same time, and tasks running out of order.

@OldLodgeSkins and @nonamewastaken, could you test on that branch, and let me know if it works ok?

[konstantinhirschfeld]

Yes, i am willing to test the addon. Could you please provide a URL to the corect version?

ChristopherRogers1991 notifications@github.com schrieb am Do., 27. Feb. 2020, 03:21:

@nonamewastaken https://github.com/nonamewastaken flagged this in #7 https://github.com/ChristopherRogers1991/mycroft_routine_skill/issues/7.

I've made an attempt at a fix. If you want to test it, you can try out the improvment/strict_task_order https://github.com/ChristopherRogers1991/mycroft_routine_skill/tree/improvment/strict_task_order branch.

On that branch, I added logic to add a task_id to the Message context. The skill tracks that task_id, and waits for a mycroft.skill.handler.complete message on the bus with the same task_id. Once the skill gets that back (or after 30s), the next Message will be sent. In theory, this should resolve the issue of multiple things running at the same time, and tasks running out of order.

@OldLodgeSkins https://github.com/OldLodgeSkins and @nonamewastaken https://github.com/nonamewastaken, could you test on that branch, and let me know if it works ok?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ChristopherRogers1991/mycroft_routine_skill/issues/4?email_source=notifications&email_token=AN3R44GGYVELLRXBGUCXKC3RE4PTHA5CNFSM4GF642FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENCU4IY#issuecomment-591744547, or unsubscribe https://github.com/notifications/unsubscribe-auth/AN3R44H3QBIA2TMSY4BV4SDRE4PTHANCNFSM4GF642FA .

[ChristopherRogers1991]

The fix is on this branch: https://github.com/ChristopherRogers1991/mycroft_routine_skill/tree/improvment/strict_task_order

You should be able to test with something like this:

cd <skill directory> && git fetch && git checkout improvment/strict_task_order