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

Pull and push Rigid constraints #297

Merged
merged 7 commits into from
Oct 3, 2023

Conversation

TosteSkDa
Copy link
Contributor

@TosteSkDa TosteSkDa commented May 2, 2023

Added feature to read, pull and push rigid constraints from/to GSA. RigidConstraints are treated by BHoM as a RigidLink but with a added Fragment that tells GSA that it is a RigCon instead. Previously added unique methods for RigCons have been removed and this is now an adjustment to existing Rigid Link methods.

NOTE: Depends on

Issues addressed by this PR

Closes #296

Test files

https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/GSA_Toolkit/%23297-Pull%20and%20Push%20Constraints?csf=1&web=1&e=fDky6R

Changelog

Additional comments

@TosteSkDa TosteSkDa added the type:feature New capability or enhancement label May 2, 2023
@TosteSkDa TosteSkDa added this to the BHoM 6.2 β MVP milestone May 2, 2023
@TosteSkDa TosteSkDa self-assigned this May 2, 2023
@TosteSkDa TosteSkDa linked an issue May 2, 2023 that may be closed by this pull request
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh left a comment

Choose a reason for hiding this comment

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

See comment below. Think good to have a call to chat through

GSA_oM/Elements/RigidConstraint.cs Show resolved Hide resolved
@peterjamesnugent
Copy link
Member

peterjamesnugent commented Sep 26, 2023

I have reviewed using the test script for 8.7 and 10.1, the Read works fine when the RigidLink (both Links and Cosntraints) have 1 node : 1 node relationships - the push also works for both.

However, when pushing a RigidLink (that is a GSA Link) with 1:n relationship, it needs to create several Rigid Links with the same name - this is problematic when pulling back as the Nodes are not pulled correctly.

Appears that the issue is pushing a RigidLink with 1 : n relationship, as it results as duplicates:
image

This has been fixed with my changes.

Copy link
Member

@peterjamesnugent peterjamesnugent left a comment

Choose a reason for hiding this comment

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

I have tested using the test script for both GSA 10.7 and 8.1 - I have had to made two minor changes for it to function correctly (and to minimise altering the intent of the original PR):

  • Change the delimiter from ',' to ' ' when pulling RigidConstraints from GSA that have a 1:many relationship
  • Change how secondary nodes are handled for pushing RigidLinks to GSA

The one issue I do have is pulling Nodes from a model, using them to create a RigidLink, creating a New File and then pushing the RigidLink. The Nodes are not assigned properly - this may be because of the GSAId not being able to force a number - outside the scope of this PR and not sure whether that is a desire from a user point of view.

I would suggest one other reviews this PR for completeness.

johannaisak and others added 7 commits September 27, 2023 16:40
…res_RigidLinks.

Rigid Constraints are now treated by BHoM as a Rigid Link object but with a fragment that if true tells GSA it is a Rigid Constraints. Removed previously added RigidConstraint Type and ENUM from this PR and instead adjusted the RigidLink methods. Also added more possible LinkConstraint options to the FromGsa - LinkConstraint method.
@peterjamesnugent peterjamesnugent force-pushed the GSA_Toolkit-#296-pull-and-push-constraints branch from ff83d23 to a0e4ee9 Compare September 27, 2023 15:41
@Artem-Holstov
Copy link

Review summary: No new issues found in tests. Rigid links and rigid constraints push and pull functions work as expected.

Comments: GSA8.7 does not start automatically when the adapter is activated. The start model needs to be open in GSA separately before activating the adapter. In the initial tests, pushed rigid constraints did not appear in GSA. After rerunning the same process, the push function worked as expected. GSA8.7 was generally very unstable and prone to crashing in the tests. Hence, the initial glitch with pushing the RCs may be attributed to GSA8.7 and not the updated BHoM code. None of the above issues were observed with GSA 10.1.

Features tested:
Pull existing -> push/update -> pull updated functions tested for rigid links and rigid constraints with GSA 10.1 and 8.7 using test script.
Pushed and pulled data was cross-checked between GH and GSA and logged in Excel.
Note the location of rigid links in the starting models was different between the files tested in GSA10.1 and GSA8.7. The script functions as expected with both starting models.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check compliance
@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Oct 3, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check branch-compliance
  • check dataset-compliance
  • check copyright-compliance
  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Oct 3, 2023

The check code-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Oct 3, 2023

The check documentation-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@bhombot-ci
Copy link

bhombot-ci bot commented Oct 3, 2023

The check project-compliance has already been run previously and recorded as a successful check. This check has not been run again at this time.

@FraserGreenroyd FraserGreenroyd dismissed IsakNaslundBh’s stale review October 3, 2023 14:20

Isak's not around to dismiss his own review, Peter and Artem have handled it.

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Oct 3, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check ready-to-merge

Copy link
Contributor

@FraserGreenroyd FraserGreenroyd left a comment

Choose a reason for hiding this comment

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

Approving based on @peterjamesnugent and @Artem-Holstov reviews.

GSA_oM/Elements/RigidConstraint.cs Show resolved Hide resolved
@FraserGreenroyd
Copy link
Contributor

@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Oct 3, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check ready-to-merge

@FraserGreenroyd FraserGreenroyd merged commit 2b9ac30 into develop Oct 3, 2023
12 checks passed
@FraserGreenroyd FraserGreenroyd deleted the GSA_Toolkit-#296-pull-and-push-constraints branch October 3, 2023 14:24
@bhombot-ci bhombot-ci bot mentioned this pull request Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GSA_Toolkit: Pull and Push Constraints
6 participants