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

Improve and fix buildsystem #3

Merged
merged 11 commits into from
Aug 26, 2017

Conversation

jcfr
Copy link
Collaborator

@jcfr jcfr commented Aug 24, 2017

No description provided.

jcfr added 10 commits August 24, 2017 02:04
Version required by PortPlacement now matches the version required by Slicer
Since Eigen3 now provides a config file and is available through the
extension index, this commit removes the local FindEigen3 module and
explicitly makes use of the CONFIG mode (this avoid attempt to use
FindEigen3.cmake provided by VTK).
This commit partially reverts 869bfd5 (ENH: Remove nlopt source tree
from PortPlacement code base) and download NLopt to fix allow the extension
to build on Linux, macOS and Windows.
This commit allows other extensions to reuse PortPlacement objects.
This commit fixes the following error:

```
from /tmp/PortPlacement/AutoPortPlacement/qSlicerAutoPortPlacementModule.cxx:19:
/tmp/PortPlacement/AutoPortPlacement/qSlicerAutoPortPlacementModule.cxx:29:1: error: static assertion failed: Old plugin system used
 Q_EXPORT_PLUGIN2(qSlicerAutoPortPlacementModule, qSlicerAutoPortPlacementModule);
 ^
```
This commit fixes the following runtime error:

```
2:   File "/tmp/PortPlacement/PortPlacement/PortPlacement.py", line 238, in onRemovePortButton
2:     index = self.portsTable.selectionModel().currentIndex()
2: TypeError: 'QModelIndex' object is not callable
```

```
2: Traceback (most recent call last):
2:   File "/tmp/PortPlacement/PortPlacement/PortPlacement.py", line 735, in test_PortPlacement1
2:     logic.toolList[i].modelNode.TransformPointFromWorld(targetWorld, targetLocal)
2: TypeError: TransformPointFromWorld argument 1: expected a sequence of 3 values, got 4 values
```
This commit fixes warnings like the following:

```
/tmp/PortPlacement/AutoPortPlacement/Logic/tests/min-distance-test.cxx:5:14: warning: unused parameter ‘argc’ [-Wunused-parameter]
 int main(int argc, char* argv[])
              ^
/tmp/PortPlacement/AutoPortPlacement/Logic/tests/min-distance-test.cxx:5:31: warning: unused parameter ‘argv’ [-Wunused-parameter]
 int main(int argc, char* argv[])
                               ^

```
This commit fixes the following warning:

```
tmp/PortPlacement/AutoPortPlacement/Logic/davinci-kinematics/davinci.cxx:726:10: warning: ‘double {anonymous}::fromPoseToExtraClearance(const DavinciKinematics&, const Matrix4d&, const Matrix4d&, const Matrix4d&)’ defined but not used [-Wunused-function]
   double fromPoseToExtraClearance(const DavinciKinematics& kin,
          ^
```
@jcfr
Copy link
Collaborator Author

jcfr commented Aug 24, 2017

Cc: @andinet

xref Slicer/ExtensionsIndex#1455

@jcfr
Copy link
Collaborator Author

jcfr commented Aug 24, 2017

All tests pass. 🎆

Only issue is that optim_find_feasible_plan_test complete after a long time (300s) which corresponds to the timeout set in function Optim::findFeasiblePlan found in AutoPortPlacement/Logic/optim/optim.cxx#L161-L162.

I wonder if it would make sense to add the timeout as a parameter of Optim::findFeasiblePlan and set a smaller timeout for the test .. I also wonder if heuristics checking if convergence is reached could instead be improved

Test project /tmp/PortPlacement-dbg/inner-build
      Start  1: py_nomainwindow_qSlicerPortPlacementModuleGenericTest
 1/10 Test  #1: py_nomainwindow_qSlicerPortPlacementModuleGenericTest ...   Passed    2.90 sec
      Start  2: py_PortPlacement
 2/10 Test  #2: py_PortPlacement ........................................   Passed    6.65 sec
      Start  3: qSlicerAutoPortPlacementModuleGenericTest
 3/10 Test  #3: qSlicerAutoPortPlacementModuleGenericTest ...............   Passed    1.94 sec
      Start  4: qSlicerAutoPortPlacementModuleWidgetGenericTest
 4/10 Test  #4: qSlicerAutoPortPlacementModuleWidgetGenericTest .........   Passed    1.95 sec
      Start  5: da_vinci_load_params_test
 5/10 Test  #5: da_vinci_load_params_test ...............................   Passed    0.00 sec
      Start  6: da_vinci_intra_ik_test
 6/10 Test  #6: da_vinci_intra_ik_test ..................................   Passed    0.00 sec
      Start  7: da_vinci_jacobian_test
 7/10 Test  #7: da_vinci_jacobian_test ..................................   Passed    0.00 sec
      Start  8: da_vinci_passive_ik_test
 8/10 Test  #8: da_vinci_passive_ik_test ................................   Passed    0.09 sec
      Start  9: collisions_distance_test
 9/10 Test  #9: collisions_distance_test ................................   Passed    0.00 sec
      Start 10: optim_find_feasible_plan_test
10/10 Test #10: optim_find_feasible_plan_test ...........................   Passed  300.06 sec

@giogadi giogadi merged commit 1a0fc89 into giogadi:master Aug 26, 2017
@jcfr jcfr deleted the improve-and-fix-buildsystem branch August 26, 2017 05:28
@jcfr
Copy link
Collaborator Author

jcfr commented Aug 26, 2017

Thanks @giogadi 👍

Hope things are going well out West

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.

2 participants