Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mqtt handleAction and handleWriteProperty do not work as expected #979

Closed
hasanheroglu opened this issue May 8, 2023 · 3 comments · Fixed by #987
Closed

Mqtt handleAction and handleWriteProperty do not work as expected #979

hasanheroglu opened this issue May 8, 2023 · 3 comments · Fixed by #987

Comments

@hasanheroglu
Copy link
Contributor

hasanheroglu commented May 8, 2023

https://github.com/eclipse/thingweb.node-wot/blob/09dfd6ab03805b7b39ab39433880496466d25e67/packages/binding-mqtt/src/mqtt-broker-server.ts#L269

https://github.com/eclipse/thingweb.node-wot/blob/09dfd6ab03805b7b39ab39433880496466d25e67/packages/binding-mqtt/src/mqtt-broker-server.ts#L303

The lines above send just a JSON object or string (in case of a parsing error for handleAction) to handleInvokeAction and handleWriteProperty function, where functions need a Content object instead. And with these non-Content values InteractionOutputs are created, after we make a call for InteractionOutputs value error occurs because InteractionOutputs content is not actually a content but JSON object or string.

I have seen that for other protocol bindings using forms' contentType, Content object is created:
https://github.com/eclipse/thingweb.node-wot/blob/09dfd6ab03805b7b39ab39433880496466d25e67/packages/binding-coap/src/coap-server.ts#L456-L460

Should we also apply same kind of logic here or should we follow some other method?

@hasanheroglu hasanheroglu changed the title Mqtt handleAction and handleWriteProperty does not work as expected Mqtt handleAction and handleWriteProperty do not work as expected May 8, 2023
@danielpeintner
Copy link
Member

I did not look into it more closely but it seems to be a left over from "very old" times.

As mentioned in another issue, I think we should come up with tests that actually "check" interactions with values in MQTT to be sure that everything works as expected.
A PR with a fix and tests is appreciated 👍

@egekorkan
Copy link
Member

For clarification: The JSON.parse way is predating all the other packages that use Content. MQTT package should do the same.

@hasanheroglu
Copy link
Contributor Author

Working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants