i8beef / HomeAutio.Mqtt.GoogleHome

MIT License
217 stars 29 forks source link

ColorSetting trait, HSV colorModel #30

Closed shady2k closed 5 years ago

shady2k commented 5 years ago

In the template "action.devices.traits.ColorSetting" trait:

{
  "action.devices.commands.ColorAbsolute": {
    "color.spectrumHSV.hue": "MQTT_COMMAND_TOPIC",
    "color.spectrumHSV.saturation": "MQTT_COMMAND_TOPIC",
    "color.spectrumHSV.value": "MQTT_COMMAND_TOPIC"
  }
}

But from Google we received: image

Template needs to be like:

{
  "action.devices.commands.ColorAbsolute": {
    "color.spectrumHSV": {
      "hue": "MQTT_COMMAND_TOPIC",
      "saturation": "MQTT_COMMAND_TOPIC",
      "value": "MQTT_COMMAND_TOPIC"
    }
  }
}

It's right? If use template, MQTT commands does not sent, because condition does not true on line 162 MqttService.cs: if (deviceSupportedCommandParams.ContainsKey(parameter.Key))

Same problem with state in template:

{
  "color.spectrumHSV.hue": {
    "topic": "MQTT_COMMAND_TOPIC",
    "googleType": "numeric",
    "valueMap": null
  },
  "color.spectrumHSV.saturation": {
    "topic": "MQTT_COMMAND_TOPIC",
    "googleType": "numeric",
    "valueMap": null
  },
  "color.spectrumHSV.value": {
    "topic": "MQTT_COMMAND_TOPIC",
    "googleType": "numeric",
    "valueMap": null
  }
}

But it throw error in Device.cs on line 77:

                    // Parameter is a cmplex object
                    var complexParameterParts = stateParam.Key.Split('.');
                    if (complexParameterParts.Count() > 2)
                        throw new System.Exception("Status key contained more than one '.'");

Because complexParameterParts contains: | complexParameterParts | {string[3]} | string[]   | [0] | "color" | string   | [1] | "spectrumHSV" | string   | [2] | "hue" | string

i8beef commented 5 years ago

Nope the template is right actually. Command / state keys are FLATTENED into a '.' delimited string.

The complex parameter parsing needs to be changed though. Ill look into it.

shady2k commented 5 years ago

Still does not work.

2018-12-24 09:57:59.110 +03:00 [Error] Connection id ""0HLJ9DKGBMT9F"", Request id ""0HLJ9DKGBMT9F:00000001"": An unhandled exception was thrown by the application.
System.Exception: Status key contained more than one '.'
   at HomeAutio.Mqtt.GoogleHome.Models.State.Device.GetGoogleState(IDictionary`2 stateCache) in /app/HomeAutio.Mqtt.GoogleHome/Models/State/Device.cs:line 77
   at HomeAutio.Mqtt.GoogleHome.Controllers.GoogleHomeController.<HandleQueryIntent>b__8_4(Device x) in /app/HomeAutio.Mqtt.GoogleHome/Controllers/GoogleHomeController.cs:line 186
   at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer)
   at System.Linq.Enumerable.ToDictionary[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector)
   at HomeAutio.Mqtt.GoogleHome.Controllers.GoogleHomeController.HandleQueryIntent(QueryIntent intent) in /app/HomeAutio.Mqtt.GoogleHome/Controllers/GoogleHomeController.cs:line 181
   at HomeAutio.Mqtt.GoogleHome.Controllers.GoogleHomeController.Post(Request request) in /app/HomeAutio.Mqtt.GoogleHome/Controllers/GoogleHomeController.cs:line 74
   at lambda_method(Closure , Object , Object[] )
   at Microsoft.Extensions.Internal.ObjectMethodExecutor.Execute(Object target, Object[] parameters)
   at Microsoft.AspNetCore.Mvc.Internal.ActionMethodExecutor.SyncActionResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeActionMethodAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeNextActionFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Rethrow(ActionExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeInnerFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResourceFilter()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
   at Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
i8beef commented 5 years ago

I haven't pushed a new version yet.

shady2k commented 5 years ago

Ah, ok. I will wait, thank you.

i8beef commented 5 years ago

Ok, its up. It should now spit warnings in the log if it thinks that your googleDevices.json file is missing something... the INTERFACE will throw errors on it and now let you save something it thinks is invalid, but I wanted to allow you to do some manual config in the file to bypass that just in case for now.

See how that works with your stuff here. With the ColorAbsolute stuff the rules are a little convoluted, so I tried my best to capture Google's intent in the validation. Note that they want "spectrumHSV" in commands and "spectrumHsv" in the states... because apparently Google can't design an API to save their life...

shady2k commented 5 years ago

DockerHub has been already updated?

i8beef commented 5 years ago

Yeah, just went up.

shady2k commented 5 years ago

It works, but some errors in Home Graph with "willReportState": true.

Log:

2018-12-25 08:28:02.924 +03:00 [Information] Start processing HTTP request "POST" https://homegraph.googleapis.com/v1/devices:reportStateAndNotification
2018-12-25 08:28:02.924 +03:00 [Information] Sending HTTP request "POST" https://homegraph.googleapis.com/v1/devices:reportStateAndNotification
2018-12-25 08:28:03.396 +03:00 [Information] Received HTTP response after 471.9025ms - BadRequest
2018-12-25 08:28:03.397 +03:00 [Information] End processing HTTP request after 472.4448ms - BadRequest
2018-12-25 08:29:30.465 +03:00 [Warning] Google Home Graph update failed for devices: home/hall/strip

Response from google:

{
  "error": {
    "code": 400,
    "message": "Request contains an invalid argument.",
    "status": "INVALID_ARGUMENT"
  }
}

Request: {"requestId":"xxxxx","eventId":"xxxxx","agentUserId":"xxxx","followUpToken":null,"payload":{"devices":{"states":{"home/hall/strip":{"on":null,"brightness":null,"color":{"spectrumHsv":{"hue":120,"saturation":null,"value":null}}}},"notifications":null}}}

i8beef commented 5 years ago

Are your state topics retained? The amount of nulls in your example there make me think that your local state isn't completely initialized. I'd expect any of those values that are tied to a specific MQTT topic to be populated here.

shady2k commented 5 years ago

Turned on reatin for state topics. Now erros only at startup, before program get all mqtt messages.

i8beef commented 5 years ago

Ok. I think that makes sense. A message with only NULLs in a lot of those probably fails validation on their side. This app keeps and internal state of all of the state topics, and QUERY, and REPORTSTATE will assume that this state is complete. That assumes a "retain" on EVERY state topic so that new MQTT connections get a complete view of "current state" when it starts up subscribes to the requisite topics.

shady2k commented 5 years ago

You are right. Maybe program won't send state queries to Home Graph, until state cache does not have all items? Otherwise message will be invalid.

i8beef commented 5 years ago

As in short circuit a reportState altogether if ANY of the state properties aren't initialized yet? I could do that to at least stop them from going over during an initialization.

shady2k commented 5 years ago

It would be nice.

i8beef commented 5 years ago

It'll spit out a warning in the log around this if it happens, but it won't error out trying to send a message that's just going to fail due to missing state values.

shady2k commented 5 years ago

Yes, it's ok.

i8beef commented 5 years ago

Closing on that confirmation.