bjorn/cnx 829 add root object builder and create collection structure #435

bjoernsteinhagen · 2024-12-04T09:14:26Z

Description & motivation

Implementation of collection structure and property extraction for CSi connectors, focusing on ETABS-specific organization while maintaining SAP2000 compatibility
Related to CNX-829

Changes:

Added root object builder and collection management system
Collection structure see Notion page
Created hierarchical architecture for CSi shared vs product-specific functionality. Included creating ETABSShared projects:
- Base geometry conversion in CSiShared (Line, Point, Mesh)
- Base property extraction framework (common to SAP 2000 and ETABS)
- ETABS-specific property extraction (story, design orientation)
- ETABS-specific collection organization by story and element type
Implemented clean separation between geometry and property converters

Screenshots:

Testing stream here.

Validation of changes:

Collection structure verified with ETABS test model
Property extraction tested with various joint, frame and shell elements
Geometry conversion validated against existing implementations

Checklist:

My pull request follows the guidelines in the Contributing guide?
My pull request does not duplicate any other open Pull Requests for the same update/change?
My commits are related to the pull request and do not amend unrelated code or documentation.
My code follows a similar style to existing code.
I have added appropriate tests.
I have updated or added relevant documentation.

- Implemented ETABSShared projects for Connectors and Converters to account for ETABS specific things that SAP 2000 doesn't account for such as levels or frames being further classified as Column, Beam or Brace or shells as wall or floor

- POC for ETABS collection structure - Logic for getting further classificiations of Frames as either Column, Beam or Brace - Same for Shells: either Wall or Floor etc. - Placeholders for object properties

- Putting thoughts to "paper" - A lot of the TODOs will be part of next milestone

…ate-collection-structure

linear · 2024-12-04T09:14:29Z

CNX-829 Add root object builder and create collection structure

got deactivated and wasn't formatting on save anymore :(

codecov · 2024-12-04T09:36:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 9.45%. Comparing base (f0e54ca) to head (3cdd1f1).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@          Coverage Diff          @@
##             dev    #435   +/-   ##
=====================================
  Coverage   9.45%   9.45%           
=====================================
  Files        223     223           
  Lines       4199    4199           
  Branches     483     483           
=====================================
  Hits         397     397           
  Misses      3786    3786           
  Partials      16      16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

oguzhankoral

It is a neat PR! Thanks for many notes and TODOs. you know already next steps!
My comments are mostly questions, willing to understand some needs better.

oguzhankoral · 2024-12-05T15:06:13Z

