[voice] Update dialog processing #2693

lolodomo · 2022-01-16T21:45:30Z

Related to #2688

Updated methods startDialog
New method stopDialog
Null annotations added to the class DialogProcessor

Allow translation of sentences "said" by the dialog processor in case of error

2 console commands added to start and stop a dialog

Enhanced integration tests

Signed-off-by: Laurent Garnier lg.hc@free.fr

lolodomo · 2022-01-22T12:40:22Z

@cweitkamp @wborn @kaikreuzer : this PR is now ready for review.

lolodomo · 2022-01-22T14:03:23Z

I have to mention that it is a API breaking change as the public VoiceManager interface is enhanced with a new method.

lolodomo · 2022-01-28T07:39:35Z

@openhab/core-maintainers : I would appreciate if this could be included in 3.3M1. This is important as first keyword spotting and Speech-to-Text addons will be included.

kaikreuzer · 2022-01-29T13:53:27Z

Hey @lolodomo, many thanks for your work on this.
Unfortunately, I wasn't available this week and will also have limited time the coming days. I'll happily review it, but I'm afraid, I cannot go through it in time for 3.3.0.M1 (which would mean it needs to be merged today).

I have to mention that it is a API breaking change as the public VoiceManager interface is enhanced with a new method.

As it is only adding a method (and new runtime exceptions to existing methods), this is not breaking for API consumers, which is fine. This interface is not meant to be implemented by anybody else, it's only meant to be used.

Related to openhab#2688 Updated methods startDialog New method stopDialog Null annotations added to the class DialogProcessor Allow translation of sentences "said" by the dialog processor in case of error 2 console commands added to start and stop a dialog Enhanced integration tests Signed-off-by: Laurent Garnier <lg.hc@free.fr>

kaikreuzer

As we have slightly postponed M1, I just found the time for the review, @lolodomo!
Just a few very minor remarks that should be quick and easy to address (and which could also be done as a follow-up PR in case you cannot address them within the next 2 hours).

kaikreuzer · 2022-01-30T14:51:05Z

...es/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java

