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

New asterisk commands #2

Merged
merged 1 commit into from
Dec 6, 2016
Merged

New asterisk commands #2

merged 1 commit into from
Dec 6, 2016

Conversation

alisinabh
Copy link
Contributor

@alisinabh alisinabh commented Dec 5, 2016

Asterisk AGI commands implemented:

  • CONTROL STREAM FILE
  • GET OPTION
  • GET DATA
  • RECORD FILE
  • WAIT FOR DIGIT
  • CHANNEL STATUS

@marcelog
Copy link
Owner

marcelog commented Dec 5, 2016

Hey :)

Thanks for the pull. I've made a few comments on the changes.

Also, could you please squash the commits into 1?

Thanks!

@alisinabh
Copy link
Contributor Author

Hi @marcelog :)
Thank you very much for your immediate response.

I've squashed commits into 1

but i cant see any comments on code?!

@@ -0,0 +1,5 @@
language: elixir
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't belong to this pull, please open a different pull request if needed.

@@ -1,4 +1,5 @@
# ElixirAgi
[![Build Status](https://travis-ci.org/alisinabh/elixir_agi.svg?branch=master)](https://travis-ci.org/alisinabh/elixir_agi)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't belong to this pull, please open a different pull request if needed.

Sending commands that are not yet implemeted in elixir_agi is pretty easy
you can just use this function with second arg as AST Command name like "STREAM" and third arg should be a list of options for command
"""
@spec custom_cmd(t, String.t, []) :: Result.t
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: What's the point of this new function if run/3 is already there?

@marcelog
Copy link
Owner

marcelog commented Dec 6, 2016

Sorry, I forgot to submit the changes. Can you see them now? Thanks!

@alisinabh
Copy link
Contributor Author

Yep, resolved!

sorry i thought run/3 is private :))

end

#TODO: Implement say_* AST commands
#TODO: Implement speeck AST commands
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot of other ones to implement, perhaps an open issue or pull request is better than these comments in the code

@marcelog
Copy link
Owner

marcelog commented Dec 6, 2016

Hi @alisinabh!

Thanks for the changes :) I've submitted one more, and when you're done, please squash everything into just 1 commit and I'll merge :)

Best!

AST cmd get_data implemented

AST cmd get_option implemented

AST cmd record_file implemented

fix record_file unsafe variable

custom_cmd fun in Agi Implemented

custom_cmd added in README

AST cmd wait_for_digit implemented

AST cmd channel_status implemented

fix argument naming issue in new AST commands

travis build badge

fix issues in review

fix readme

removed TODO lines
@alisinabh
Copy link
Contributor Author

TODO lines removed!
Squashed and ready B)

by the way, i don't know why are we squashing commits? i'm new to github pulls

@marcelog marcelog merged commit 2dba77d into marcelog:master Dec 6, 2016
@marcelog
Copy link
Owner

marcelog commented Dec 6, 2016

Thanks for the contribution @alisinabh!

P.S: Squashing is used in this case to make it look more clean.

Cheers!

@alisinabh
Copy link
Contributor Author

@marcelog thank you very much for your time!

This library is gonna be so useful to me. Looking forward to improve it. 👍

Great job!

@marcelog
Copy link
Owner

marcelog commented Dec 6, 2016

Great! Thanks for your words, looking forward to merge those improvements :)

All the best!

alisinabh added a commit to alisinabh/elixir_agi that referenced this pull request Dec 9, 2016
Merge pull request marcelog#2 from alisinabh/master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants