-
Notifications
You must be signed in to change notification settings - Fork 60
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
Conversation
5a9173f
to
78661d9
Compare
* 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
01ed950
to
ecdfeb6
Compare
packages/core/src/data-sources/site-wise/client/getAggregatedPropertyDataPoints.ts
Show resolved
Hide resolved
let defaultResolutionDataQueries: SiteWiseAssetDataStreamQuery | undefined; | ||
|
||
query.assets.forEach(({ assetId, properties }) => { | ||
let aggregatedDataProperties: PropertyQuery[] | undefined; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
- 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>
* 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>
Overview
feat: customizable resolutions
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.