Feature/sdl 0189 restructuring on reset timeout

[LitvinenkoIra]

Implements #376

This PR is [ready] for review.

Testing Plan

0189_Restructuring OnResetTimeout_HMI_instruction.xlsx

Summary

The ability to send OnResetTimeout for RPS with different interfaces has been added.

CLA

• I have signed the CLA

[atiwari9]

More changes are needed as per this SDL_Core comment: smartdevicelink/sdl_core#3726 (comment)

[atiwari9]

@ValeriiMalkov , @LitvinenkoIra - Please let me know when the PR is ready to be reviewed again after above mentioned changes.

[AKalinich-Luxoft]

@ValeriiMalkov , @LitvinenkoIra - Please let me know when the PR is ready to be reviewed again after above mentioned changes.

All required changes have been provided in 99c82a9

[LitvinenkoIra]

@theresalech This PR is ready for Livio review. Thank you!

[iCollin]

When UI interactions are deactivated for reason other than timeout, they must be removed from SDL.ResetTimeoutPopUp.resetTimeoutRPCs so that a timeout is not sent later.

[iCollin]

Many components had their "timeout timers" moved to the ResetTimeoutPopup component like this:

SDL.ResetTimeoutPopUp.expandCallbacks(function(){
                SDL.ScrollableMessage.deactivate();
              }, request.method);

But many of these same components still have a "timer" that seems to be for timeout in the components themselves. These duplicate timers should be removed.

[iCollin]

It will be common for apps to have multiple pending requests at the same time. The SDL.ResetTimeoutPopUp needs to be able to handle this, and requires a few changes:

• Remove any calls to SDL.ResetTimeoutPopUp.DeactivatePopUp when there are other pending messages
  • AlertManeuver does this already
• Modify SDL.ResetTimeoutPopUp.resetTimeout or provide another method to be able to reset the timeout of just one message.
  • Currently this method is called from both clicking resetTimeoutButton as well as when the timer should be reset for ScrollableMessage and Slider. At least the calls from the RPCs should be for just that one RPC's timeout.

[iCollin]

I recommend consolidating the objects indexed by [method] on SDL.ResetTimeoutPopUp to one object of objects. Currently we are tracking the requestIDs, timeoutSeconds, callbacks and resetTimeoutCallback data like this:

// array resetTimeoutRPCs: [
	'UI.Alert'
]
// object 1 requestIDs: {
	'UI.Alert': 1
}
// object 2 timeoutSeconds: {
	'UI.Alert': 5
}
// object 3 callbacks: {
	'UI.Alert': () => { console.log('alert timed out'); }
}
// object 4 resetTimeoutCallback: {
	'UI.Alert': () => { console.log('alert timeout reset'); }
}

And it may be easier to use in code like this:

// object resetTimeoutRPCs: {
	1: { // key is requestID
		method: 'UI.Alert',
		timeoutSeconds: 5,
		callback: () => { console.log('alert timed out'); },
		resetTimeoutCallback: () => { console.log('alert timeout reset'); }
	}
}

There are two reasons:

• Easier to use in code
  • For example, right now only the resetTimeoutRPCs member of those 5 is cleaned up before deactivate is called.
  • Cleanup is just delete resetTimeoutRPCs[1] instead of modifying many objects
• Supports multiple of the same method
  • Not necessary now, but may be in the future if interactions can be sent to multiple windows etc.

[iCollin]

Where applicable, it would be much easier to follow these changes if the code for setting up the timeout callbacks was placed within interaction components.

For example, I believe all the changes from ffw/NavigationRPC.js should be moved to the SDL.AlertManeuverPopUp.AlertManeuverActive method. This same pattern can be followed for other RPC file changes.

[iCollin]

Please merge in develop and feel free to message me on slack if you have any questions!

[ValeriiMalkov]

I recommend consolidating the objects indexed by [method] on SDL.ResetTimeoutPopUp to one object of objects. Currently we are tracking the requestIDs, timeoutSeconds, callbacks and resetTimeoutCallback data like this:

