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

DialogFlow - Suggested update to detect_intent_stream.php sample script. Avoiding Exception when using OutputAudioConfig() #911

Open
00j opened this issue Jun 7, 2019 · 1 comment
Labels
samples Issues that are directly related to samples. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@00j
Copy link

00j commented Jun 7, 2019

Hello, Please can I suggest a change?

In this example script:

https://github.com/GoogleCloudPlatform/php-docs-samples/blob/master/dialogflow/src/detect_intent_stream.php

we have the following extract:

    foreach ($stream->closeWriteAndReadAll() as $response) {
        $recognitionResult = $response->getRecognitionResult();
        if ($recognitionResult) {
            $transcript = $recognitionResult->getTranscript();
            printf('Intermediate transcript: %s' . PHP_EOL, $transcript);
        }
    }

    // get final response and relevant info
    if ($response) {
        print(str_repeat("=", 20) . PHP_EOL);
        $queryResult = $response->getQueryResult();
        $queryText = $queryResult->getQueryText();
        $intent = $queryResult->getIntent();
        $displayName = $intent->getDisplayName();
        $confidence = $queryResult->getIntentDetectionConfidence();
        $fulfilmentText = $queryResult->getFulfillmentText();
        // output relevant info
        printf('Query text: %s' . PHP_EOL, $queryText);
        printf('Detected intent: %s (confidence: %f)' . PHP_EOL, $displayName,
            $confidence);
        print(PHP_EOL);
        printf('Fulfilment text: %s' . PHP_EOL, $fulfilmentText);
    }

Which relies on using $response that is set from the last iteration of the loop and might not always be available.

Suggestion:

    $queryResult = null;
    foreach ($stream->closeWriteAndReadAll() as $response) {
        $recognitionResult = $response->getRecognitionResult();
        if ($recognitionResult) {
            $transcript = $recognitionResult->getTranscript();
            printf('Intermediate transcript: %s' . PHP_EOL, $transcript);
        }

        $queryResult = !$queryResult && $response->getQueryResult()
            ? $response->getQueryResult()
            : $queryResult;
    }

    // get final response and relevant info
    if ($queryResult) {
        print(str_repeat("=", 20) . PHP_EOL);
        $queryText = $queryResult->getQueryText();
        $intent = $queryResult->getIntent();
        $displayName = $intent->getDisplayName();
        $confidence = $queryResult->getIntentDetectionConfidence();
        $fulfilmentText = $queryResult->getFulfillmentText();
        // output relevant info
        printf('Query text: %s' . PHP_EOL, $queryText);
        printf('Detected intent: %s (confidence: %f)' . PHP_EOL, $displayName,
            $confidence);
        print(PHP_EOL);
        printf('Fulfilment text: %s' . PHP_EOL, $fulfilmentText);
    }

In this example setting $queryResult has been moved into the foreach ($stream->closeWriteAndReadAll() as $response) loop because getQueryResult() isn't always available on the final iteration of the loop.

For example when utilising OutputAudioConfig() it is set on the penultimate iteration. Which would throw an exception in the current example and result in the loss of results.

Related issue:
googleapis/google-cloud-php#2026

@00j 00j changed the title DialogFlow - Suggested update to detect_intent_stream.php example to avoid Exception when using OutputAudioConfig() DialogFlow - Suggested update to detect_intent_stream.php sample script. Avoiding Exception when using OutputAudioConfig() Jun 7, 2019
@bshaffer bshaffer self-assigned this Jun 13, 2019
@bshaffer
Copy link
Contributor

This looks good, although shouldn't it be this instead, since we want the final iteration of it?

$queryResult = $response->getQueryResult() ?: $queryResult

@JustinBeckwith JustinBeckwith added 🚨 This issue needs some love. triage me I really want to be triaged. labels Oct 3, 2019
@bshaffer bshaffer added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Oct 7, 2019
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants