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

[constraint] adds JointConstraint #274

Merged
merged 10 commits into from
Jun 4, 2024
Merged

[constraint] adds JointConstraint #274

merged 10 commits into from
Jun 4, 2024

Conversation

EulalieCoevoet
Copy link
Member

@EulalieCoevoet EulalieCoevoet commented Apr 9, 2024

Thank you @TanguyNav for this work!

Proposition of changes:

  • Instead of maxPositiveDisp / maxNegativeDisp, I would prefer using maxDisplacement / minDisplacement. When we want a positive minimum it's really weird to give a negative value. Even in the general case I find it confusing (I don't like it either in CableConstraint...).
  • For me the references to both displacement and angle are confusing. It's true that the joint does not have to be angular and could be a linear displacement. I would remove all the references to angle.
  • From a user point of view (from the python script) we use the data names value and valueType in the other constraints (CableConstraint, SurfacePressureConstraint, PositionConstraint). I would prefer to keep it uniform and thus avoid using imposedValue instead of value.

olivier-roussel and others added 8 commits July 26, 2023 17:03
* Update CI to build v23.06 branch.

* Disable MacOS CI build for now.
* Fix install SofaPython3 CI action.

Use non-nightly SofaPython3 builds for CI.

* Allow macOS CI build but do not stop on deploy failure.
…ource

Fix source code archives assets from release branch and not master.
@EulalieCoevoet EulalieCoevoet changed the title [constraint] adds JointConstraint [WIP][constraint] adds JointConstraint Apr 9, 2024
Copy link
Member

@alxbilger alxbilger left a comment

Choose a reason for hiding this comment

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

It would be great to have an example scene.

@EulalieCoevoet
Copy link
Member Author

Well if no one disagree I'll make the changes I proposed.

- changes maxPositiveDisp/maxNegativeDisp to maxDisplacement/minDisplacement
- removes references to angle
- renames imposedValue to value
- updates example wrt changes
@EulalieCoevoet EulalieCoevoet changed the title [WIP][constraint] adds JointConstraint [constraint] adds JointConstraint Apr 30, 2024
@EulalieCoevoet
Copy link
Member Author

This is good for me. I'll merge it in two weeks, unless you need more time to look at it @TanguyNav @alxbilger ?

@EulalieCoevoet EulalieCoevoet requested a review from alxbilger May 16, 2024 16:18
@alxbilger alxbilger merged commit 5f7b614 into master Jun 4, 2024
7 of 8 checks passed

def createScene(rootNode):
rootNode.addObject('RequiredPlugin', name='SoftRobots')
rootNode.addObject('RequiredPlugin', name='SoftRobots.Inverse')
Copy link
Member

Choose a reason for hiding this comment

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

@EulalieCoevoet Could you make sure that this plugin is not required? Thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

Oups sorry. Not needed indeed.

@EulalieCoevoet EulalieCoevoet deleted the JointConstraint_v23.06 branch June 19, 2024 14:01
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.

4 participants