Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial version of astronomy agent #198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

initial version of astronomy agent #198

wants to merge 1 commit into from

Conversation

qedi-r
Copy link

@qedi-r qedi-r commented Mar 31, 2014

Agent that pulls the Weather Underground API to look for sunrise/sunset/moonrise/moonset and create events based on those times.

@cantino
Copy link
Member

cantino commented Apr 1, 2014

Nice @qedi-r!

Two things:

  1. can you add a spec?
  2. can you extract the Weather Underground behavior from this and the WeatherAgent into a shared Concern like this one: https://github.com/cantino/huginn/blob/master/app/concerns/twitter_concern.rb (this has a nice benefit of allowing support for credentials)

@qedi-r
Copy link
Author

qedi-r commented Apr 1, 2014

Yes, I was going down that road a bit, but it's easier said than done for using said concern in WeatherAgent.

I'm not sure how we can optionally include the weather underground concern or a pending forecastio concern without including both. This would run into issues if we wanted to, say, validate that the api key worked in the concern by testing the service. If we blindly include both concerns, one validation would fail necessarily.

Any thoughts on how this would be done for the current version of the Weather Agent?

@cantino
Copy link
Member

cantino commented Apr 1, 2014

Yea, maybe since Wunderground already has a good gem, it's not worth it. You could modify WeatherAgent as follows:

    def api_key
      if options['api_key'].present? && options['api_key'] != "your-key"
        options['api_key']
      else
        credential("#{service}_api_key")
      end
    end

    def key_setup?
      api_key.present?
    end

    ....

    def wunderground
      Wunderground.new(api_key).forecast_for(location)['forecast']['simpleforecast']['forecastday'] if key_setup?
    end

    def forecastio
      if key_setup?
        ForecastIO.api_key = api_key
        lat, lng = location.split(',')
        ForecastIO.forecast(lat,lng)['daily']['data']
      end
    end

and then you could just repeat the api_key and wunderground methods in your agent as well, so that you get the same behavior. These two could also be pulled into a Concern, but it might not be worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants