Hypfer / Valetudo

Cloud replacement for vacuum robots enabling local-only operation
https://valetudo.cloud
Apache License 2.0
6.72k stars 397 forks source link

GoToLocationCapabilityMqttHandle wants JSON, documentation says "send a string"; results in unhandledRejection SyntaxError: Unexpected token a in JSON error #960

Closed kquinsland closed 3 years ago

kquinsland commented 3 years ago

Describe the bug

When attempting to send my vac to a specific location, I observe an error:

[2021-05-31T17:50:06.157Z] [ERROR] unhandledRejection SyntaxError: Unexpected token a in JSON at position 1
    at JSON.parse (<anonymous>)
    at PropertyMqttHandle.setter (/snapshot/Valetudo/backend/lib/mqtt/capabilities/GoToLocationCapabilityMqttHandle.js:49:37)
    at PropertyMqttHandle.set (/snapshot/Valetudo/backend/lib/mqtt/handles/PropertyMqttHandle.js:105:27)
    at PropertyMqttHandle.setHomie (/snapshot/Valetudo/backend/lib/mqtt/handles/MqttHandle.js:298:20)
    at Object.topics.<computed> [as valetudo/madison/GoToLocationCapability/go/set] (/snapshot/Valetudo/backend/lib/mqtt/handles/MqttHandle.js:82:28)
    at MqttClient.<anonymous> (/snapshot/Valetudo/backend/lib/mqtt/MqttController.js:288:42)
    at MqttClient.emit (node:events:365:28)
    at MqttClient._handlePublish (/snapshot/Valetudo/node_modules/mqtt/lib/client.js:1277:12)
    at MqttClient._handlePacket (/snapshot/Valetudo/node_modules/mqtt/lib/client.js:410:12)
    at work (/snapshot/Valetudo/node_modules/mqtt/lib/client.js:321:12)

To Reproduce

This is the action that my Home Assistant is taking:

service: mqtt.publish
data:
  topic: valetudo/vac/GoToLocationCapability/go/set
  payload: "3ac1b81a-DEAD-BEEF-0123-45cff8d6897d"

The docs say: Data type: string but the code tries to parse JSON:

https://github.com/Hypfer/Valetudo/blob/master/backend/lib/mqtt/capabilities/GoToLocationCapabilityMqttHandle.js#L49

Screenshots

Vacuum Model

This is a Roborock S6.

Valetudo Version

2021.05.0 / 6b35e4d96bfc9f55b7bd8d51172d450a805e8351

Expected behavior

I would expect that the device navigate to the destination.

Additional context

A string is invalid JSON, so this is why the exception is thrown. The closest thing to a string that is also valid JSON would be an array with a single string in it:

["3ac1b81a-DEAD-BEEF-0123-45cff8d6897d"]

When i send that ^ payload, the command silently fails (yes, even with TRACE level logging). If the docs are updated to indicate that the payload should be an array with a single string, then the code can be adjusted:

                    const id = JSON.parse(value)[0];

And that would set id to the string. Optionally, the code could be written such that the JSON.parse() call is removed.

Hypfer commented 3 years ago

A string is invalid JSON, so this is why the exception is thrown. The closest thing to a string that is also valid JSON would be an array with a single string in it

Actually no. Having just a single string is according to spec, as long as it is wrapped in quotes.

Relevant quote:

A string is a sequence of zero or more Unicode characters, wrapped in double quotes, using backslash escapes.

> JSON.parse("\"foo\"")
'foo'
> JSON.parse("\"\"")
''
> JSON.parse("foo")
Uncaught SyntaxError: Unexpected token o in JSON at position 1

But yeah you're absolutely right that this is ugly and unexpected. This most likely happend because the code for zone preset handling was simply copy-pasted over

kquinsland commented 3 years ago

Having just a single string is according to spec, as long as it is wrapped in quotes.

TIL! Thanks for the tip. For anybody else that googles their way here, the fix is simple:

service: mqtt.publish
data:
  topic: valetudo/vac/GoToLocationCapability/go/set
  payload: "\"3ac1....97d\""