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.

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

feat: customizable resolutions #23

Merged
merged 2 commits into from
Dec 21, 2021
Merged

Conversation

NorbertNader
Copy link
Contributor

@NorbertNader NorbertNader commented Dec 21, 2021

Overview

feat: customizable resolutions

  • added customizable resolutions for sitewise data source which means we can specify a resolution per asset property
  • refactored resolutionMapping to resolution and allowed to pass string if we want a specific resolution for all queries

Screen Shot 2021-12-20 at 10 08 31 PM

Tests

https://github.com/awslabs/iot-app-kit/runs/4597925804?check_suite_focus=true

Legal

This project is available under the Apache 2.0 License.

@NorbertNader NorbertNader added the enhancement New feature or request label Dec 21, 2021
@NorbertNader NorbertNader self-assigned this Dec 21, 2021
@NorbertNader NorbertNader force-pushed the customizable-resolutions branch 3 times, most recently from 5a9173f to 78661d9 Compare December 21, 2021 07:51
* added customizable resolutions for sitewise data source which means we can specify a resolution per asset property
* refactored resolutionMapping to resolution and allowed to pass string if we want a specific resolution for all queries
let defaultResolutionDataQueries: SiteWiseAssetDataStreamQuery | undefined;

query.assets.forEach(({ assetId, properties }) => {
let aggregatedDataProperties: PropertyQuery[] | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove if conditions by defining the type as an Array only and in the line 87 just add to the array instead of initializing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would rather be specific of the shape of the array and keep it as PropertyQuery type or just undefined to keep it consistent with the return type. This was my first approach though but I ran into type errors like

TS7034: Variable 'aggregatedDataProperties' implicitly has type 'any[]' in some locations where its type cannot be determined.

Copy link
Contributor

@fpauer fpauer Dec 21, 2021

Choose a reason for hiding this comment

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

But if you defined an array of type PropertyQuery you should not get any ts error. I would rather reduce if conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I define an array of the PropertyQuery it would have to be like { propertyId: '' } since we don't have any properties yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can just have an empty array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, we can just initialize them as empty array.

@NorbertNader NorbertNader merged commit 08129f1 into main Dec 21, 2021
@NorbertNader NorbertNader deleted the customizable-resolutions branch January 14, 2022 22:47
sheilaXu pushed a commit that referenced this pull request Sep 23, 2022
- Save/Loading Works
- Fixed issues with Compatibility with Enhanced Edit Feature
- SubModels are compatbile with any tool that operates on an object 3D

Co-authored-by: Mitchell Lee <mitchlee@amazon.com>
TheEvilDev pushed a commit that referenced this pull request Nov 2, 2022
* added customizable resolutions for sitewise data source which means we can specify a resolution per asset property
* refactored resolutionMapping to resolution and allowed to pass string if we want a specific resolution for all queries

Co-authored-by: Norbert Nader <nnader@amazon.com>
This was referenced Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants