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

Update gather-rcml documentation #2392

Merged
merged 4 commits into from
Jul 31, 2017
Merged

Update gather-rcml documentation #2392

merged 4 commits into from
Jul 31, 2017

Conversation

YevgenL
Copy link

@YevgenL YevgenL commented Jul 25, 2017

No description provided.

@maria-farooq maria-farooq self-assigned this Jul 31, 2017

public AsrSignal(String driver, String lang, List<URI> initialPrompts, String endInputKey, long maximumRecTimer, long waitingInputTimer,
long timeAfterSpeech, String hotWords) {
long timeAfterSpeech, String hotWords, String input, int numberOfDigits) {

Choose a reason for hiding this comment

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

imo this constructor deserves to have a java doc, with a brief description of these parameters.. you can take from specification document.

Copy link
Author

Choose a reason for hiding this comment

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

Added JavaDoc

try {
asrr = new String(Hex.decodeHex(asrr.toCharArray()));
} catch (DecoderException e) {
logger.error("asrr parameter cannot be decoded");

Choose a reason for hiding this comment

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

can we add atleast code in this error msg
or if it would make sense then print complete paramters

observer.tell(result, self());
}
} else {
logger.error("asrr parameter is missing");

Choose a reason for hiding this comment

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

can we add atleast code in this error msg
or if it would make sense then print complete paramters

Copy link
Author

Choose a reason for hiding this comment

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

added stacktrace output

@@ -218,8 +218,8 @@ protected void collect(final Object message) {
this.lastEvent = AUMgcpEvent.aupc;
} else {
this.lastEvent = AsrSignal.REQUEST_ASR;
signal = new AsrSignal(request.getDriver(), request.lang(), request.prompts(), request.endInputKey(), request.timeout(), request.timeout(),
request.timeout(), request.getHints());
signal = new AsrSignal(request.getDriver(), request.lang(), request.prompts(), request.endInputKey(), 60, request.timeout(),

Choose a reason for hiding this comment

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

is there a reason why 60 is hardcoded here?

Copy link
Author

Choose a reason for hiding this comment

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

that is my bad. I added it for test purposes and forgot to revert

@@ -57,6 +60,9 @@ public AsrSignal(String driver, String lang, List<URI> initialPrompts, String en
this.timeAfterSpeech = timeAfterSpeech;
this.hotWords = hotWords;
this.lang = lang;
this.input = input;
this.minNumber = numberOfDigits;
this.maxNumber = numberOfDigits;

Choose a reason for hiding this comment

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

can we add a code comment here why we are assigning both members same incoming value

Copy link
Author

Choose a reason for hiding this comment

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

RMS expects two parameters but there is the only one input 'Number of Digits' in collect in RVD for DTMF mode.

Choose a reason for hiding this comment

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

thanks @YevgenL as i suggested in my initial comment, can we add this explanation as comment so its helpful for code reader?

Copy link
Author

Choose a reason for hiding this comment

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

comment added

@@ -57,6 +60,9 @@ public AsrSignal(String driver, String lang, List<URI> initialPrompts, String en
this.timeAfterSpeech = timeAfterSpeech;
this.hotWords = hotWords;
this.lang = lang;
this.input = input;
this.minNumber = numberOfDigits;
this.maxNumber = numberOfDigits;

Choose a reason for hiding this comment

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

thanks @YevgenL as i suggested in my initial comment, can we add this explanation as comment so its helpful for code reader?

try {
asrr = new String(Hex.decodeHex(asrr.toCharArray()));
} catch (DecoderException e) {
logger.error("asrr parameter cannot be decoded: " + e.getStackTrace());

Choose a reason for hiding this comment

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

to print the stack trace of an exception using Java and Log4J we have to pass exception object as argument.
so statement would be:
logger.error("asrr parameter cannot be decoded.", e);

Copy link
Author

Choose a reason for hiding this comment

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

Changed

@maria-farooq maria-farooq merged commit 33795a4 into RestComm:asr Jul 31, 2017
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