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.

Sign up for GitHub

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

Jump to bottom

Change conditions to transfer on button press on button event #757

Closed
wants to merge 3 commits into from
Closed

Change conditions to transfer on button press on button event #757

wants to merge 3 commits into from

Conversation

OHerasym
Copy link
Contributor

PR Changes:

  • Fix OnButtonPress & OnButtonEvent description
  • Change conditions to transfer OnButtonEvent
  • Change conditions to transfer OnButtonPress

Related: APPLINK-25470

@OHerasym
Copy link
Contributor Author

OHerasym commented Aug 16, 2016 •
edited
Loading

Please review: @dtrunov , @anosach-luxoft , @LuxoftAKutsan ,@VVeremjova, @orhan-mehmedov

@OHerasym
Copy link
Contributor Author

@TMelnyk, Please review HMI_API.xml file

@TMelnyk
Copy link

TMelnyk commented Aug 16, 2016 •
edited
Loading

@OHerasym , please change:

a. OnButtonEvent
param name="appID" type="Integer" mandatory="false">

In case the ButtonName is CUSTOM_BUTTON or OK, HMI must include appID parameters to OnButtonPress notification sent to SDL.
If appID is not sent together with CUSTOM_BUTTON, this notification will be ignored by SDL.
If appID is present for OK button -> SDL transfers notification to the named app only if -if- -> it is in FULL or LIMITED (ignores if app is in NONE or BACKGROUND).
If appID is omited for OK button -> SDL -tramsfers- transfers notification to app in FULL

b. OnButtonPress

In case the ButtonName is CUSTOM_BUTTON or OK, HMI must include appID parameters to OnButtonEvent notification sent to SDL. If appID is not sent together with CUSTOM_BUTTON, this notification will be ignored by SDL. If appID is present for OK button -> SDL transfers notification to the named app only if _-if-_ **it** is in FULL or LIMITED (ignores if app is in NONE or BACKGROUND). If appID is omited for OK button -> SDL _-tramsfers-_ **transfers** notification to app in FULL

c. Also please update date and version of corresponding interface at HMI_API per https://adc.luxoft.com/confluence/pages/viewpage.action?title=API+xml+versioning+approach&spaceKey=APPLINK)

dtrunov
dtrunov reviewed Aug 16, 2016
View reviewed changes
src/components/application_manager/src/commands/mobile/on_button_event_notification.cc
(!subscribed_app->IsFullscreen())) {
continue;
}
if(app.valid()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@OHerasym please use clang format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dtrunov , 9c4f086, Done

@dtrunov
Copy link
Contributor

dtrunov commented Aug 16, 2016

@OHerasym Reviewed

2 similar comments
@VVeremjova
Copy link
Contributor

@OHerasym Reviewed

@orhan-mehmedov
Copy link
Contributor

@OHerasym Reviewed

@OHerasym
Copy link
Contributor Author

@TMelnyk , 9c4f086, updated date & interface version

anosach-luxoft
anosach-luxoft reviewed Aug 18, 2016
View reviewed changes
src/components/application_manager/src/commands/mobile/on_button_event_notification.cc
(!subscribed_app->IsFullscreen())) {
continue;
}
if (app.valid()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@OHerasym you just can check if(app)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anosach-luxoft , 21d4ff1 , Done

@anosach-luxoft
Copy link
Contributor

@OHerasym please do not name commits as temp even for comments with changes after review

@OHerasym OHerasym closed this Aug 19, 2016
@TMelnyk
Copy link

TMelnyk commented Aug 19, 2016 •
edited
Loading

@OHerasym
Still does NOT updated for OnButtonEvent and OnButtonPress:
" If appID is present for OK button -> SDL transfers notification to the named app only if if (should be IT) is in FULL or LIMITED (ignores if app is in NONE or BACKGROUND)."

@joeygrover
Copy link
Member

joeygrover commented Aug 22, 2016 •
edited
Loading

@OHerasym What was the resolution here? Will a new PR be opened to solve the issue?

@OHerasym OHerasym mentioned this pull request Aug 23, 2016
Merged
@OHerasym
Copy link
Contributor Author

@joeygrover , Yes , new pull request is created #775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@OHerasym @TMelnyk @dtrunov @VVeremjova @orhan-mehmedov @anosach-luxoft @joeygrover