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

Fix responses sequence for Alert and SubtleAlert #464

Conversation

AKalinich-Luxoft
Copy link
Contributor

@AKalinich-Luxoft AKalinich-Luxoft commented Nov 10, 2020

Fixes #465

This PR is ready for review.

Testing Plan

Covered by manual test plan

Summary

There was noticed an issue #3563 when SDL sends unexpected TTS.StopSpeaking request to HMI. This is a correct SDL behavior according to requirements for case when SDL receives response on UI part of Alert/SubtleAlert BEFORE receiving response on TTS.Speak part of request. HMI has been implemented in a way to send responses on UI part right away after receiving, so this sequence triggers SDL to stop TTS from it's side explicitly, however corresponding speak request was even not started yet.
To fix that sequence, added logic to postpone responses for UI.Alert and UI.SubtleAlert if TTS part is expected from SDL. In that case, if by some reason request is being rejected and TTS part is expected, HMI will register TTS.Speak listener and send response to UI part only after sending response to TTS part. In that case SDL will not send redundant TTS.StopSpeaking request to HMI.
Note TTS part of request for Alert/SubtleAlert can be predicted by HMI by alertType parameter of UI part. However, there is no way to predict the same for AlertManeuver and PerformAPT RPCs. Probably, this is a good candidate for a new proposal.

CLA

@AKalinich-Luxoft AKalinich-Luxoft force-pushed the fix/fix_subtle_alert_reject_sequence branch from b2f957f to c34cec9 Compare November 12, 2020 22:27
@AKalinich-Luxoft AKalinich-Luxoft force-pushed the fix/fix_subtle_alert_reject_sequence branch from c34cec9 to 69b119f Compare November 16, 2020 19:07
@jordynmackool
Copy link

@AKalinich-Luxoft I noticed there is an approval on the PR from the Luxoft team. Is this PR ready for Livio review?

@KhrystynaDubovyk
Copy link

@jordynmackool, currently the PR is not ready for Livio review.
We will let you know as soon as it is ready.

@AKalinich-Luxoft
Copy link
Contributor Author

@jordynmackool all conflicts have been resolved and PR is ready for Livio review

@Jack-Byrne
Copy link
Collaborator

@AKalinich-Luxoft Does this fix mean that smartdevicelink/sdl_core#3563 can be closed?

@AKalinich-Luxoft
Copy link
Contributor Author

@AKalinich-Luxoft Does this fix mean that smartdevicelink/sdl_core#3563 can be closed?

@JackLivio yes, SDL issue can be closed as its behavior is correct in this case

@Jack-Byrne Jack-Byrne merged commit 8e9640d into smartdevicelink:develop Jul 19, 2021
ValeriiMalkov pushed a commit to LuxoftSDL/sdl_hmi that referenced this pull request Jul 21, 2021
* Add TTS.Speak callbacks handlers and generic logic

* Added postponed callback for UI.Alert

* Added postponed callback for UI.SubtleAlert
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.

5 participants