From 684149f5a760c7cdf2cc06cbc6fd35614a062b36 Mon Sep 17 00:00:00 2001 From: Gwendal Roulleau Date: Fri, 31 Dec 2021 01:34:36 +0100 Subject: [PATCH] Refactor volume management Delete the duck channel Workaround for Mycroft bad management of volume. Signed-off-by: Gwendal Roulleau --- bundles/org.openhab.binding.mycroft/README.md | 14 ++-- .../internal/MycroftBindingConstants.java | 1 - .../internal/MycroftConfiguration.java | 1 + .../mycroft/internal/MycroftHandler.java | 4 +- .../internal/api/dto/MessageVolumeSet.java | 6 +- .../internal/channels/DuckChannel.java | 68 ------------------- .../internal/channels/MuteChannel.java | 49 +++++++++++-- .../internal/channels/VolumeChannel.java | 41 ++++++----- .../main/resources/OH-INF/binding/binding.xml | 10 ++- .../resources/OH-INF/thing/thing-types.xml | 15 ++-- 10 files changed, 94 insertions(+), 115 deletions(-) delete mode 100644 bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/DuckChannel.java diff --git a/bundles/org.openhab.binding.mycroft/README.md b/bundles/org.openhab.binding.mycroft/README.md index 73ccbcbeb4ec4..62c4dfc5987d7 100644 --- a/bundles/org.openhab.binding.mycroft/README.md +++ b/bundles/org.openhab.binding.mycroft/README.md @@ -8,7 +8,7 @@ Possibilies include: - Simulate a voice command to launch a skill, as if you just spoke it - Send some text that Mycroft will say (Using its Text To Speech service) - Control the music player -- Mute or duck the sound volume of Mycroft +- Mute the sound volume of Mycroft - React to all the aforementioned events ... - ... and send/receive any other kind of messages on the message bus @@ -37,10 +37,11 @@ The default port is 8181, which can be changed. Thing mycroft:mycroft:myMycroft "Mycroft A.I." @ "Living Room" [host="192.168.X.X"] ``` -| property | type | description | mandatory | -|---------------|---------------------------------|-------------------------------------------------------------------------|-----------| -| host | IP or string | IP address or hostname | Yes | -| port | integer | Port to reach Mycroft (default 8181) | No | +| property | type | description | mandatory | +|--------------------------|------------------------|------------------------------------------------------------------|-----------| +| host | IP or string | IP address or hostname | Yes | +| port | integer | Port to reach Mycroft (default 8181) | No | +| volume_restoration_level | integer | When unmuted, force Mycroft to restore volume to this value | No | ## Channels @@ -55,7 +56,6 @@ A Mycroft thing has the following channels: | utterance | String | The last utterance Mycroft receive | | player | Player | The music player Mycroft is currently controlling | | volume_mute | Switch | Mute the Mycroft speaker | -| volume_duck | Switch | Duck the volume of the Mycroft speaker | | full_message | String | The last message (full json) seen on the Mycroft Bus. Filtered by the messageTypes properties | @@ -85,7 +85,6 @@ The `mycroft.item` file: ```java Switch myMycroft_mute "Mute" { channel="mycroft:mycroft:myMycroft:volume_mute" } -Switch myMycroft_duck "Duck" { channel="mycroft:mycroft:myMycroft:volume_duck" } Player myMycroft_player "Control" { channel="mycroft:mycroft:myMycroft:player" } Switch myMycroft_listen "Wake and listen" { channel="mycroft:mycroft:myMycroft:listen" } String myMycroft_speak "Speak STT" { channel="mycroft:mycroft:myMycroft:speak" } @@ -102,7 +101,6 @@ sitemap demo label="myMycroft" { Frame label="myMycroft" { Switch item=myMycroft_mute - Switch item=myMycroft_duck Default item=myMycroft_player Switch item=myMycroft_listen Text item=myMycroft_speak diff --git a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftBindingConstants.java b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftBindingConstants.java index 289fd297050d9..b35bfe3ea6370 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftBindingConstants.java +++ b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftBindingConstants.java @@ -35,7 +35,6 @@ public class MycroftBindingConstants { public static final String PLAYER_CHANNEL = "player"; public static final String VOLUME_CHANNEL = "volume"; public static final String VOLUME_MUTE_CHANNEL = "volume_mute"; - public static final String VOLUME_DUCK_CHANNEL = "volume_duck"; public static final String UTTERANCE_CHANNEL = "utterance"; public static final String FULL_MESSAGE_CHANNEL = "full_message"; diff --git a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftConfiguration.java b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftConfiguration.java index 02fa90c00265a..d80273e22e4b2 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftConfiguration.java +++ b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftConfiguration.java @@ -24,4 +24,5 @@ public class MycroftConfiguration { public String host = ""; public int port = 8181; + public int volume_restoration_level = 0; } diff --git a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftHandler.java b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftHandler.java index 85b347801829f..9b7566dfe4c70 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftHandler.java +++ b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/MycroftHandler.java @@ -29,7 +29,6 @@ import org.openhab.binding.mycroft.internal.api.dto.MessageVolumeGet; import org.openhab.binding.mycroft.internal.channels.AudioPlayerChannel; import org.openhab.binding.mycroft.internal.channels.ChannelCommandHandler; -import org.openhab.binding.mycroft.internal.channels.DuckChannel; import org.openhab.binding.mycroft.internal.channels.FullMessageChannel; import org.openhab.binding.mycroft.internal.channels.ListenChannel; import org.openhab.binding.mycroft.internal.channels.MuteChannel; @@ -150,8 +149,7 @@ public void initialize() { registerChannel(new ListenChannel(this)); registerChannel(new VolumeChannel(this)); - registerChannel(new MuteChannel(this)); - registerChannel(new DuckChannel(this)); + registerChannel(new MuteChannel(this, config.volume_restoration_level)); registerChannel(new SpeakChannel(this)); registerChannel(new AudioPlayerChannel(this)); registerChannel(new UtteranceChannel(this)); diff --git a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/api/dto/MessageVolumeSet.java b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/api/dto/MessageVolumeSet.java index ea7b885990ade..541cfa858817c 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/api/dto/MessageVolumeSet.java +++ b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/api/dto/MessageVolumeSet.java @@ -15,8 +15,10 @@ import org.openhab.binding.mycroft.internal.api.MessageType; /** - * This message asks Mycroft to set the volume to an amount - * specified in the data payload + * This message asks IN THEORY Mycroft to set the volume to an amount + * specified in the data payload. + * But it seems in fact to be a message to inform third party of a + * volume change * * @author Gwendal Roulleau - Initial contribution */ diff --git a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/DuckChannel.java b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/DuckChannel.java deleted file mode 100644 index 8893e2850e615..0000000000000 --- a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/DuckChannel.java +++ /dev/null @@ -1,68 +0,0 @@ -/** - * Copyright (c) 2010-2021 Contributors to the openHAB project - * - * See the NOTICE file(s) distributed with this work for additional - * information. - * - * This program and the accompanying materials are made available under the - * terms of the Eclipse Public License 2.0 which is available at - * http://www.eclipse.org/legal/epl-2.0 - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.openhab.binding.mycroft.internal.channels; - -import java.util.Arrays; -import java.util.List; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.openhab.binding.mycroft.internal.MycroftBindingConstants; -import org.openhab.binding.mycroft.internal.MycroftHandler; -import org.openhab.binding.mycroft.internal.api.MessageType; -import org.openhab.binding.mycroft.internal.api.dto.BaseMessage; -import org.openhab.binding.mycroft.internal.api.dto.MessageVolumeDuck; -import org.openhab.binding.mycroft.internal.api.dto.MessageVolumeUnduck; -import org.openhab.core.library.types.OnOffType; -import org.openhab.core.types.Command; - -/** - * The channel responsible for triggering STT recognition - * - * @author Gwendal Roulleau - Initial contribution - */ -@NonNullByDefault -public class DuckChannel extends MycroftChannel { - - public DuckChannel(MycroftHandler handler) { - super(handler, MycroftBindingConstants.VOLUME_DUCK_CHANNEL); - } - - @Override - public List getMessageToListenTo() { - return Arrays.asList(MessageType.mycroft_volume_duck, MessageType.mycroft_volume_unduck); - } - - @Override - public void messageReceived(BaseMessage message) { - if (message.type == MessageType.mycroft_volume_duck) { - updateMyState(OnOffType.ON); - } else if (message.type == MessageType.mycroft_volume_unduck) { - updateMyState(OnOffType.OFF); - } - } - - @Override - public void handleCommand(Command command) { - if (command instanceof OnOffType) { - if (command == OnOffType.ON) { - if (handler.sendMessage(new MessageVolumeDuck())) { - updateMyState(OnOffType.ON); - } - } else if (command == OnOffType.OFF) { - if (handler.sendMessage(new MessageVolumeUnduck())) { - updateMyState(OnOffType.OFF); - } - } - } - } -} diff --git a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/MuteChannel.java b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/MuteChannel.java index 55e3ee578fe73..f34a0aca8effa 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/MuteChannel.java +++ b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/MuteChannel.java @@ -21,6 +21,7 @@ import org.openhab.binding.mycroft.internal.api.MessageType; import org.openhab.binding.mycroft.internal.api.dto.BaseMessage; import org.openhab.binding.mycroft.internal.api.dto.MessageVolumeMute; +import org.openhab.binding.mycroft.internal.api.dto.MessageVolumeSet; import org.openhab.binding.mycroft.internal.api.dto.MessageVolumeUnmute; import org.openhab.core.library.types.OnOffType; import org.openhab.core.types.Command; @@ -33,24 +34,49 @@ @NonNullByDefault public class MuteChannel extends MycroftChannel { - public MuteChannel(MycroftHandler handler) { + private int volumeRestorationLevel; + + public MuteChannel(MycroftHandler handler, int volumeRestorationLevel) { super(handler, MycroftBindingConstants.VOLUME_MUTE_CHANNEL); + this.volumeRestorationLevel = volumeRestorationLevel; } @Override public List getMessageToListenTo() { - return Arrays.asList(MessageType.mycroft_volume_mute, MessageType.mycroft_volume_unmute); + // we don't listen to mute/unmute message because duck/unduck seems sufficient + // and we don't want to change state twice for the same event + // but it should be tested on mark I, as volume is handled differently + return Arrays.asList(MessageType.mycroft_volume_duck, MessageType.mycroft_volume_unduck, + MessageType.mycroft_volume_set, MessageType.mycroft_volume_increase); } @Override public void messageReceived(BaseMessage message) { - if (message.type == MessageType.mycroft_volume_mute) { - updateMyState(OnOffType.ON); - } else if (message.type == MessageType.mycroft_volume_unmute) { - updateMyState(OnOffType.OFF); + switch (message.type) { + case mycroft_volume_mute: + case mycroft_volume_duck: + updateMyState(OnOffType.ON); + break; + case mycroft_volume_unmute: + case mycroft_volume_unduck: + case mycroft_volume_increase: + updateMyState(OnOffType.OFF); + break; + case mycroft_volume_set: + if (((MessageVolumeSet) message).data.percent > 0) { + updateMyState(OnOffType.OFF); + } + break; + default: } } + private boolean sendVolumeSetMessage(float volume) { + String messageToSend = VolumeChannel.VOLUME_SETTER_MESSAGE.replaceAll("\\$\\$VOLUME", + Float.valueOf(volume).toString()); + return handler.sendMessage(messageToSend); + } + @Override public void handleCommand(Command command) { if (command instanceof OnOffType) { @@ -61,6 +87,17 @@ public void handleCommand(Command command) { } else if (command == OnOffType.OFF) { if (handler.sendMessage(new MessageVolumeUnmute())) { updateMyState(OnOffType.OFF); + // if configured, we can restore the volume to a fixed amount + // usefull as a workaround for the broken Mycroft volume behavior + if (volumeRestorationLevel > 0) { + // we must wait 100ms for Mycroft to handle the message and + // setting old volume before forcing to our value + try { + Thread.sleep(100); + } catch (InterruptedException e) { + } + sendVolumeSetMessage(Float.valueOf(volumeRestorationLevel)); + } } } } diff --git a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/VolumeChannel.java b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/VolumeChannel.java index 4e5de23e769df..efcf2cb11e62d 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/VolumeChannel.java +++ b/bundles/org.openhab.binding.mycroft/src/main/java/org/openhab/binding/mycroft/internal/channels/VolumeChannel.java @@ -34,14 +34,21 @@ /** * The channel responsible for handling the volume of the Mycroft speaker - * NOT FUNCTIONAL - * (see https://community.mycroft.ai/t/openhab-plugin-development-audio-volume-message-types-missing/10576) + * QUITE FUNCTIONAL but with workaround + * (see https://community.mycroft.ai/t/openhab-plugin-development-audio-volume-message-types-missing/10576 + * and https://github.com/MycroftAI/skill-volume/issues/53) * * @author Gwendal Roulleau - Initial contribution */ @NonNullByDefault public class VolumeChannel extends MycroftChannel { + /** + * As the MessageVolumeSet is, contrary to the documentation, not listened to by Mycroft, + * we use a workaround and send a message simulating an intent detection + */ + public static final String VOLUME_SETTER_MESSAGE = "{\"type\": \"mycroft-volume.mycroftai:SetVolume\", \"data\": {\"intent_type\": \"mycroft-volume.mycroftai:SetVolume\", \"mycroft_volume_mycroftaiVolume\": \"volume\", \"mycroft_volume_mycroftaiLevel\": \"$$VOLUME\", \"mycroft_volume_mycroftaiTo\": \"to\", \"target\": null, \"confidence\": 0.6000000000000001, \"__tags__\": [{\"match\": \"volume\", \"key\": \"volume\", \"start_token\": 1, \"entities\": [{\"key\": \"volume\", \"match\": \"volume\", \"data\": [[\"volume\", \"mycroft_volume_mycroftaiVolume\"]], \"confidence\": 1.0}], \"end_token\": 1, \"from_context\": false}, {\"match\": \"$$VOLUME\", \"key\": \"$$VOLUME\", \"start_token\": 3, \"entities\": [{\"key\": \"$$VOLUME\", \"match\": \"$$VOLUME\", \"data\": [[\"$$VOLUME\", \"mycroft_volume_mycroftaiLevel\"]], \"confidence\": 1.0}], \"end_token\": 3, \"from_context\": false}, {\"match\": \"to\", \"key\": \"to\", \"start_token\": 2, \"entities\": [{\"key\": \"to\", \"match\": \"to\", \"data\": [[\"to\", \"mycroft_volume_mycroftaiTo\"]], \"confidence\": 1.0}], \"end_token\": 2, \"from_context\": false}], \"utterance\": \"set volume to $$VOLUME\", \"utterances\": [\"set volume to X\"]}, \"context\": {\"client_name\": \"mycroft_cli\", \"source\": [\"skills\"], \"destination\": \"debug_cli\"}}"; + private PercentType lastVolume = new PercentType(50); private PercentType lastNonZeroVolume = new PercentType(50); @@ -51,9 +58,12 @@ public VolumeChannel(MycroftHandler handler) { @Override public List getMessageToListenTo() { + // we don't listen to mute/unmute message because duck/unduck seems sufficient + // and we don't want to change state twice for the same event + // but it should be tested on mark I, as volume is handled differently return Arrays.asList(MessageType.mycroft_volume_get_response, MessageType.mycroft_volume_set, - MessageType.mycroft_volume_mute, MessageType.mycroft_volume_unmute, MessageType.mycroft_volume_increase, - MessageType.mycroft_volume_decrease); + MessageType.mycroft_volume_increase, MessageType.mycroft_volume_decrease, + MessageType.mycroft_volume_duck, MessageType.mycroft_volume_unduck); } @Override @@ -65,9 +75,9 @@ public void messageReceived(BaseMessage message) { } else if (message.type == MessageType.mycroft_volume_set) { float volumeSet = ((MessageVolumeSet) message).data.percent; updateAndSaveMyState(normalizeVolume(volumeSet)); - } else if (message.type == MessageType.mycroft_volume_mute) { + } else if (message.type == MessageType.mycroft_volume_duck) { updateAndSaveMyState(new PercentType(0)); - } else if (message.type == MessageType.mycroft_volume_unmute) { + } else if (message.type == MessageType.mycroft_volume_unduck) { updateAndSaveMyState(lastNonZeroVolume); } else if (message.type == MessageType.mycroft_volume_increase) { updateAndSaveMyState(normalizeVolume(lastVolume.intValue() + 10)); @@ -122,27 +132,28 @@ private PercentType normalizeVolume(float volume) { } public float toMycroftVolume(PercentType percentType) { - return Float.valueOf(percentType.intValue() / 100f); + return Float.valueOf(percentType.intValue()); } public PercentType computeNewVolume(int valueAdded) { return new PercentType(lastVolume.intValue() + valueAdded); } + private boolean sendSetMessage(float volume) { + String messageToSend = VOLUME_SETTER_MESSAGE.replaceAll("\\$\\$VOLUME", Float.valueOf(volume).toString()); + return handler.sendMessage(messageToSend); + } + @Override public void handleCommand(Command command) { if (command instanceof OnOffType) { if (command == OnOffType.ON) { - MessageVolumeSet messageVolumeSet = new MessageVolumeSet(); - messageVolumeSet.data.percent = toMycroftVolume(lastNonZeroVolume); - if (handler.sendMessage(messageVolumeSet)) { + if (sendSetMessage(toMycroftVolume(lastNonZeroVolume))) { updateAndSaveMyState(lastNonZeroVolume); } } if (command == OnOffType.OFF) { - MessageVolumeSet messageVolumeSet = new MessageVolumeSet(); - messageVolumeSet.data.percent = 0; - if (handler.sendMessage(messageVolumeSet)) { + if (sendSetMessage(0)) { updateAndSaveMyState(PercentType.ZERO); } } @@ -157,9 +168,7 @@ public void handleCommand(Command command) { updateAndSaveMyState(computeNewVolume(-10)); } } else if (command instanceof PercentType) { - MessageVolumeSet messageVolumeSet = new MessageVolumeSet(); - messageVolumeSet.data.percent = toMycroftVolume((PercentType) command); - handler.sendMessage(messageVolumeSet); + sendSetMessage(toMycroftVolume((PercentType) command)); updateAndSaveMyState((PercentType) command); } else if (command instanceof RefreshType) { handler.sendMessage(new MessageVolumeGet()); diff --git a/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/binding/binding.xml b/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/binding/binding.xml index 41ed59d5e1e8b..03d6cb09a67c5 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/binding/binding.xml +++ b/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/binding/binding.xml @@ -4,8 +4,12 @@ xsi:schemaLocation="https://openhab.org/schemas/binding/v1.0.0 https://openhab.org/schemas/binding-1.0.0.xsd"> Mycroft Binding - Connects to a Mycroft instance in order to receive information from, and send commands to it. - Typical usage includes triggering Mycroft to listen (as if a wake word was detected), sending text for Mycroft to - speak, reacting on some specific intent, command skills by faking a spoken utterance, etc. + + Connects to a Mycroft instance in order to receive information from, and send commands to it. Typical + usage includes + triggering Mycroft to listen (as if a wake word was detected), sending text for Mycroft to speak, + reacting on some + specific intent, command skills by faking a spoken utterance, etc. + diff --git a/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/thing/thing-types.xml b/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/thing/thing-types.xml index bd913506c1c2e..26e384f46647c 100644 --- a/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/thing/thing-types.xml +++ b/bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/thing/thing-types.xml @@ -13,9 +13,8 @@ - + - @@ -30,6 +29,12 @@ This is the port to connect to. 8181 + + true + + When unmuted, force Mycroft to restore volume to this value + + @@ -65,10 +70,4 @@ - - Switch - - Duck the volume of the Mycroft speaker - -