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

ResetGlobalProperties doesn't reset HELPPROMPT and VRHELPITEMS to default values #2337

Merged
merged 1 commit into from May 11, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jul 11, 2018

Fixes #1306

This PR is ready for review.

Risk

This PR makes no API changes.

Summary

This PR provided bugfixing ResetGlobalProperties request from mobile, when the SDL after restart should send to HMI UI.SetGlobalProperties("vrHelpTitle", "vrHelp") with default values and TTS.SetGlobalProperties with helpPrompt parameter that should be reset to default value from smartDeviceLink.ini file.

CLA

Copy link
Contributor

@AKalinich-Luxoft AKalinich-Luxoft left a comment

Choose a reason for hiding this comment

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

Please check and fix your build with enabled unit tests. As I could see it is failing now

smart_objects::SmartObject so_help_prompt =
smart_objects::SmartObject(smart_objects::SmartType_Array);

for (uint32_t i = 0; i < help_prompt.size(); ++i) {
smart_objects::SmartObject helpPrompt =
Copy link
Contributor

Choose a reason for hiding this comment

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

@ValeriiMalkov please use our coding style naming. And better to rename this to help_prompt_item

Copy link
Author

@ghost ghost Jul 12, 2018

Choose a reason for hiding this comment

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

@AKalinich-Luxoft That was fixed at the commit f42464f

@@ -1291,9 +1291,16 @@ smart_objects::SmartObjectSPtr MessageHelper::CreateAppVrHelp(
return NULL;
}
smart_objects::SmartObject& vr_help = *result;
vr_help[strings::vr_help_title] = app->name();

vr_help[strings::vr_help_title] =
Copy link
Contributor

Choose a reason for hiding this comment

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

@ValeriiMalkov please add if condition and check:

  1. Returned from vr_help_title() pointer is not NULL
  2. strings::vr_help_title key exists in returned smart object

Copy link
Author

@ghost ghost Jul 12, 2018

Choose a reason for hiding this comment

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

@AKalinich-Luxoft That was fixed at the commit f42464f

int32_t index = 0;

smart_objects::SmartObject so_vr_help(smart_objects::SmartType_Map);
so_vr_help[strings::position] = index+1;
Copy link
Contributor

Choose a reason for hiding this comment

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

@ValeriiMalkov please run check_style.sh as I could see some style errors

Copy link
Author

@ghost ghost Jul 12, 2018

Choose a reason for hiding this comment

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

@AKalinich-Luxoft That was fixed at the commit f42464f

if (app->vr_help_title()) {
vr_help[strings::vr_help_title] =
(*app->vr_help_title())[strings::vr_help_title].asString();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@ValeriiMalkov

const smart_objects::SmartObject* vr_help_title = app->vr_help_title();
if (NULL != vr_help_title && vr_help_title->keyExists(strings::vr_help_title)) {
  vr_help[strings::vr_help_title] = (*vr_help_title)[strings::vr_help_title].asString();
}

Copy link
Author

Choose a reason for hiding this comment

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

@AKalinich-Luxoft That was fixed and force updated.
See commit f42464f

smart_objects::SmartObject so_help_prompt =
smart_objects::SmartObject(smart_objects::SmartType_Array);

for (uint32_t i = 0; i < help_prompt.size(); ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ValeriiMalkov What if size will return 64bit value?

Copy link
Author

@ghost ghost Jul 13, 2018

Choose a reason for hiding this comment

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

@AByzhynar That's really not correct. I changed it to size_t type which returns method 'size()' from 'std::vector' container. See commit: ad33e6e

@@ -1291,9 +1291,20 @@ smart_objects::SmartObjectSPtr MessageHelper::CreateAppVrHelp(
return NULL;
}
smart_objects::SmartObject& vr_help = *result;
vr_help[strings::vr_help_title] = app->name();
const smart_objects::SmartObject* vr_help_title = app->vr_help_title();
if (NULL != vr_help_title &&
Copy link
Contributor

Choose a reason for hiding this comment

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

@ValeriiMalkov use nullptr instead of NULL or even if(ptr)

Copy link
Author

@ghost ghost Jul 13, 2018

Choose a reason for hiding this comment

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

@AByzhynar That was fixed at the commit: ad33e6e

Fix bug `ResetGlobalProperties` does't reset `HELPPROMPT`
and `VRHELPITEMS` to default values

Fix UT's after bugfixing

 - Fix `ResetGlobalPropertiesRequestTest`
 - Add mock function into `MockApplicationManagerSettings`

Github issue #1306
@JenkinsSDLOnCloud
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

@EKuliiev EKuliiev left a comment

Choose a reason for hiding this comment

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

./test_scripts/Smoke/API/002_ResetGlobalProperties_PositiveCase_SUCCESS.lua failed.

--- Preconditions -----------------------------------------------------------------------------------
  commonSteps:DeletePolicyTable : policy.sqlite is not found  
[15:41:07,250] Clean_environment                                                                     [SUCCESS] (1 ms)
SDL pid 3615
tcp        0      0 127.0.0.1:8087          0.0.0.0:*               LISTEN     
HMI initialized
HMI is ready
Mobile connected
[15:41:08,864] Start_SDL__HMI__connect_Mobile__start_Session                                         [SUCCESS] (624 ms)
[15:41:09,505] RAI                                                                                   [SUCCESS] (218 ms)
[15:41:09,732] Activate_App                                                                          [SUCCESS] (511 ms)
--- Test --------------------------------------------------------------------------------------------
[15:41:10,246] ResetGlobalProperties_Positive_Case                                                   [FAIL] (77 ms)
 HMI call TTS.SetGlobalProperties: ValidIf: 
number of array elements params.helpPrompt: expected 0, actual: 2
 HMI call UI.SetGlobalProperties: ValidIf: 
params.vrHelpTitle: expected: Test Application, actual value: Available Vr Commands List
--- Postconditions ----------------------------------------------------------------------------------
[15:41:10,328] Stop_SDL                                                                              [SUCCESS] (4 ms)
Total executing time is 5s 163ms (summary 5163ms)
==============================
Finish './test_scripts/Smoke/API/002_ResetGlobalProperties_PositiveCase_SUCCESS.lua'

@@ -237,8 +250,16 @@ bool ResetGlobalPropertiesRequest::ResetVrHelpTitleItems(
SendResponse(false, mobile_apis::Result::APPLICATION_NOT_REGISTERED);
return false;
}

const std::string& vr_help_title =

Choose a reason for hiding this comment

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

These changes generate the following log:

"vrHelpTitle" : 
{
    "vrHelpTitle" : "Available Vr Commands List"
}

@iCollin iCollin changed the base branch from develop to fix/resetglobalprops_vrhelp May 11, 2020 14:45
@iCollin iCollin merged commit 000e14c into smartdevicelink:fix/resetglobalprops_vrhelp May 11, 2020
iCollin added a commit that referenced this pull request May 18, 2020
…ault values (#3357)

* Fix bug `ResetGlobalProperties` (#2337)

Fix bug `ResetGlobalProperties` does't reset `HELPPROMPT`
and `VRHELPITEMS` to default values

Fix UT's after bugfixing

 - Fix `ResetGlobalPropertiesRequestTest`
 - Add mock function into `MockApplicationManagerSettings`

Github issue #1306

Co-authored-by: v-malko4 <[email protected]>

* fix vr_help_title SO type, rename help_prompt_ in unit tests to so_help_prompt

* fix style

Co-authored-by: v-malko4 <[email protected]>
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.

8 participants