+    private void closeStreamKS() {
+        AudioStream stream = streamKS;
+        if (stream != null) {
+            // Due to the current implementation of JavaSoundAudioSource, the stream is not closed as it would


Could you elaborate a bit (in the comment) on what the exact problem is?

This is explained in issue #2702

Could you add a link to this issue in the code then, please?
It looks as this should/could be easily fixed as well, right?

I will, but I can't do it immediately. (not at home)

Yes a fix is apparently easy. The idea was to discuss the issue with the author code. Maybe you are the author? I don't remember and I have not checked.

kaikreuzer · 2022-01-30T14:51:21Z

...es/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java

+        if (stream != null) {
+            // Due to the current implementation of JavaSoundAudioSource, the stream is not closed as it would
+            // lead to problem
+            // try {


I'd prefer to then remove the commented code here.

You mean we keep the method the comment but not the commented code. Ok with that.

kaikreuzer · 2022-01-30T14:51:34Z

...es/org.openhab.core.voice/src/main/java/org/openhab/core/voice/internal/DialogProcessor.java

+    private void closeStreamSTT() {
+        AudioStream stream = streamSTT;
+        if (stream != null) {
+            // Due to the current implementation of JavaSoundAudioSource, the stream is not closed as it would


kaikreuzer · 2022-01-30T14:55:44Z

...b.core.voice/src/main/java/org/openhab/core/voice/internal/VoiceConsoleCommandExtension.java

@@ -65,7 +76,11 @@ public VoiceConsoleCommandExtension(final @Reference VoiceManager voiceManager,
    public List<String> getUsages() {
        return List.of(buildCommandUsage(SUBCMD_SAY + " <text>", "speaks a text"),
                buildCommandUsage(SUBCMD_INTERPRET + " <command>", "interprets a human language command"),
-                buildCommandUsage(SUBCMD_VOICES, "lists available voices of the TTS services"));
+                buildCommandUsage(SUBCMD_VOICES, "lists available voices of the TTS services"),
+                buildCommandUsage(SUBCMD_START_DIALOG + " [<source> <interpreter> <sink> <keyword>]",


It looks as if you allow a flexible number of parameters in the implementation, so the usage might better be something like

[<source> [<interpreter> [<sink> [<keyword>]]]]

Yes you're right.

kaikreuzer

@lolodomo Ok, I merge it now so that it is in 3.3 M1 and you can do a follow-up PR.

lolodomo · 2022-01-30T17:34:45Z

Thank you Kai

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Related to openhab#2688 Updated methods startDialog New method stopDialog Null annotations added to the class DialogProcessor Allow translation of sentences "said" by the dialog processor in case of error 2 console commands added to start and stop a dialog Enhanced integration tests Signed-off-by: Laurent Garnier <lg.hc@free.fr> GitOrigin-RevId: 8f0dd94

…nhab#2729) Signed-off-by: Laurent Garnier <lg.hc@free.fr> GitOrigin-RevId: 3e94dd6

lolodomo requested a review from a team as a code owner January 16, 2022 21:45

lolodomo force-pushed the voice_dialog branch from ee5c58c to 9317b44 Compare January 17, 2022 00:06

lolodomo changed the title ~~[WIP][voice] Update the handling of a dialog~~ [WIP][voice] Update dialog processing Jan 17, 2022

lolodomo force-pushed the voice_dialog branch 20 times, most recently from 9b5e80c to f9d3fed Compare January 22, 2022 12:33

lolodomo changed the title ~~[WIP][voice] Update dialog processing~~ [voice] Update dialog processing Jan 22, 2022

GiviMAD mentioned this pull request Jan 22, 2022

[audio] JavaSoundAudioSource returning closed AudioStream #2702

Closed

cweitkamp added the enhancement An enhancement or new feature of the Core label Jan 24, 2022

GiviMAD mentioned this pull request Jan 29, 2022

[watsonstt] initial contribution openhab/openhab-addons#12161

Merged

lolodomo force-pushed the voice_dialog branch from f9d3fed to e46df3c Compare January 30, 2022 11:42

kaikreuzer requested changes Jan 30, 2022

View reviewed changes

kaikreuzer approved these changes Jan 30, 2022

View reviewed changes

kaikreuzer merged commit 8f0dd94 into openhab:main Jan 30, 2022

kaikreuzer added this to the 3.3 milestone Jan 30, 2022

lolodomo deleted the voice_dialog branch January 30, 2022 17:35

lolodomo added a commit to lolodomo/openhab-core that referenced this pull request Feb 3, 2022

[voice] Minor changes requested during review of PR openhab#2693

83489f9

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

kaikreuzer pushed a commit that referenced this pull request Feb 3, 2022

[voice] Minor changes requested during review of PR #2693 (#2729)

3e94dd6

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 12, 2023

[voice] Minor changes requested during review of PR openhab#2693 (ope…

11a1fdc

…nhab#2729) Signed-off-by: Laurent Garnier <lg.hc@free.fr> GitOrigin-RevId: 3e94dd6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[voice] Update dialog processing #2693

[voice] Update dialog processing #2693

lolodomo commented Jan 16, 2022 •

edited

Loading

lolodomo commented Jan 22, 2022

lolodomo commented Jan 22, 2022

lolodomo commented Jan 28, 2022

kaikreuzer commented Jan 29, 2022

kaikreuzer left a comment

kaikreuzer Jan 30, 2022

lolodomo Jan 30, 2022

kaikreuzer Jan 30, 2022

lolodomo Jan 30, 2022

kaikreuzer Jan 30, 2022

lolodomo Jan 30, 2022

kaikreuzer Jan 30, 2022

kaikreuzer Jan 30, 2022

lolodomo Jan 30, 2022

kaikreuzer left a comment

lolodomo commented Jan 30, 2022

[voice] Update dialog processing #2693

[voice] Update dialog processing #2693

Conversation

lolodomo commented Jan 16, 2022 • edited Loading

lolodomo commented Jan 22, 2022

lolodomo commented Jan 22, 2022

lolodomo commented Jan 28, 2022

kaikreuzer commented Jan 29, 2022

kaikreuzer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaikreuzer left a comment

Choose a reason for hiding this comment

lolodomo commented Jan 30, 2022

lolodomo commented Jan 16, 2022 •

edited

Loading