Refactor volume management

Delete the duck channel
Workaround for Mycroft bad management of volume.

Signed-off-by: Gwendal Roulleau <[email protected]>

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

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";
 

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;
 }

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));

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
  */

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<OnOffType> {
 
-    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<MessageType> 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));
+                    }
                 }
             }
         }

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<State> {
 
+    /**
+     * 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<MessageType> 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());

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">
 
 	<name>Mycroft Binding</name>
-	<description>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.</description>
+	<description>
+		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.
+	</description>
 
 </binding:binding>

bundles/org.openhab.binding.mycroft/src/main/resources/OH-INF/thing/thing-types.xml

@@ -13,9 +13,8 @@
 			<channel id="speak" typeId="speak-channel"/>
 			<channel id="utterance" typeId="utterance-channel"/>
 			<channel id="player" typeId="system.media-control"/>
-			<!-- <channel id="volume" typeId="system.volume"/> -->
+			<!-- channel id="volume" typeId="system.volume"/> -->
 			<channel id="volume_mute" typeId="system.mute"/>
-			<channel id="volume_duck" typeId="volume-duck-channel"/>
 			<channel id="full_message" typeId="full-message-channel"/>
 		</channels>
 
@@ -30,6 +29,12 @@
 				<description>This is the port to connect to.</description>
 				<default>8181</default>
 			</parameter>
+			<parameter name="volume_restoration_level" type="integer" required="false" min="1" max="100">
+				<advanced>true</advanced>
+				<label>Volume Restoration Level</label>
+				<description>When unmuted, force Mycroft to restore volume to this value</description>
+			</parameter>
+
 		</config-description>
 
 	</thing-type>
@@ -65,10 +70,4 @@
 		</config-description>
 	</channel-type>
 
-	<channel-type id="volume-duck-channel">
-		<item-type>Switch</item-type>
-		<label>Volume Duck</label>
-		<description>Duck the volume of the Mycroft speaker</description>
-	</channel-type>
-
 </thing:thing-descriptions>