search

ocra-recipes / ocra-wbi-plugins

Controller implementations and plugins for communicating between the whole body controller libraries developed at ISIR, ocra-recipes, and the iCub Whole Body Interface, WBI, libraries.
GNU General Public License v3.0
3 stars 3 forks source link

Personalized tasks and sequences, also updated the LoadSequence function. #2

Closed alexandrelheinen closed 9 years ago

alexandrelheinen commented 9 years ago

I've made some changes on the project because, when compiling CoDyCo Superbuild, ISIR_MODULES weren't compiling successfully due to errors like note: candidate expects 3 arguments, 0 provided. Furthermore, I also changed the LoadSequence function on sequenceLibrary.cpp to avoid the if/else chain.

To some tests I "improved" FixedBaseMinimalTask and consequently add some auxiliary functions to sequenceTools, but I think it wouldn't be useful to merge it into the main project.

rlober commented 9 years ago

Alex,

First off, thank you for playing with the software and trying to contribute, it will only help make it more robust. Unfortunately, your pull request is full of issues and I can't merge it until they are resolved. In fact I would suggest deleting this pull request and starting over once your local git is clean.

  1. When you get compile errors such as those in your comment, first make sure your repo is up to date. and then create an issue with the specific error, so it can be fixed and so that the fix is logged.
  2. Make sure to commit only the files you have changed. If you do git add . then you may commit a bunch of junk. This clutters up the commit. An example is all of the whitespace in your commits.
  3. Always pull and merge locally before creating a pull request. I see a lot of your files are missing changes that have been added.
  4. The Minimal tasks are designed to have the fewest necessary tasks for startup. You should not add to these. If you wish to add functionality, make a new sequence and add it to the library.
  5. If you want to make changes to fundamental structures such as the loadSequence() function please open an issue so that we can discuss. One reason we did not use a map initially was because it doesn't solve the problem of having to hard code a one to one string-constructor relationship, and actually requires more functions, when a simple if else chain does the same job in a very clear manner. Open an issue for this and we can discuss pros and cons.
  6. Finally, if you make changes that are specific to your personal machine, be sure to exclude them from your commits. The remote repo needs to remain agnostic. If you don't think something is useful then do not put it in the pull request.

Come see me if you have any questions.