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

Fix/Get Display Capabilities based on template #590

Merged
merged 3 commits into from
Aug 17, 2021
Merged

Fix/Get Display Capabilities based on template #590

merged 3 commits into from
Aug 17, 2021

Conversation

ShobhitAd
Copy link
Contributor

Implements/Fixes #586

This PR is ready for review.

Testing Plan

  • Test changing the template for Show and SetDisplayLayout and observe the OnSystemCapabilityUpdated(DISPLAYS) notification.
  • Test adding a widget using CreateWindow and observe the OnSystemCapabilityUpdated(DISPLAYS) notification.

Summary

  • Added method getDisplayCapabilities in Controller.js
  • Replaced usages of getDefaultCapabilities with getDisplayCapabilities in UIRPCs.js

CLA

@ShobhitAd ShobhitAd changed the base branch from master to develop August 16, 2021 14:10
iCollin
iCollin reviewed Aug 16, 2021
View reviewed changes
app/controller/sdl/Abstract/Controller.js Outdated
appModel.getWidgetModels().forEach(element => {
let capability = SDL.templateCapabilities[element.content.templateConfiguration.template].displayCapabilities;
capability.windowTypeSupported = windowTypeSupportedWidget;
displayCapabilities.concat(capability)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
displayCapabilities.concat(capability)
displayCapabilities.push(capability);

Even after I modified this line, the widget capability seems to be cut off by Core. When the HMI sends with displayCapabilities.length == 2 I am still getting displayCapabilities.length == 1 on mobile. Not sure why, but the Widget I created was the bare minimum and I didn't send a Show.

Copy link
Contributor Author

@ShobhitAd ShobhitAd Aug 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Based on https://github.com/smartdevicelink/sdl_core/blob/master/src/components/interfaces/HMI_API.xml#L3815, it looks like the OnSystemCapabilityUpdated notification should only contain capabilities for the updated window. Added the changes in ce1db5c

So, when you send a CreateWindow request, the returned OSCU notification should only contain the capabilities for the widget(should include the correct windowID) but to get the capabilities for all windows you'll have to send a GetSystemCapability(DISPLAYS) request

iCollin
iCollin approved these changes Aug 17, 2021
View reviewed changes
app/controller/sdl/Abstract/Controller.js Outdated
// MAIN window
let displayCapabilities = [SDL.templateCapabilities[appModel.templateConfiguration.template].displayCapabilities];
displayCapabilities[0].windowTypeSupported = windowTypeSupportedMain;
getDisplayCapabilities: function(appID, windowID){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that this method just returns one displayCapability, I think naming it getDisplayCapability might be more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Changed in ba1b347

@ShobhitAd ShobhitAd merged commit 8d81482 into develop Aug 17, 2021
@ShobhitAd ShobhitAd deleted the fix/display_capabilities_based_on_template branch August 17, 2021 21:06
@ShobhitAd ShobhitAd mentioned this pull request Aug 17, 2021
Closed
@jacobkeeler jacobkeeler mentioned this pull request Sep 20, 2021
Closed
@ShobhitAd ShobhitAd mentioned this pull request Sep 20, 2021
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iCollin iCollin iCollin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HMI does not update capabilities based on template
2 participants
@ShobhitAd @iCollin