-
-
Couldn't load subscription status.
- 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
Open
snfeld
wants to merge
1
commit into
MyRobotLab:develop
Choose a base branch
from
snfeld:getText
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
added getText #1440
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
So maybe the last 2 are the 'designed' way to handle this?