Connectors/CSi/Speckle.Connectors.ETABS21/Plugin/SpeckleForm.cs

 {
-  protected override HostAppVersion GetVersion() => HostAppVersion.v2021;
+  protected override HostAppVersion GetVersion() => HostAppVersion.v2021; // TODO: We need a v21


should not be forgotten adding v21 and v22 before release

specklesystems/speckle-sharp-sdk#187

oguzhankoral · 2024-12-05T15:08:51Z

Connectors/CSi/Speckle.Connectors.ETABSShared/HostApp/ETABSSendCollectionManager.cs

+
+  // TODO: This is gross. Too many strings. Improve as part of next milestone. Out of scope of "First Send".
+  private string GetObjectType(Base convertedObject, Dictionary<string, object>? properties)
+  {


a bit confused on why we mutate properties on collection manager instead of converter

Good point. Probably better to add a collection parameter during the conversions and just simply use that on the collection manager to group (and not mix design concerns)

oguzhankoral · 2024-12-05T15:10:18Z

Connectors/CSi/Speckle.Connectors.ETABSShared/HostApp/ETABSSendCollectionManager.cs

+  // TODO: This is gross. Too many strings. Improve as part of next milestone. Out of scope of "First Send".
+  public override Collection AddObjectCollectionToRoot(Base convertedObject, Collection rootObject)
+  {
+    var properties = convertedObject["properties"] as Dictionary<string, object>;


hmm, why we need properties in collection manager? this looks like anti-pattern with other connectors. happy to chat and understand if i am missing something on ETABS side

Definitely room for improvement here. Happy to sync

Converters/CSi/Speckle.Converters.CSiShared/ServiceRegistration.cs

oguzhankoral · 2024-12-05T15:19:38Z

Converters/CSi/Speckle.Converters.CSiShared/ToSpeckle/Geometry/LineToSpeckleConverter.cs

  {
-    // frame points
+    // TODO: Better exception handling


ideally you should not necessarily handle exceptions here in typed converters. We handle conversion exceptions on RootObjectBuilders to report them UI. see here . But not sure you have special cases around line, let me know if so, i would like to know

For Etabs' API, the return for queries is an int, saying if it was successfull or not. All of these should be returning 0 (CSi's way of saying succeeded). Currently, this is handled in the if blocks. It felt weird just discarding all of these returns and assuming a success?

oguzhankoral · 2024-12-05T15:22:35Z

Converters/CSi/Speckle.Converters.CSiShared/CSiWrappers.cs

-      6 => new CSiSolidWrapper { Name = name },
-      7 => new CSiLinkWrapper { Name = name },
+      6 => new CSiSolidWrapper { Name = name }, // TODO: CSiSolidWrapper
+      7 => new CSiLinkWrapper { Name = name }, // TODO: CSiLinkWrapper


I am curious on this wrappers. Do they needed to detect different objects in converters?

Yeah, a bit of a workaround, since the API doesn't give us the instances, we can only query the selection.

So here we don't get objects, but an int telling us what objectType it is and an objectName which is unique for that objectType only. So, in order to be able to ResolveConverter we need to wrap this info into an "object" that keeps Etabs / Csi name and type dependencies.

oguzhankoral · 2024-12-05T15:27:03Z

Converters/CSi/Speckle.Converters.CSiShared/ToSpeckle/TopLevel/CSiObjectToSpeckleConverter.cs

+    else if (target is CSiShellWrapper)
+    {
+      _ = _settingsStore.Current.SapModel.AreaObj.GetGUID(target.Name, ref applicationId);
+    }


i guess we have assumption here if the target is unknown, we let applicationId as empty string. Just a side note for later, applicationIds are important once you start to get automation results in ETABS UI to be able highlight them etc. But this is maybe for later. For now it is ok.

Absolutely! We have it earmarked for next milestone, here CNX-881. I am currently not happy with the state of applicationId. It is currently this way because Etabs doesn't use those when querying the model, but the name, so I haven't paid much attention to it.

oguzhankoral · 2024-12-05T15:29:00Z

Converters/CSi/Speckle.Converters.CSiShared/ToSpeckle/Raw/CSiFrameToSpeckleConverter.cs

+    return properties;
+  }
+
+  private void AddCommonClassProperties(CSiFrameWrapper frame, Dictionary<string, object> properties)


i will let @clairekuang to judge mutating properties on abstract class, bc we were doing them with some extractors?

For sure just a placeholder for now. We discussed for CNX-882 how to do it better, so changes are coming on these definitions.

…ate-collection-structure

bjoernsteinhagen added 5 commits December 3, 2024 17:31

ETABSShared

b042bee

- Implemented ETABSShared projects for Connectors and Converters to account for ETABS specific things that SAP 2000 doesn't account for such as levels or frames being further classified as Column, Beam or Brace or shells as wall or floor

collection poc

dd50dbf

- POC for ETABS collection structure - Logic for getting further classificiations of Frames as either Column, Beam or Brace - Same for Shells: either Wall or Floor etc. - Placeholders for object properties

documentation

de23658

- Putting thoughts to "paper" - A lot of the TODOs will be part of next milestone

Merge branch 'dev' into bjorn/cnx-835-add-root-object-builder-and-cre…

eb59815

…ate-collection-structure

adding new projects to local.sln

b324039

bjoernsteinhagen requested a review from oguzhankoral December 4, 2024 09:14

csharpier

df020e9

got deactivated and wasn't formatting on save anymore :(

clairekuang self-requested a review December 4, 2024 19:17

oguzhankoral reviewed Dec 5, 2024

View reviewed changes

bjoernsteinhagen and others added 3 commits December 5, 2024 20:27

unnecessary registration

051dd70

using directives

ff60fd3

Merge branch 'dev' into bjorn/cnx-829-add-root-object-builder-and-cre…

3cdd1f1

…ate-collection-structure

oguzhankoral approved these changes Dec 5, 2024

View reviewed changes

oguzhankoral merged commit 0341ebc into dev Dec 5, 2024
5 checks passed

oguzhankoral deleted the bjorn/cnx-829-add-root-object-builder-and-create-collection-structure branch December 5, 2024 20:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bjorn/cnx 829 add root object builder and create collection structure #435

bjorn/cnx 829 add root object builder and create collection structure #435

bjoernsteinhagen commented Dec 4, 2024

linear bot commented Dec 4, 2024

codecov bot commented Dec 4, 2024 •

edited

Loading

oguzhankoral left a comment

oguzhankoral Dec 5, 2024

bjoernsteinhagen Dec 5, 2024

oguzhankoral Dec 5, 2024

bjoernsteinhagen Dec 5, 2024

oguzhankoral Dec 5, 2024

bjoernsteinhagen Dec 5, 2024

oguzhankoral Dec 5, 2024

bjoernsteinhagen Dec 5, 2024 •

edited

Loading

oguzhankoral Dec 5, 2024

bjoernsteinhagen Dec 5, 2024

oguzhankoral Dec 5, 2024

bjoernsteinhagen Dec 5, 2024

oguzhankoral Dec 5, 2024

bjoernsteinhagen Dec 5, 2024

bjorn/cnx 829 add root object builder and create collection structure #435

bjorn/cnx 829 add root object builder and create collection structure #435

Conversation

bjoernsteinhagen commented Dec 4, 2024

Description & motivation

Changes:

Screenshots:

Validation of changes:

Checklist:

linear bot commented Dec 4, 2024

codecov bot commented Dec 4, 2024 • edited Loading

Codecov Report

oguzhankoral left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjoernsteinhagen Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 4, 2024 •

edited

Loading

bjoernsteinhagen Dec 5, 2024 •

edited

Loading