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

Added Door <-> IfcDoor conversion #1012

Merged
merged 13 commits into from
Sep 14, 2023

Conversation

srudenkoamc
Copy link
Contributor

@srudenkoamc srudenkoamc commented Aug 24, 2023

BACKGROUND:

  • IFC serialization is to be improved.

DESCRIPTION:

  • Moved Door class with related enums to Elements.
  • Added Door <-> IfcDoor conversion.

TESTING:

  • Added a test for Door -> IfcDoor conversion.
  • Tested IfcDoor -> Door on AC-20-Smiley-West-10-Bldg.ifc (Representation: IfcMappedItem).
  • Converted the door test resulting model back from IFC (Representation: IfcExtrudedAreaSolid).

FUTURE WORK:

  • The conversion supports only left-hand, right-hand and double doors opening along with single swing and double swing. Other opening types aren't supported yet.
  • IfcDoorLiningProperties and IfcDoorPanelProperties aren't supported.
  • Add support for IfcFacetedBrep representation.

This change is Reviewable

@srudenkoamc srudenkoamc marked this pull request as ready for review August 24, 2023 14:45
@anthonie-kramer anthonie-kramer self-assigned this Aug 24, 2023
@anthonie-kramer anthonie-kramer removed their assignment Aug 24, 2023
Copy link
Contributor

@anthonie-kramer anthonie-kramer left a comment

Choose a reason for hiding this comment

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

Looks good!

I assume that the Door.cs definition will be unified with the one from the Doors layout function.

Also, do you mind producing an IFC file with a door that I can review?

Reviewed 4 of 7 files at r1, 4 of 4 files at r3, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @srudenkoamc)


Elements/test/ContentTests.cs line 94 at r3 (raw file):

        }

        // TODO: The test fails in 70 of 100 test runs. Uncomment when it is fixed.

Did you get this working?

@srudenkoamc
Copy link
Contributor Author

Looks good!

I assume that the Door.cs definition will be unified with the one from the Doors layout function.

Also, do you mind producing an IFC file with a door that I can review?

Reviewed 4 of 7 files at r1, 4 of 4 files at r3, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @srudenkoamc)

Elements/test/ContentTests.cs line 94 at r3 (raw file):

        }

        // TODO: The test fails in 70 of 100 test runs. Uncomment when it is fixed.

Did you get this working?

The Door.cs definition will be unified with the one from the Doors layout function. After the merge I'm planning to make a separate PR to update 'Elements' in HyparSpace. During this I'll check if everything works and will remove the old Door definition.

About the test - I've just commented it for now because it sometimes does break the build and sometimes doesn't. Eric said that he worked with it and that he will take a look.

Copy link
Contributor

@anthonie-kramer anthonie-kramer left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r4, 3 of 3 files at r5, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@anthonie-kramer anthonie-kramer merged commit 59e6315 into hypar-io:master Sep 14, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants