Fix applying heart_beat_timeout_ms from PreDataConsent #1551

VVeremjova · 2017-05-18T06:37:31Z

After sending SDL.OnAllowSDLFunctionality(allowed = false) application is assigned to PreDataConsent group.
After this HB timeout is set to timeout from PreDataConsent

VVeremjova · 2017-05-18T06:37:36Z

@okozlovlux, @LevchenkoS, @LitvinenkoIra please review

LuxoftAKutsan · 2017-05-18T06:58:17Z

src/components/application_manager/src/policies/policy_handler.cc

+void PolicyHandler::SetHeartBeatTimeout(const std::string& policy_app_id,
+                                        uint32_t app_id) {
+  LOG4CXX_AUTO_TRACE(logger_);
+  int timeout = policy_manager_->HeartBeatTimeout(policy_app_id);


@VVeremjova const and probably uint32_t

@LuxoftAKutsan See dcf7dda

LuxoftAKutsan · 2017-05-18T06:59:10Z

src/components/policy/policy_external/src/cache_manager.cc

  CACHE_MANAGER_CHECK(0);
  uint32_t result = 0;
  if (!IsApplicationRepresented(app_id)) {
+    LOG4CXX_DEBUG(logger_, "App is not represented");


@VVeremjova please print there app_id. And probably it is warning? No?

@LuxoftAKutsan See dcf7dda

LuxoftAKutsan · 2017-05-18T07:00:24Z

src/components/policy/policy_external/src/cache_manager.cc

@@ -415,6 +418,15 @@ void CacheManager::ProcessUpdate(
  const std::string& app_id = initial_policy_iter->first;
  RequestTypes merged_pt_request_types;

+  if (app_id == kPreDataConsentId) {


@VVeremjova invert comparator please :

kPreDataConsentId == app_id

@LuxoftAKutsan See dcf7dda

LuxoftAKutsan · 2017-05-18T07:01:24Z

src/components/policy/policy_external/src/cache_manager.cc

@@ -415,6 +418,15 @@ void CacheManager::ProcessUpdate(
  const std::string& app_id = initial_policy_iter->first;
  RequestTypes merged_pt_request_types;

+  if (app_id == kPreDataConsentId) {
+    *(pt_->policy_table.app_policies_section.apps[app_id]


@VVeremjova please extract long string to local variable to better read

@LuxoftAKutsan See dcf7dda

LuxoftAKutsan · 2017-05-18T07:02:17Z

src/components/application_manager/src/policies/policy_handler.cc

+    application_manager_.connection_handler().SetHeartBeatTimeout(app_id,
+                                                                  timeout);
+  } else {
+    LOG4CXX_INFO(logger_, "SetHeartBeatTimeout for " << app_id << "  ignored");


@VVeremjova is it info?

@LuxoftAKutsan See dcf7dda

LuxoftAKutsan · 2017-05-18T07:02:30Z

src/components/application_manager/include/application_manager/policies/policy_handler.h

@@ -479,6 +479,8 @@ class PolicyHandler : public PolicyHandlerInterface,
   */
  void LinkAppsToDevice();

+  void SetHeartBeatTimeout(const std::string& policy_app_id, uint32_t app_id);


@VVeremjova uint32_t app_id can be const?

@LuxoftAKutsan See dcf7dda

LuxoftAKutsan · 2017-05-18T07:02:59Z

@VVeremjova please add @dcherniev @indlu to review

ATymchenko

@VVeremjova, reviewed

dcherniev

@VVeremjova Reviewed.

JenkinsSDLOnCloud · 2017-05-19T07:47:14Z

ATF failed http://opensdl-jenkins.luxoft.com:8080/job/OpenSDL_Develop_PR/job/OpenSDL_develop_ATF/316/

JenkinsSDLOnCloud · 2017-10-07T08:36:50Z

Can one of the admins verify this patch?

JenkinsSDLOnCloud · 2017-10-10T16:09:45Z

Can one of the admins verify this patch?

JenkinsSDLOnCloud · 2017-10-13T10:16:50Z

Can one of the admins verify this patch?

After sending SDL.OnAllowSDLFunctionality(allowed = false) application is assigned to PreDataConsent group. After this HB timeout is set to timeout from PreDataConsent

iCollin

the LOG4CXX calls will need to be updated to the new abstract log macros as well

iCollin · 2020-11-16T22:25:29Z

src/components/policy/policy_external/src/cache_manager.cc

@@ -2154,6 +2167,10 @@ bool policy::CacheManager::SetIsPredata(const std::string& app_id) {
  if (IsApplicationRepresented(app_id)) {
    pt_->policy_table.app_policies_section.apps[app_id].set_to_string(
        kPreDataConsentId);
+
+    pt_->policy_table.app_policies_section.apps[app_id].heart_beat_timeout_ms =


am I misunderstanding that we just set the entry to a string, not an object?

Is this not what the policy entry would look like?
pt_->policy_table.app_policies_section.apps[app_id]:

"app_policies": { "app_id": "pre_DataConsent" }

It was my understanding we don't need to set properties on app_policies.app_id as it is a reference to app_policies.pre_DataConsent where the correct heartbeat timeout ms should live.

i see that this change does fix the issue, but really, when an app's section (app_policies_section.apps[app_id]) is a reference (like "app_id": "pre_DataConsent") should the properties like heart_beat_timeout_ms be pulled from the app that the reference points to (in provided case, that would be app_policies_section.apps["pre_DataConsent"])

@iCollin in this example we just mark that entry as a reference to preData entity and assign value of preData.heart_beat_timeout_ms to app_id.heart_beat_timeout_ms, however preData is still the initial source of that value. Looks like there was no another place where SDL pulls that value from preData.
When snapshot is generated, this app_id will be properly collapsed into record you mentioned above.

…outFromPreData

AKalinich-Luxoft · 2020-12-18T01:49:24Z

@iCollin regression and verification scripts are PASSED, fix is ready

dboltovskyi · 2020-12-18T13:36:14Z

@iCollin Please also consider update in scripts smartdevicelink/sdl_atf_test_scripts#2499 which enables related test

VVeremjova requested a review from LuxoftAKutsan May 18, 2017 06:37

LuxoftAKutsan suggested changes May 18, 2017

View reviewed changes

VVeremjova requested review from ATymchenko and dcherniev May 18, 2017 07:20

kozlov-oleksandr approved these changes May 18, 2017

View reviewed changes

ATymchenko approved these changes May 18, 2017

View reviewed changes

dcherniev approved these changes May 18, 2017

View reviewed changes

VVeremjova force-pushed the fix/FixApplyHBTimeoutFromPreData branch from 5400318 to e0bb7ba Compare May 19, 2017 06:54

LitvinenkoIra approved these changes May 22, 2017

View reviewed changes

LuxoftAKutsan approved these changes May 24, 2017

View reviewed changes

APipko mentioned this pull request Aug 29, 2018

SDL doesn't apply heart_beat_timeout_ms received after PTU #2550

Closed

LitvinenkoIra changed the base branch from develop to release/5.0.0 September 26, 2018 11:02

Veronica Veremjova added 3 commits September 26, 2018 14:05

Fix applying heart_beat_timeout_ms from PreDataConsent

d1010a8

After sending SDL.OnAllowSDLFunctionality(allowed = false) application is assigned to PreDataConsent group. After this HB timeout is set to timeout from PreDataConsent

Updated logs messages and simplify vars after review

1219419

Update unit tests

ef93b27

LitvinenkoIra force-pushed the fix/FixApplyHBTimeoutFromPreData branch from e0bb7ba to ef93b27 Compare September 26, 2018 11:08

iCollin changed the base branch from release/5.0.0 to develop November 16, 2020 22:13

iCollin requested changes Nov 16, 2020

View reviewed changes

LitvinenkoIra added the Waiting for Feedback Maintainers are waiting for feedback from an author or contributor label Nov 17, 2020

AKalinich-Luxoft added 3 commits December 17, 2020 14:20

Merge remote-tracking branch 'origin/develop' into fix/FixApplyHBTime…

8a59f2a

…outFromPreData

Fix syntax errors after rebase

17415e3

fixup! Fix syntax errors after rebase

059ef03

iCollin approved these changes Dec 22, 2020

View reviewed changes

iCollin merged commit 218aee1 into smartdevicelink:develop Dec 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix applying heart_beat_timeout_ms from PreDataConsent #1551

Fix applying heart_beat_timeout_ms from PreDataConsent #1551

VVeremjova commented May 18, 2017

VVeremjova commented May 18, 2017

LuxoftAKutsan May 18, 2017

VVeremjova May 18, 2017

LuxoftAKutsan May 18, 2017

VVeremjova May 18, 2017

LuxoftAKutsan May 18, 2017

VVeremjova May 18, 2017

LuxoftAKutsan May 18, 2017

VVeremjova May 18, 2017

LuxoftAKutsan May 18, 2017

VVeremjova May 18, 2017

LuxoftAKutsan May 18, 2017

VVeremjova May 18, 2017

LuxoftAKutsan commented May 18, 2017

ATymchenko left a comment

dcherniev left a comment

JenkinsSDLOnCloud commented May 19, 2017

JenkinsSDLOnCloud commented Oct 7, 2017

JenkinsSDLOnCloud commented Oct 10, 2017

JenkinsSDLOnCloud commented Oct 13, 2017

iCollin left a comment

iCollin Nov 16, 2020 •

edited

Loading

AKalinich-Luxoft Dec 17, 2020

AKalinich-Luxoft commented Dec 18, 2020

dboltovskyi commented Dec 18, 2020

Fix applying heart_beat_timeout_ms from PreDataConsent #1551

Fix applying heart_beat_timeout_ms from PreDataConsent #1551

Conversation

VVeremjova commented May 18, 2017

VVeremjova commented May 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LuxoftAKutsan commented May 18, 2017

ATymchenko left a comment

Choose a reason for hiding this comment

dcherniev left a comment

Choose a reason for hiding this comment

JenkinsSDLOnCloud commented May 19, 2017

JenkinsSDLOnCloud commented Oct 7, 2017

JenkinsSDLOnCloud commented Oct 10, 2017

JenkinsSDLOnCloud commented Oct 13, 2017

iCollin left a comment

Choose a reason for hiding this comment

iCollin Nov 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AKalinich-Luxoft commented Dec 18, 2020

dboltovskyi commented Dec 18, 2020

iCollin Nov 16, 2020 •

edited

Loading