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

[rest] VoiceResource: Return answer from /interpreters endpoint #4434

Merged
merged 1 commit into from
Nov 2, 2024

Conversation

florian-h05
Copy link
Contributor

Also add annotations for the response.

@florian-h05 florian-h05 requested a review from a team as a code owner October 30, 2024 17:22
@florian-h05
Copy link
Contributor Author

As the current answer was undocumented, WDYT about changing the format to JSON and putting the current answer string into a answer key?
My plans for the UI need extended core APIs ...

@wborn
Copy link
Member

wborn commented Nov 2, 2024

WDYT about changing the format to JSON and putting the current answer string into a answer key?

I wouldn't change it now unless there is a good reason and it is actually used for that. Seems like this API has been around since OH 2.x so changing it will most likely break some user integrations.

Comment on lines +194 to +195
String answer = hli.interpret(locale, text);
return Response.ok(answer, MediaType.TEXT_PLAIN).build();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did it always return null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What to you mean?

@florian-h05
Copy link
Contributor Author

My problem is, that I not only need to return an answer from the HLI but also some „action“ data to instruct the UI to show a widget for some Item, but that would also need changes to the HLI interface. Not sure how to handle that.

@wborn
Copy link
Member

wborn commented Nov 2, 2024

It might be possible to support both if you add another method with the @Consumes(MediaType.APPLICATION_JSON) annotation and keep the method having @Consumes(MediaType.TEXT_PLAIN). 😉

@florian-h05
Copy link
Contributor Author

Thanks for the hint — I will try that out then, but let’s get this merged first.

Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@wborn wborn merged commit 9646607 into openhab:main Nov 2, 2024
5 checks passed
@wborn wborn added this to the 4.3 milestone Nov 2, 2024
@florian-h05 florian-h05 deleted the rest-voice-interpret branch November 2, 2024 10:10
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 this pull request may close these issues.

2 participants