Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.

Conversation

@deekshithreddyr
Copy link

No description provided.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@Override
protected void onPreExecute() {
super.onPreExecute();
showProgressDialog();
Copy link
Contributor

Choose a reason for hiding this comment

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

With this line gone, you can remove the onPreExecute() method override now that we just use the default.

Copy link
Author

Choose a reason for hiding this comment

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

Updated

- [Create a Service account](https://cloud.google.com/iam/docs/creating-managing-service-accounts) with the following IAM role: `Dialogflow API Client`. Example name: `dialogflow-client`. ([For more info on: how to add roles to a Service Account](https://cloud.google.com/iam/docs/granting-roles-to-service-accounts#granting_access_to_a_service_account_for_a_resource))
- Under Dialogflow-client, Click on edit permission icon on the right and add another role as follows and save the changes:
- Dialogflow Client
- Dialogflow Reader
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooo let's add a small note that sorta explains why we chose each Role.

- Dialogflow Client (Used by the app to make detect intent requests)
- Dialogflow Reader (Used by the app to list knowledge bases)

Copy link
Author

Choose a reason for hiding this comment

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

Updated

@nnegrey
Copy link
Contributor

nnegrey commented Aug 19, 2019

Oh I forgot about this one, we need to add the LICENSE to the top of the java files.

/*
 * Copyright 2019 Google LLC
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 * http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

*/
private void showProgressDialog() {
AlertDialog.Builder builder = new AlertDialog.Builder(this);
builder.setMessage("Please wait...");
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: "Please wait..." --> "Fetching auth token..."

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Updated

@nnegrey
Copy link
Contributor

nnegrey commented Aug 28, 2019

I think my last 2 big things are:

  1. If you can easily switch over to the streaming audio call for Dialogflow so as the user is speaking the audio is sent.
  2. Take the really long lines and just make them multi line for code readability.
    Example:
    From 223 columns.
    private DetectIntentRequest getDetectIntentRequest(SessionName sessionName, QueryInput queryInput,boolean tts, boolean sentiment, boolean knowledge, FixedCredentialsProvider fixedCredentialsProvider) throws Exception {

To something under 100 columns.

    private DetectIntentRequest getDetectIntentRequest(
            SessionName sessionName, QueryInput queryInput,boolean tts, boolean sentiment,
            boolean knowledge, FixedCredentialsProvider fixedCredentialsProvider) throws Exception {

@bdannoville
Copy link

bdannoville commented Aug 29, 2019

Hi there,
please note that at lign 50 of the readme, the link to the index.js containing nodeJS cloud functions for Dialogflow is broken. This file doesn't exist in the targeted github project: GoogleCloudPlatform/nodejs-docs-samples.

@nnegrey
Copy link
Contributor

nnegrey commented Aug 29, 2019

public static void playAudio(byte[] byteArray) {
MediaPlayer mediaPlayer = new MediaPlayer();
try {
File tempFile = File.createTempFile("dialogFlow", null, Environment.getExternalStorageDirectory());
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename temp file to speech2speech or something similar.

}

class Holder {
TextView text1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can text1 be a bit more descriptive?
Not sure of its purpose

getNewToken();
}
} else {
Toast.makeText(this, "Please enter or say some message to send.", Toast.LENGTH_SHORT).show();
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Let's switch this to just Please enter a message

I tried with the voice and not saying anything and the audio already has a way to detect if nothing is said and they only press the send button when typing, where the audio is automatically sent.

setContentView(R.layout.activity_settings);

if (AppController.PROJECT_ID.equals("GCP_PROJECT_ID")) {
Toast.makeText(this, "Please update the GCP_PROJECT_ID in strings.xml", Toast.LENGTH_LONG).show();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to have this toast happen but not shut down the app?

I forgot to update the strings.xml and took me a few openings to notice the toast message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Side question that I don't know if it is possible, but can you retrieve the project id from the google-services.json file?
IF so, that would be neat.

@@ -0,0 +1,13 @@
*.iml
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a README for speech to speech

protected void onPostExecute(List<Language> response) {
super.onPostExecute(response);
if (TextUtils.isEmpty(sourceLanguageCode)) {
sourceLanguageList.addAll(response);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is me not having set up the correct permissions yet, but I got an error and crash.

java.lang.NullPointerException: Attempt to invoke interface method 'java.lang.Object[] java.util.Collection.toArray()' on a null object reference
        at java.util.ArrayList.addAll(ArrayList.java:588)
        at com.example.translate.ui.SettingsActivity$LanguageList.onPostExecute(SettingsActivity.java:304)
        at com.example.translate.ui.SettingsActivity$LanguageList.onPostExecute(SettingsActivity.java:283)

Map<String, String> data = new HashMap<>();
data.put("deviceID", firebaseInstanceId);

FirebaseFunctions.getInstance()
Copy link
Contributor

Choose a reason for hiding this comment

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

This was referenced Sep 16, 2019
@nnegrey
Copy link
Contributor

nnegrey commented Sep 16, 2019

Samples moved into multiple PRs:
#112
#113

Closing.

@nnegrey nnegrey closed this Sep 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants