Fix response to SetGlobalProperties in case of UNSUPPORTED TTS #1574
Conversation
|
Please review @AKalinich-Luxoft @anosach-luxoft @VProdanov |
| @@ -298,6 +298,14 @@ bool SetGlobalPropertiesRequest::PrepareResponseParameters( | |||
| ui_response_info_); | |||
| return result; | |||
| } | |||
| if (ui_properties_info.is_ok && tts_properties_info.is_unsupported_resource) { | |||
There was a problem hiding this comment.
@pvvasilev I propose you to move all this if-s between 290-308 lines to overriden PrepareResultForMobileResponse/PrepareResultCodeForResponse functions as it's done in pull requests #1575, #1567 and #1562 for other RPCs with similar defect.
There was a problem hiding this comment.
@AKalinich-Luxoft Done in 07650f4
| return true; | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
@pvvasilev Maybe
if (ui_info.is_ok && tts_info.is_unsupported_resource) {
return true;
}
return CommandRequestImpl::PrepareResultForMobileResponse(ui_info, tts_info);
There was a problem hiding this comment.
@VProdanov No. The ui_info and tts_info are initialized in the CommandRequestImpl::PrepareResultForMobileResponse call.
| } | ||
| if (HmiInterfaces::STATE_AVAILABLE == tts_info.interface_state && | ||
| tts_info.is_unsupported_resource) { | ||
| tts_response_info_ = "Unsupported phoneme type sent in a prompt"; |
There was a problem hiding this comment.
@pvvasilev please remove this assignment. According to APPLINK-19591 SDL should transfer info string received from HMI response but not generate it by itself.
There was a problem hiding this comment.
@AKalinich-Luxoft I do not see any mention of this in the cited ticket.
There was a problem hiding this comment.
@pvvasilev it's not mentioned but it should. Please also look at related requirement APPLINK-31653 which contains more detailed description. I hope it's applicable for all RPCs which contains TTSChunk element as well.
There was a problem hiding this comment.
@AKalinich-Luxoft Again I do not see any mention of this. Please give me an exact requirement specifying this. Otherwise I do not agree with you. Also I think these requirements you are citing are not for the correct project.
There was a problem hiding this comment.
@pvvasilev these requirements are applicable for GENIVI & Ford-Specific (specified in Commitment field of ticket). In cited APPLINK-31653:
SDL must:
...
respond with "WARNINGS, success:true" + info: <message_from_HMI> to mobile app IN CASE
HMI respond with UNSUPPORTED_RESOURCE
I think it is exactly this case.
And requirement regarding info parameter: SDLOPEN-1309 req 4 and 5.
There was a problem hiding this comment.
@this is not the same case. The case you are referring to is the if above this one. There the info is forwarded.
If the response to `TTS` is `UNSUPPORTED_RESOURCE` and the response to `UI` is `SUCCESS`, SDL should respond with `success`:`true` and `resultCode`:`WARNINGS`. Added a check to verify this case.
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
For SetGlobalProperties RPC, which is split to
TTSandUIrequests,if the response to
TTSisUNSUPPORTED_RESOURCEand the responseto
UIisSUCCESS, SDL should respond withsuccess:trueandresultCode:WARNINGS. Added a check to verify this case.