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

Added boolean and integer extractor #3128

Merged
merged 30 commits into from
Oct 18, 2022
Merged

Added boolean and integer extractor #3128

merged 30 commits into from
Oct 18, 2022

Conversation

JayHuLBL
Copy link
Contributor

@JayHuLBL JayHuLBL commented Oct 5, 2022

This closes #3125.

@JayHuLBL JayHuLBL requested a review from mwetter October 5, 2022 23:00
@JayHuLBL JayHuLBL self-assigned this Oct 5, 2022
mwetter
mwetter requested changes Oct 6, 2022
View reviewed changes
Copy link
Member

@mwetter mwetter left a comment

Choose a reason for hiding this comment

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

See inline comments, lets discuss the implementation and documentation.

Buildings/Controls/OBC/CDL/Routing/BooleanExtractSignal.mo Show resolved Hide resolved
Buildings/Controls/OBC/CDL/Routing/BooleanExtractor.mo Outdated Show resolved Hide resolved
Buildings/Controls/OBC/CDL/Routing/BooleanExtractor.mo Outdated Show resolved Hide resolved
Buildings/Controls/OBC/CDL/Routing/BooleanExtractor.mo Outdated Show resolved Hide resolved
Buildings/Controls/OBC/CDL/Routing/IntegerExtractor.mo Outdated Show resolved Hide resolved
Buildings/Controls/OBC/CDL/Routing/Validation/BooleanExtractor.mo Show resolved Hide resolved
@JayHuLBL
Copy link
Contributor Author

@mwetter in current implementation, it will issue the warning when the extract index is out of range and the output value will be the nearest valid index.

@JayHuLBL
Copy link
Contributor Author

@mwetter It will be ready for your review when the CI tests passed.

@JayHuLBL
Copy link
Contributor Author

@mwetter The last commit has some minor changes to the text. I have tested locally the html syntax and the file format.

It's ready for your review.

mwetter
mwetter requested changes Oct 14, 2022
View reviewed changes
Copy link
Member

@mwetter mwetter left a comment

Choose a reason for hiding this comment

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

@JayHuLBL : Please see inline comments for initial feedback on this revised version that is not ready for release.

Buildings/Controls/OBC/CDL/Routing/BooleanExtractor.mo Outdated Show resolved Hide resolved
Buildings/Controls/OBC/CDL/Routing/RealExtractor.mo Outdated Show resolved Hide resolved
Buildings/Controls/OBC/CDL/Routing/BooleanExtractSignal.mo Outdated Show resolved Hide resolved
@JayHuLBL
Copy link
Contributor Author

@mwetter The last commit changed line color. It is ready for your review.

mwetter
mwetter requested changes Oct 17, 2022
View reviewed changes
Copy link
Member

@mwetter mwetter left a comment

Choose a reason for hiding this comment

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

Please see inline comment.

Buildings/Resources/Scripts/Conversion/ConvertBuildings_from_9_to_10.0.0.mos Outdated Show resolved Hide resolved
Buildings/Resources/Scripts/Conversion/ConvertBuildings_from_9_to_10.0.0.mos Show resolved Hide resolved
@mwetter mwetter enabled auto-merge (squash) October 18, 2022 01:34
@mwetter mwetter merged commit 5742d50 into master Oct 18, 2022
@mwetter mwetter deleted the issue3125_addExtractors branch October 18, 2022 05:46
@JayHuLBL JayHuLBL mentioned this pull request Mar 10, 2023
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwetter mwetter mwetter approved these changes

Assignees

@JayHuLBL JayHuLBL

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add boolean and integer extractor as CDL elementary block
2 participants
@JayHuLBL @mwetter