-
-
Notifications
You must be signed in to change notification settings - Fork 117
added getText #1440
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
base: develop
Are you sure you want to change the base?
added getText #1440
Conversation
Useful if you store language dependet text in aiml files. This method gets the text for a given text-tag and does nothing else.
This is an addition to the chatbot. Only text is returned and the caller has to control speech. |
* @return language/bot dependent text | ||
*/ | ||
public String getText(String inText) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to get the current user/session and stuff here.. the getResponse method should handle that for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm quite new to the codebase. The getText is a copy of the getRespone with publishing removed. So I thought I need the same session handling as in getResponse. But for the current usage scenario it is very likely that a session already exists. Can I really reduce it to a simple getSession()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should probably start with, what's the reason for adding this method in the first place? ProgramAB.. I believe also handles a "publishText" method that you can subscribe to...
that being said.. your method could likely just be refactored to just have the following 2 lines... because the session management is handled in the getResponse(inText) method to choose the current user/bot.
Response resp = getResponse(inText);
return resp.msg;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is the right question. Its a bit longer answer.
The reason for this function is to NOT do the publishing in getResponse. Origin is an aiml pattern that calls a python function that does a bunch of movements and describing speech output. The text python wants to speak is language dependent or call it bot dependent. Now the idea was to have those texts in aiml too. So if you have an english bot you have the english text and the german bot has the german text. To have only different bots not an extra python script for each language, python uses kind of tag that matches for the correct text. If you now use getResponse() in python to get that text, you loose control and movements and speech are not synchronized. If you use speakBlocking(getText()) you are back in sync.
When I first saw this, I thought it is a misuse of the chatbot, but after thinking for a while, i didn't have a better solution. This keeps all the language dependency in one place with one scheme.
Hope this helps to understand the motivation. Maybe there is another solution in mrl that fits better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sorry for the slow response, but this is a slightly bigger topic. This is really a question of how localization works in MRL. There have been many approaches to doing this and support had been added. Normally for InMoov, it was handled with language specific AIML. It seems like you want to fetch localized responses from AIML calling into Python.
ProgramAB supports the use of "maps" for a bot. You could define all your localization in there and use the AIML map get to return the right response.
I understand that's not how you want it to work, and who knows it it would solve your problem or not...
I think we should rename this method to getResponseText ... (it's a bit more descriptive and explicit about that it's returning.) we don't want to duplicate the code for handling the picking of the current session .... if we want this to be a permanent feature in ProgramAB, we should add a unit test for it in ProgramABTest... and lastly, we should add some javadoc to the method so people know what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right this comes from Inmoov.
The flow is as follows in programAB you have a pattern that calls a python script. The python script then does multiple actions with describing text, to be specific: 'how many motors do you have' calls servos(). servos() uses speakBlocking() which is language dependent. The speech output must be synchronized to the movement.
I see three possible solutions to have sync and language:
- this getText() method so in the servos() script you now write speakBlocking(i01_chatBot.getText('TAG'))
- all speech to input parameters in servos ... now calling servos('text1','text2' ...)
- break into several methods, so servos() becomes servos1('text1'); servos2('text2')...
So maybe the last 2 are the 'designed' way to handle this?
.github/workflows/build.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your local build.yml file shouldn't be in the pull request...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed that.
Useful if you store language dependent text in aiml files. This method gets the text for a given text-tag and does nothing else. No publish.