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

Feature/optimalcontrol #399

Merged
merged 2 commits into from
Dec 7, 2017
Merged

Conversation

S-Dafarra
Copy link
Contributor

First skeleton of a optimal control library (very skinny for the time being).

This branch is on top of improvements/spline, for which a PR is already opened #398

@traversaro traversaro changed the base branch from devel to improvements/spline December 6, 2017 15:12
@traversaro
Copy link
Member

If we plan to keep this library "unstable" (i.e. have the API of the library to change without deprecations) we should add this classes to iDynTreeExperimental, as in

.
Check also the codacy comment: #399 (comment) .

@S-Dafarra
Copy link
Contributor Author

Regarding the Codacy comment #399 (comment) I don't understand completely why it complains. Isn't it fine to use the operator ++ with an iterator?

Regarding the iDynTreeExperimental tag, is there a way to do it automatically for the whole directory?

@traversaro
Copy link
Member

There are also a few warnings that may be worth checking: https://travis-ci.org/robotology/idyntree/jobs/312456782#L7303 .

@traversaro
Copy link
Member

Fixed some CMake problems with the tests in 53db94c .


# Dependencies
# IPOPT
# SUNDIALS (?)
Copy link
Member

Choose a reason for hiding this comment

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

I guess in the end you went for implementing your own integrator, we can remove this comments.

target_link_libraries(ConstraintsGroupTest PRIVATE idyntree-optimalcontrol ${iDynTree_LIBRARIES})


add_test(NAME ConstraintsGroupTest
Copy link
Member

Choose a reason for hiding this comment

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

Again a low priority comment, but we should follow the same scheme of the tests of the other components and also enabled Valgrind-based tests.

@robotology robotology deleted a comment Dec 6, 2017
@traversaro
Copy link
Member

Regarding the iDynTreeExperimental tag, is there a way to do it automatically for the whole directory?

Unfortunately no, but the basic idea is that every class belongs to a doxygen group, so eventually that part could be switched to a \ingroup iDynTreeOptimalControl.

@traversaro
Copy link
Member

Regarding the Codacy comment #399 (comment) I don't understand completely why it complains. Isn't it fine to use the operator ++ with an iterator?**

For objects that are not integers or similarly simple objects, it is better (even is typically the difference is negligible) to use the prefix ++ in place of the postfix ++, see http://www.idinews.com/peeves/prefixIncr.html .

@traversaro
Copy link
Member

The only failure is related to robotology/yarp#1486 , merging. The rest of open issues are stored in #400 .

@traversaro traversaro merged commit 42782a6 into improvements/spline Dec 7, 2017
@traversaro traversaro deleted the feature/optimalcontrol branch December 7, 2017 08:10
@traversaro traversaro restored the feature/optimalcontrol branch December 7, 2017 08:10
traversaro added a commit that referenced this pull request Dec 7, 2017
@traversaro traversaro deleted the feature/optimalcontrol branch December 7, 2017 08:15
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.

3 participants