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

MicrophoneException, API Authentication Exception, Flac library #64

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Jerry0022
Copy link

Hi,

  • Added a "MicrophoneException" in Microphone.java which is thrown for example in case you don't have a micro connected.
  • Added a callback for API authentication in GSpeechDuplex and other failures on the HTTP connection to Google.
  • Added also the flac library to classpath just to get it working (if you don't like to publish it, just remove it please).

I just tested it without micro and without api key - it worked.

(The first commit was only playing around with your library therefore I undid most of it, except the addition of the flac lib)

Best Regards
Jeremy Treder

@Skylion007
Copy link
Collaborator

Look good for the most part.

The one issue I have is that your exceptions actually provide less useful information than the original exception. I would much rather have the exception typed with custom input. Also,I question why you need to create new exceptions in the first place when you can simply throw an Unavailable exception in the first place.

While this pull request does have a lot of benefits, I really do not like how you handle exceptions with this pull request.

Instead, you should probably at the very least wrap the other type of Exception within the Microphone Exception so we can see whether it was a targetdataline or illegal argument exception.

Secondly, I would strongly suggest that you utilize one of the already implemented HTTP exceptions instead of throwing your own. For instance, within the code we have access to the response code from the server and could at the very least have the appropriate error number thrown so the user. I would suggest extending Java's builtin HTTP Exception

Alternatively, we can throw an authentication error if we receive the 500+ error code on the upstream.

Fix the exceptions, and I'll be happy to merge and thank you so much for contributing to this repo.

@Skylion007
Copy link
Collaborator

To clarify the actual HTTP response code error is thrown with the openHttpsConnection(url) method.

@RahulChandrashekar17061984

Initially i had issues while connecting --- thought it is a proxy issue .... but later i think the URL is always returns a response code 400

https://www.google.com/speech-api/full-duplex/v1/down?maxresults=1&pair=819091834851160

Please find my comments on the exception in the below link

https://www.youtube.com/watch?v=HzpY-uKF4d0

@Skylion007
Copy link
Collaborator

Firstly, this is a place to discuss pull requests, not issues you are having with the code. The server will always return 400 if you try to access it via a browser or other means so that does not necessarily rule out that it is a proxy issue. Try testing it without a proxy and see if you can replicate the error.

@RahulChandrashekar17061984

Hi I still have the same issue

It would be great if we had a short telecom / webEx

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.

3 participants