Fix #49 - Implement AMAZON.RepeatIntent for arrivals, city, and stop

[barbeau]

• Add unit test as well

[coveralls]

Changes Unknown when pulling 635773a1de3dbe089d4b728612a03841c517bf35 on repeat into \ on master**.

[philipmw]

What if, instead of remembering what was said, the skill simply re-queries OBA for the latest info? That would make the logic simpler, and more up-to-date to boot.

[barbeau]

I thought about that, but I think it's much harder to maintain going forward as more functionality is added. For example, if we add support for "Get me arrivals for stop X," then we'd need to store both the intent that was invoked as well as the parameters. Seems like this just clutters the DB with extra fields that will typically be empty. The primary use case is immediately repeating what the user just heard, so I think hitting the API again doesn't really gain much in terms of freshness, and just results in extra traffic.

[barbeau]

After further testing last night with the repeat intent, I definitely think we should stick to repeating the last message, vs. requerying for new info - I think it's a better UX. Lets say I ask for arrival times, and Alexa reads back five arrivals, and I'm interested in the fourth but miss the exact time due to background noise. I say repeat, and am waiting to hear the fourth arrival again so I can catch the exact time. However, if we requery the API the position in the list of arrivals that I'm interested in can change. It's more cognitive load for the user to re-determine which arrival in the list they are interested in, vs. knowing the content will be the same and just needing to listen for clarification. Also, in our skill if they want fresh info its very easy to just open the skill again, so I think we should have different behavior for repeat. Plus, there is an increased chance of failure requerying.

[philipmw]

@barbeau, I added a commit to your branch that fixes up the unit test. The problem was that the second time you called into the intent, you passed a fresh session, so no previous activity was captured. See what you think.

[coveralls]

Changes Unknown when pulling de986ad10bf1c9b2b2acb951c620621364a59b85 on repeat into \ on master**.

[barbeau]

@philipmw To make it clearer what I'm aiming for, I reverted your commit (so we still have it if needed) and then change the test to the way I think it should be. It currently fails, but it shouldn't.

Note - I did notice an error in my previous test logic, but I think it was the opposite of your comment - I was re-using a previous session for the repeat intent, when it actually should have been a new session.

I think the current problem is in mocking the obaDao - it's not pulling the repeat intent text from the persisted data to populate the session when onIntent() is called with the repeat intent.

[philipmw]

You're right, my initial assessment of the problem was wrong. I have time today to dive into it.

[barbeau]

Awesome, thanks! I've been meaning to read up more on JMockit but last few weeks have been nuts.

Also, any idea why Coveralls stopped reporting code coverage? On Apr 12, 2016 8:52 PM, "Philip M. White" notifications@github.com wrote:

You're right, my initial assessment of the problem was wrong. I have time today to dive into it.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/OneBusAway/onebusaway-alexa/pull/51#issuecomment-209169347

[coveralls]

Changes Unknown when pulling bfab6ea86381033ab70063ecdb6370c8d8bca436 on repeat into \ on master**.

[philipmw]

I did another update... but ignore it for now. How do you TEST the repeat action? I cannot find a way. I believe the Repeat functionality works ONLY in the same session -- only when Alexa asks you a question, waits for a response, and reuses the session ID for your reply. If so, we never need to persist the string in the database... that's what my current update does.

[barbeau]

I'm pretty sure the Repeat intent is the same as the Help intent - Amazon only takes care of maintaining the utterance mappings, but we are responsible for implementing the actual functionality. So, saying "Alexa, ask OneBusAway to repeat" triggers the Repeat intent, and, because this is a new session, we have to grab the text to repeat from the persisted data. This is what my implementation did.

[philipmw]

I see. Ugh, ok. I'll look into it again. Sorry for all this back and forth. Does Amazon require us to have the repeat functionality to launch the app?

[barbeau]

They wanted either repeat intent supported or a period following every arrival to introduce a longer pause. I really didn't want to do the latter because the pause gets annoying and slows down the entire interaction.

Another option is just to merge my original implementation of repeat intent in this PR (minus the test), ship it, and add the test in a new PR.

[philipmw]

That works for me. If the skill gets wildly popular, we'll want to move response caching away from DynamoDB, because otherwise we'll have to provision write capacity (expensive) linearly with the popularity of our skill. But we can cross that bridge when we get to it.

[barbeau]

Sounds good. I'll force push my original commit minus the test in a bit, and you can review/merge that, and we'll move the test to another issue/PR.

[barbeau]

Done - new commit force pushed that contains only the repeat intent implementation, no unit test.

[barbeau]

After this is merged I'll open a new PR rebased on master that has the unit test (previous branch we were working with).

[coveralls]

Changes Unknown when pulling 999ba20d478eb0098366f3cc1bf9a3cd3a19b840 on repeat into \ on master**.