search

ethz-asl / curves

A library of curves for estimation.
BSD 3-Clause "New" or "Revised" License
74 stars 28 forks source link

Feature/curvebase refactor #48

Closed mikebosse closed 9 years ago

mikebosse commented 9 years ago

significant refactor of the curves; mostly to make curve coefficients private to derived curve classes, and refine integration with GTSAM.

furgalep commented 9 years ago

I love seeing all that deleted code!

LGTM

furgalep commented 9 years ago

test this please

furgalep commented 9 years ago

test this please

furgalep commented 9 years ago

@rdube @gawela, this now fails legitimately on the server. Can you

  1. Fix the failure
  2. Clean up this PR (remove dead code or renable unit tests)
  3. Address the many warnings (things like /Users/slynen/workspace/curves/label/osx/src/curves/test/test_Expressions.cpp:106:18: warning: comparison of integers of different signs: 'int' and 'size_type' (aka 'unsigned long') [-Wsign-compare] for(int i=0; i < measurements.size(); i++) {)

Then I'll go through and hopefully merge later today!

Great stuff!

rdube commented 9 years ago

Please note that in order to have the unit tests working on jenkins for that PR, we would need to (1) specify in the configurations that this package depends on minkindr branch feature/cubic_hermite or (2) wait until that minkindr branch can is pulled into master.

furgalep commented 9 years ago

OK! @rdube, do you have time to address my comments in https://github.com/ethz-asl/minkindr/pull/20? You can ignore the stuff about needing jacobians with respect to the double scale parameters for now.