// array resetTimeoutRPCs: [
	'UI.Alert'
]
// object 1 requestIDs: {
	'UI.Alert': 1
}
// object 2 timeoutSeconds: {
	'UI.Alert': 5
}
// object 3 callbacks: {
	'UI.Alert': () => { console.log('alert timed out'); }
}
// object 4 resetTimeoutCallback: {
	'UI.Alert': () => { console.log('alert timeout reset'); }
}

And it may be easier to use in code like this:

// object resetTimeoutRPCs: {
	1: { // key is requestID
		method: 'UI.Alert',
		timeoutSeconds: 5,
		callback: () => { console.log('alert timed out'); },
		resetTimeoutCallback: () => { console.log('alert timeout reset'); }
	}
}

There are two reasons:

• Easier to use in code

  • For example, right now only the resetTimeoutRPCs member of those 5 is cleaned up before deactivate is called.
  • Cleanup is just delete resetTimeoutRPCs[1] instead of modifying many objects

• Supports multiple of the same method

  • Not necessary now, but may be in the future if interactions can be sent to multiple windows etc.

Note that this is a significant refactoring and it will require a full testing cycle.
According to our estimates we will be able to deliver these changes by the end of next week (07.30). Are you ok with that?

[iCollin]

There is one comment above that I think was missed: #575 (comment)

[iCollin]

Updates look great, thank you for the quick responses.

[iCollin]

This is checked when the key is added to timeoutExpired, i think this check is redundant.

[ValeriiMalkov]

@iCollin Agree 0f733e6

[ValeriiMalkov]

@iCollin Implemented according a comment in commit e9d199d

[iCollin]

It seems resetting the timeout for DialNumber was missed, but it was mentioned explicitly in the proposal.
This currently works because Core enforces no timeout for this RPC.

[iCollin]

If an app sends a ScrollableMessage with a timeout of 50000ms, and then clicks the scroll bar to scroll down after say 10 seconds, the SDL HMI will send an OnResetTimeout(resetPeriod=10000ms), essentially now shortening the timeout of the ScrollableMessage to 20000ms.
Previously the timeout would have been reset to the original value. This is also the case with PerformInteraction, and with Slider when the data is changed.
I think the resetPeriod we send needs to be the original timeout value to preserve the timeout functionality.

To solve this we could add a line like this to the object created in addRpc:

originalTimeout: timeoutSeconds

And then check for this value when resetting the timeout.

[ValeriiMalkov]

If an app sends a ScrollableMessage with a timeout of 50000ms, and then clicks the scroll bar to scroll down after say 10 seconds, the SDL HMI will send an OnResetTimeout(resetPeriod=10000ms), essentially now shortening the timeout of the ScrollableMessage to 20000ms.
Previously the timeout would have been reset to the original value. This is also the case with PerformInteraction, and with Slider when the data is changed.
I think the resetPeriod we send needs to be the original timeout value to preserve the timeout functionality.

To solve this we could add a line like this to the object created in addRpc:

originalTimeout: timeoutSeconds

And then check for this value when resetting the timeout.

@iCollin Implemented in commit 22d6fb1
Please take a look changes

[iCollin]

@iCollin Implemented in commit 22d6fb1

These changes need to also be applied to resetTimeoutSpecificRpc

[ValeriiMalkov]

@iCollin Implemented in commit 22d6fb1

These changes need to also be applied to resetTimeoutSpecificRpc

@iCollin Applied 10efb8b
and here f1416cb

[iCollin]

@ValeriiMalkov Keep context soft buttons are not resetting the timeout. Cases for some UI Components need to be re-added to keepContextSoftButton in app/controller/sdl/Abstract/Controller.js

[ValeriiMalkov]

@ValeriiMalkov Keep context soft buttons are not resetting the timeout. Cases for some UI Components need to be re-added to keepContextSoftButton in app/controller/sdl/Abstract/Controller.js

@iCollin Added for Alert and SubtleAlert cf12e81