Some viewport profiles in getViewportProfile are not determined correctly

TAKARA328 commented 3 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Expected Behavior

getViewportProfile() returns the correct viewport profile.

Current Behavior

When I tried it with the Alexa simulator, I got the following results.

No.	Alexa Simulator	getViewportProfile()
1	Hub Round Small	HUB-ROUND-SMALL	✔︎
2	Hub Landscape Small	HUB-LANDSCAPE-SMALL	✔︎
3	Hub Landscape Medium	MOBILE-LANDSCAPE-MEDIUM
4	Hub Landscape Large	HUB-LANDSCAPE-LARGE	✔︎
5	TV Landscape Extra Large	TV-LANDSCAPE-XLARGE	✔︎
6	Mobile Small	UNKNOWN-VIEWPORT-PROFILE
7	Mobile Medium	HUB-LANDSCAPE-MEDIUM
8	Mobile Large	MOBILE-LANDSCAPE-MEDIUM

Possible Solution

The cause is that there is an error in the conditional expression. Modify the conditional expression and use mode. Add the condition of MOBILE-LANDSCAPE-LARGE and MOBILE-PORTRAIT-LARGE.

getViewportProfile()	Changes
HUB-LANDSCAPE-MEDIUM	- viewportDpiGroup to 'MEDIUM'
MOBILE-LANDSCAPE-SMALL MOBILE-PORTRAIT-SMALL	- viewportDpiGroup to 'LOW'
MOBILE-LANDSCAPE-MEDIUM	- viewportDpiGroup to 'LOW' - pixelHeightSizeGroup to 'XSMALL'
MOBILE-PORTRAIT-MEDIUM	- pixelWidthSizeGroup to 'LOW' - pixelHeightSizeGroup to 'XSMALL'
MOBILE-LANDSCAPE-LARGE MOBILE-PORTRAIT-LARGE	- New addition

        if (mode === 'HUB') {
            if (shape === 'ROUND'
                && viewportOrientation === 'EQUAL'
                && viewportDpiGroup === 'LOW'
                      :
                      :

Modify the test case of ViewportUtils.spec.ts.

For example HUB-LANDSCAPE-MEDIUM Added mode. Changed dpi from 160 to 213.

        requestEnvelope.context.Viewport.shape = 'RECTANGLE';
        requestEnvelope.context.Viewport.mode = 'HUB';
        requestEnvelope.context.Viewport.currentPixelHeight = 600;
        requestEnvelope.context.Viewport.currentPixelWidth = 960;
        requestEnvelope.context.Viewport.dpi = 213;
        expect(getViewportProfile(requestEnvelope)).eq('HUB-LANDSCAPE-MEDIUM');

Steps to Reproduce (for bugs)

Context

Your Environment

ASK SDK for Node.js used: 2.10.2 (ask-sdk-core)
Operating System and version: Windows 10

Node.js and NPM Info

Node.js version used for development: 14.17.0
NPM version used for development: 7.16.0

ShenChen93 commented 3 years ago

Hi @TAKARA328

Thanks for posting this issue. It turns out this util function is not up to date with latest viewportProfile table. We will update this util function ASAP after we confirm with viewportProfile team.

Thanks, Shen

TAKARA328 commented 3 years ago

Hi @ShenChen-Amazon

Thanks for good reply. I'm looking forward to it fixed.

Thanks, TAKARA

aknad commented 2 years ago

Hi @ShenChen-Amazon, do you have an update about this issue? thanks

aknad commented 2 years ago

the viewports of the echo show 15 are also missing

rahulawl commented 2 years ago

Is this issue/feature-request still relevant? We are working on prioritization of relevant issues and cleanup of rest. If we don’t hear back in 2 weeks, we will assume that the issue is not relevant and we will close it.

TAKARA328 commented 2 years ago

I would like you to fix it.

alexa / alexa-skills-kit-sdk-for-nodejs