OneBusAway / onebusaway-alexa

An Java-based app to communicate with Amazon Alexa for devices such as the Amazon Echo
Other
52 stars 18 forks source link

First functionality: configure and get arrivals #8

Closed philipmw closed 8 years ago

philipmw commented 8 years ago

Features:

  1. Configure city and stop ID using voice;
  2. Persist user's city and stop ID across sessions;
  3. Get list of arrivals!

Internals:

  1. Use Spring for externalized configuration and easier unit testing;
  2. Use AWS CloudFormation to provision the database and app's IAM user.
  3. Support finding the correct API endpoint based on user's city.

I updated the README with how to deploy this app properly.

philipmw commented 8 years ago

The specific scenario that I've tested:

  1. "open onebusaway"
  2. "I live in Seattle"
  3. "My stop is 26635" (That's 9th Ave N & Mercer St, S bound)

That concludes the setup. Then use it day-to-day:

  1. "open onebusaway"
barbeau commented 8 years ago

Awesome! Can't wait to try this out. On Feb 11, 2016 10:32 PM, "Philip M. White" notifications@github.com wrote:

The specific scenario that I've tested:

  1. "open onebusaway"
  2. "I live in Seattle"
  3. "My stop is 26635" (That's 9th Ave N & Mercer St, S bound)

That concludes the setup. Then use it day-to-day:

  1. "open onebusaway"

— Reply to this email directly or view it on GitHub https://github.com/OneBusAway/onebusaway-alexa/pull/8#issuecomment-183166652 .

philipmw commented 8 years ago

Updated based on feedback.

barbeau commented 8 years ago

Sorry, looks like my README edit introduced a conflict. Would you mind rebasing against master? Should be simple to resolve.

philipmw commented 8 years ago

Rebased.

barbeau commented 8 years ago

Hmmm...I'm getting the following when I invoke open onebusaway:

org/slf4j/LoggerFactory: java.lang.NoClassDefFoundError
java.lang.NoClassDefFoundError: org/slf4j/LoggerFactory
    at com.amazon.speech.speechlet.verifier.ApplicationIdSpeechletRequestVerifier.<clinit>(ApplicationIdSpeechletRequestVerifier.java:18)
    at com.amazon.speech.speechlet.lambda.SpeechletRequestStreamHandler.<init>(SpeechletRequestStreamHandler.java:56)
    at org.onebusaway.alexa.LambdaFunctionHandler.<init>(LambdaFunctionHandler.java:46)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:422)
Caused by: java.lang.ClassNotFoundException: org.slf4j.LoggerFactory
    at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    ... 7 more

I'm going to poke around more and see if I can figure out what's going on - let me know if you have any suggestions.

barbeau commented 8 years ago

Ah - looks like the following dependencies were pulled from pom.xml:

        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
            <version>1.7.13</version>
            <scope>compile</scope>
        </dependency>
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-simple</artifactId>
            <version>1.7.13</version>
            <scope>compile</scope>
        </dependency>

The above exist in master branch, but not in this PR.

barbeau commented 8 years ago

Yup, adding those dependencies did it! So cool to yell at Alexa and get bus times back :). :+1:

So I think we're ready to merge if you can update the PR based on the above comments.

An aside - for some reason it wouldn't recognize Tampa as an OBA region, although I did get it working with your Seattle example above - I need to look at this a bit more to troubleshoot that. If you get to it first let me know.

barbeau commented 8 years ago

One more item for future clean up (so I don't forget) - when it reads the stop ID back, it currently include the agency ID and the underscore prefix. For example - in Seattle, if the stop ID is 4, it has the form of "1_4".

philipmw commented 8 years ago

It didn't recognize Tampa because right now I have my own mapping (the YAML file) from city name to OBA region. I don't know of any other way to do it. And that file has only Seattle right now!

Is there a better way to find the correct OBA region?

philipmw commented 8 years ago

Re: slf4j, I removed it because removing it didn't break anything for me -- my JAR runs on Lambda just fine without those dependencies. I am mystified why it works for me but not you. I re-added those dependencies.

philipmw commented 8 years ago

And I addressed the other feedback (minus getting remote time) and "1_" issues.

barbeau commented 8 years ago

Is there a better way to find the correct OBA region?

Ah, I see. I hadn't delved into that code that deeply.

Yes, I think there is a better way - I'll go ahead and merge this PR, and then we can tackle that. Here's what I'm thinking:

  1. Get location of city (we're already doing this)
  2. Use OBA Regions API to get the closest region:
ObaRegionsResponse response = ObaRegionsRequest.newRequest().call();
ArrayList<ObaRegion> regions = new ArrayList<ObaRegion>(Arrays.asList(response.getRegions()));
ObaRegion r = RegionUtils.getClosestRegion(regions, location);
  1. Get and persist the base URL for the closest region:
String baseUrl = r.getObaBaseUrl();

Then just set the client with the persisted base URL when the user asks for arrival times. There needs to be some error handling here, but this would allow for an open set of cities, so we don't need to modify OBA Alexa when more cities are added to the Regions API.

barbeau commented 8 years ago

Thanks @philipmw, this is awesome!