i8beef / HomeAutio.Mqtt.GoogleHome

MIT License
212 stars 29 forks source link

Commands without parameters #83

Closed profiluefter closed 4 years ago

profiluefter commented 4 years ago

How do commands without parameters like in the TransportControl Trait work? I looked at the HandleGoogleHomeCommand function and as far as I understood it, it would not work because messages are sent per parameter.

Really awesome project btw! I'm currently building a Google Home integration for my Hama Internet Radio that has an api but no integration. I also already had nginx and mosquitto set up so this really fits well for me. Thanks for making and maintaining this!

i8beef commented 4 years ago

They... don't really. I've played around with this a few times and not landed on a solution I really like yet. These are "commands" in a bit more traditional sense rather than "state changes" which is really what this is built around... There's not really a way for me to generically handle such commands...

The idea I've had so far that came closest was to allow for commands to be sent "as is" to a given MQTT topic as an alternative to the default state management approach. Something like in the config you'd do:

        "commands": {
          "action.devices.commands.TransportControl": {
            "_": "my/mqtt/command/topic"
          }

Then if it found any topic maps to "_" it will just JSON serialize the entire command param set (if there are any, otherwise just an empty JSON object), and send it to that MQTT topic...

Then if you are running node-red or similar, you can process that command yourself.

I had started down this path along with a half dozen other features that were causing a significant rearchitecture, and it got shelved with those changes, and since no one has been clamoring for them yet, its remained there. I'm still not completely sure how I'm going to tackle those, but I for sure won't be doing arbitrary command parsing here, I'd just be chasing behind Google as they add and change things constantly (this is already a painful experience that I've written scripts to help with to generate Device, Trait, and Commands by scraping their documentation pages as they change things without any sort of notification all the time).

So some sort of "delegate this by just republishing to MQTT" is in order.

i8beef commented 4 years ago

Ah, I remember why I backed off on this. Its actually what caused me to consider a wide rearchitecture. The problem here is as follows

  1. Google's API is sort of all over the place, which makes it difficult to actually implement 100% of the smart home footprint. A lot of implementations actually just say "Nah, we just aren't going to support everything"... but I'm stubborn.
  2. I can actually do the above part for accepting the command and delegating it to an external MQTT message handler quite easily... the PROBLEM becomes
  3. Google expects a response back with specific state information (i.e., the state changed to X due to this action). For most of the state-based command types, I fake this by just returning the parameter set back to it (since MQTT is by default ASYNC, and I won't have any modified state back yet when I need to return to Google). The problem with these command types is that they carry no information about what state they change, so I can't actually use that shortcut... I actually need a unique HANDLER for EVERY command that has intimate knowledge of how the command and its parameters are going to transform the state so I can still fake it... For things like NetworkControl there is a lot there.
  4. I MIGHT be able to accept the command, send it, and just return "success" to Google without fulfilling their request for updated state... but I don't actually know what it's gonna do. That's a significantly smaller change...

Let me play with that today and see if I can get a proof of concept together... Google just might complain about the fact that I return "non-standard" responses without all the information they expect, but TECHNICALLY report state should handle updating those ANYWAY when they change for the most part...

i8beef commented 4 years ago

Alright, I've added support for this that you can try... (should be 1.1.0.116 when its done building)

If you add a "_" like this

        "commands": {
          "action.devices.commands.TransportControl": {
            "_": "my/mqtt/command/topic"
          }

It will serialize the entire command parameter set, or send an empty payload (maybe that should be an empty {} so you can always assume JSON?) if it has no parameters. You can then handle that command however you want. I will return to Google a SUCCESS, but no state information and we'll see how pissed it gets. With the state of the rest of Google's API here I wouldn't be surprised if it doesn't do any validation anyway. :-)

If a command has MORE than "_" mapped it will continue and try to publish things for those mappings as well so you can mix and match a little, though frankly I question if that was wise vs just forcing you to choose EITHER full command delegation or state based direct MQTT publishes. My thought here is that doing this will allow some other options, for instance, if someone wants to delegate ALL command handling to something like node-red and bypass some of the things I have here like mapping, etc., they can do that, it just might piss Google off when they don't get properly formatted state back.

profiluefter commented 4 years ago

Hi, thanks for your response! I just got a prototype working with a few changes!

The only issue right now is that there are two text responses to my request on my phone and I don't know why. I'll open a pull request after I tested a few more things.

Edit: Misread your last comment; Your implementation (probably) works and looks better! I'll try it in a few minutes.

profiluefter commented 4 years ago

Just tried it and got a ArgumentNullException in Line 214 of MqttService.cs. execution.Params is null and not empty when there are no parameters. Don't know why though...

i8beef commented 4 years ago

That problem COULD come up in a few places... but I am pushing a fix for that SPECIFIC instance right now.

profiluefter commented 4 years ago

It works! (at least for my configuration) There are still two text responses and no voice response but I don't know why (no logs in GCE). It's not an issue for me though. Thanks btw for implementing this that quickly!

i8beef commented 4 years ago

Hmmm don't know on those two... it could be an artifact of me not returning states, but who knows what weirdness Google's side does here... I have confirmed I'm not returning multiple responses or anything like that that I can see.

Which command are you actually trying? Could just be something that Google hasn't fully implemented yet or something, or that requires a state response to complete, i.e., with "The media is paused" when I never returned a state param that actually SAYS it was paused, etc.

You happened to catch me on my week off.

profiluefter commented 4 years ago

I was trying it with action.devices.commands.mediaNext. I'll try a few others and edit this shortly.

Edit: I think I broke something after I changed the name and nicknames a bit. It's not recognizing the name as a device anymore but I'll just let it rest/sync for a while. Not related to this issue. Edit: The issue was that I somehow got a newline into the env variables

i8beef commented 4 years ago

Tip: You can "Hey Google, sync my devices" to get an immediate sync update.

profiluefter commented 4 years ago

I just finished the "Backend" implementation of my radio integration and it works!

Skipping and all the other parameterless commands are working fine on my Google Home. Specifically the parameterless commands still trigger two responses when using Google Assistant in text mode though. Not my use case so I don't know if I should close this...

One last issue that I'm having is that I can almost never trigger the action.devices.commands.SelectChannel command from the Channel trait. I think it's a localization issue on Google's side because I'm from Austria (Google Country is Germany though because not even half of the features are available in Austria) and language support even with the common commands is already poor. This command is another weird case though because the parameters vary based on how the command is triggered. I don't think this currently works with HomeAutio but I can't use it productively anyways.

Thanks again for making this software and I'll maybe look into it myself in the near future.

i8beef commented 4 years ago

Im not sure there's much else to do with the double response... you might have to play with that. This is already kind of a hack :-)

Open a new ticket on the Channel issue. I already know what it is. Just yet another Google-ism. It's sending action.devices.commands.selectChannel instead of the documented action.devices.commands.SelectChannel.

i8beef commented 4 years ago

Try the latest with the setChannel difference fixed.