adding noether_simulator #51

trentwall · 2019-07-31T15:36:41Z

Adding the noether_simulator package required minor modifications to existing packages.

drchrislewis · 2019-07-31T18:44:50Z

noether_examples/src/thickness_simulator_client.cpp

@@ -0,0 +1,182 @@
+#include <ros/ros.h>


To me this looks like a piece of code for testing. At least put in a comment block that describes what it does and how to use it.

drchrislewis · 2019-07-31T18:52:45Z

noether_examples/src/thickness_simulator_client.cpp

+  return points;
+}
+
+int main (int argc, char **argv)


This would be a more compelling demo if it also allowed the user to add a mesh from a file in addition to those meshes you auto-generate.

I will start working on it.

drchrislewis · 2019-07-31T18:55:40Z

noether_simulator/include/noether_simulator/noether_simulator.h

+}
+
+#endif // NOETHER_SIMULATOR_H
+


Nice that you have both doxygen and good naming conventions!

drchrislewis · 2019-07-31T18:56:08Z

noether_simulator/package.xml

+  <name>noether_simulator</name>
+  <version>0.0.0</version>
+  <description>The noether_simulator package</description>
+  <maintainer email="[email protected]">alex</maintainer>


You're not alex goins

drchrislewis · 2019-07-31T18:59:23Z

noether_simulator/src/noether_simulator.cpp

+  void  NoetherSimulator::runSimulation()
+  {
+
+    cout << "starting simulation\n";


Use one of the logging functions instead of cout directly so that output can be turned on and off level by level. (e.g. ROS_ERROR, ROS_INFO, and ROS_DEBUG)

drchrislewis · 2019-07-31T19:05:18Z

noether_simulator/src/noether_simulator.cpp

+    cylinder->SetResolution(20);
+    cylinder->Update();
+
+    //vtkSmartPointer<vtkOBBTree> tree = vtkSmartPointer<vtkOBBTree>::New();


remove commented out lines. You can add a real comment, if you want to remind yourself of the other options

drchrislewis · 2019-07-31T19:06:55Z

noether_simulator/src/noether_simulator.cpp

+        vtkSmartPointer<vtkPoints> test_pts = vtkSmartPointer<vtkPoints>::New();
+        vtkSmartPointer<vtkPoints> integration_pts = vtkSmartPointer<vtkPoints>::New();
+
+        //test_pts->SetNumberOfPoints(2);


more commented out code

drchrislewis · 2019-07-31T19:09:00Z

noether_simulator/src/noether_simulator.cpp

+
+    // TODO: the test currently uses a cylinder source, with the y-axis as the major axis
+    // for correct visualization, need to set the norm vector (u) to the y-axis
+    Eigen::Affine3d epose = Eigen::Affine3d::Identity();


I'm not sure why, but I believe Affine3d is depricated for Isometry3d. I don't know the difference.
I also forgot that you copied this code from an old version created by alex goins.

drchrislewis · 2019-07-31T19:16:27Z

noether_simulator/src/thickness_simulator_node.cpp

+
+    //add tool
+    tool_path_planner::ProcessTool tool;
+    tool.pt_spacing = 0.5;


get all these parameters from the ros parameter server, but use these values as default if not set on ros-p-server

drchrislewis · 2019-07-31T19:18:15Z

tool_path_planner/src/utilities.cpp

@@ -196,10 +196,13 @@ noether_msgs::ToolPathConfig toTppMsg(const tool_path_planner::ProcessTool& tool
  tpp_config_msg.line_spacing = tool_config.line_spacing;
  tpp_config_msg.tool_offset = tool_config.tool_offset;
  tpp_config_msg.intersecting_plane_height = tool_config.intersecting_plane_height;
+  tpp_config_msg.nearest_neighbors = tool_config.nearest_neighbors;//Trent added


// Trent added not necessary since we use git to record all commits

drchrislewis · 2019-07-31T19:19:28Z

vtk_viewer/test/utest.cpp

  vtkSmartPointer<vtkPolyData> cut = vtkSmartPointer<vtkPolyData>::New();
  cut = vtk_viewer::cutMesh(data, points2, false);

  cout << "cutter points: " << cut->GetPoints()->GetNumberOfPoints() << "\n";
  cout << "cutter lines: " << cut->GetNumberOfLines() << "\n";

-
-
  vtk_viewer::VTKViewer viz;
  std::vector<float> color(3);



@trentwall please make suggested changes, and ping me for a final review before merge

I have finished all the changes that you and mat requested.

mpowelson

I saw Chris looked at this, so I did not look too much at how it actually works. I also did not test it. I did note some things about the structure of the package.

mpowelson · 2019-07-31T16:16:31Z

noether_simulator/include/noether_simulator/noether_simulator.h

+#define NOETHER_SIMULATOR_H
+
+/*
+ * Copyright (c) 2016, Southwest Research Institute


Add Apache 2 license like in #45. Here's an example.

/** * Copyright (c) 2016, Southwest Research Institute * @file mesh_segmenter.h * All rights reserved. * @copyright Copyright (c) 2019, Southwest Research Institute * * * @par License * Software License Agreement (Apache License) * @par * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * http://www.apache.org/licenses/LICENSE-2.0 * @par * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ */

@trentwall Add the license to all your headers

mpowelson · 2019-07-31T16:20:22Z

noether_simulator/test/utest.cpp

+  color[2] = 0.2;
+  viz.addPolyNormalsDisplay(paths[i].derivatives, color, scale);
+  }
+  viz.renderDisplay();


This will need to be in a #ifndef NDEBUG. If not, CI will hang and fail. See the unit tests in segmentation or one of the other packages

mpowelson · 2019-07-31T16:22:13Z

tool_path_planner/src/raster_tool_path_planner.cpp

@@ -6,6 +6,7 @@

 #include <limits>
 #include <cmath>
+#include <stdio.h>


Remove this

mpowelson · 2019-07-31T20:04:11Z

noether_simulator/package.xml

@@ -0,0 +1,42 @@
+<?xml version="1.0"?>
+<package>


The rest of noether is package.xml format 2. We should convert this to 2 as well.

mpowelson · 2019-07-31T20:04:24Z

noether_simulator/package.xml

+  <version>0.0.0</version>
+  <description>The noether_simulator package</description>
+  <maintainer email="[email protected]">alex</maintainer>
+  <license>Proprietary</license>


mpowelson · 2019-07-31T20:20:51Z

noether_simulator/CMakeLists.txt

+    genmsg
+    actionlib
+    actionlib_msgs
+)


I know some of this can be removed, I suspect a lot of it. Just taking a guess, after removing the ROS node these can probably be removed.

message_generation mesh_segmenter path_sequence_planner sensor_msgs geometry_msgs roscpp pcl_msgs std_srvs pcl_conversions dynamic_reconfigure genmsg actionlib actionlib_msgs

I see that you need some of them for the tests. In that case you can add them below.

mpowelson · 2019-07-31T20:22:26Z

noether_simulator/CMakeLists.txt

+    ${PCL_LIBRARIES}
+)
+
+#Test


Add test depends here similar to

############# ## Testing ## ############# if(CATKIN_ENABLE_TESTING) find_package(catkin REQUIRED COMPONENTS vtk_viewer) include_directories( include ${catkin_INCLUDE_DIRS} ) catkin_add_gtest(${PROJECT_NAME}-test test/utest.cpp ) target_link_libraries(${PROJECT_NAME}-test ${PROJECT_NAME} ${catkin_LIBRARIES} ) endif()

There's also a TEST_DEPEND flag in the package.xml you can use.

noether_simulator/cfg/NoetherSimulator.cfg

mpowelson · 2019-07-31T20:24:02Z

noether_simulator/CMakeLists.txt

+    actionlib_msgs
+)
+
+generate_messages(


mpowelson · 2019-07-31T20:29:25Z

noether_simulator/package.xml

+  <name>noether_simulator</name>
+  <version>0.0.0</version>
+  <description>The noether_simulator package</description>
+  <maintainer email="[email protected]">alex</maintainer>


This should probably be format 2 package.xml to match everything else.

Maintainer should probably be Jorge

License is definitely Apache 2!

I have pushed all the requested changes.

drchrislewis · 2019-08-02T14:01:34Z

noether_examples/src/thickness_simulator_client.cpp

+ * If the mesh is sufficently painted the bool corasponding to that mesh in the painted[]
+ * is set to true, the mesh is excluded from future iterations, and the path is added to the reuqiredPaths[] unless,
+ * it has already been added.
+ * once all the meshes have been evaluated using the first path, the simulator moves on to the second path.


once should be Once

drchrislewis · 2019-08-02T14:04:14Z

noether_examples/src/thickness_simulator_client.cpp

+ * The simulator evaluates the remaining meshes with the second path and so on until all meshes have the coraspoinding
+ * bool value in painted[] set to true, or all paths have been tried.
+ *
+ * To use roslauch to launch the thick_sim.launch file. Then, in a seperate termial window use rosrun to run the


how about
usage:
roslaunch noether_examples thick_sim.launch & rosrun noether_simuator noether_simulator

Give the user something they can cut and paste and try. Test it to make sure it works

drchrislewis · 2019-08-02T14:16:30Z

noether_examples/src/thickness_simulator_client.cpp

+ *
+*/
+
+//make modified meshes


Use doxygen style comments (e.g. @brief This is a helper function to create test meshes

drchrislewis · 2019-08-02T14:20:40Z

noether_msgs/action/SimulateThickness.action

@@ -0,0 +1,10 @@
+#goal definition - Input Mesh


This should probably be thought of as simulating the work function. One might use Noether for sanding, grinding, power washing, applying paint, chemicals, laser paint removal, sand blasting etc. A similar simulation applies to all. The results should be work_complete rather than painted. Or perhaps coverage would be a better term.

drchrislewis · 2019-08-02T14:26:07Z

noether_simulator/include/noether_simulator/noether_simulator.h

+   */
+  void setTool(tool_path_planner::ProcessTool tool){tool_ = tool;}
+
+  void runSimulation();


Add doxygen to every public function

drchrislewis · 2019-08-02T14:29:06Z

noether_simulator/src/noether_simulator.cpp

+
+  double NoetherSimulator::integral(double pt[3])
+  {
+


why is the integral x^3/3 + z^3/3? Put some comments in here to help us understand what's going on

drchrislewis · 2019-08-02T14:31:49Z

noether_simulator/src/noether_simulator.cpp

+  vtkSmartPointer<vtkMatrix4x4> NoetherSimulator::createMatrix(double pt[3], double norm[3], double derv[3])
+  {
+    // Get the point normal and derivative for creating the 3x3 transform
+    //double* norm = paths[j].line->GetPointData()->GetNormals()->GetTuple(k);


more commented out code. Remove it.

drchrislewis · 2019-08-02T14:40:13Z

noether_simulator/src/thickness_simulator_node.cpp

+    derivatives_->GetPointData()->SetNormals(deriv_normals);
+    path.derivatives = derivatives_;
+
+    vtkSmartPointer<vtkPolyData> inter_ = vtkSmartPointer<vtkPolyData>::New();


just adding empty pointers as place holders?

drchrislewis · 2019-08-02T14:42:14Z

noether_simulator/src/thickness_simulator_node.cpp

+    nh_.param("/noether_simulator/pt_spacing",tool.pt_spacing, 0.5);
+    nh_.param("/nnoether_simulator/line_spacing",tool.line_spacing,0.75);
+    nh_.param("/noether_simulator/tool_offset",tool.tool_offset, 0.0);
+    nh_.param("/noether_simulator/intersecting_plane_hiehgt",tool.intersecting_plane_height,0.15);


@drchrislewis I have made the requested changes.

hiehgt is not a correct spelling

drchrislewis

@trentwall Just a few more comments to address. Ping when done

drchrislewis · 2019-08-02T19:35:44Z

noether_msgs/action/SimulateThickness.action

@@ -3,7 +3,7 @@ pcl_msgs/PolygonMesh[] input_mesh
 noether_msgs/ToolRasterPath[] path
 ---
 #result definition - are messhes painted enough


change name of action to simulateCoverage.action

drchrislewis · 2019-08-02T19:37:49Z

noether_simulator/src/noether_simulator.cpp

@@ -50,15 +50,17 @@ namespace noether_simulator
    simulation_points_ = vtkSmartPointer<vtkPolyData>::New();
  }

+  //Integrate in x^2 and z^2


This is too terse to remember what's going on.
This is a definite integral of the function (x^2 + z^2)/10 from one point pt1 to another point pt2.
The factor of one 10th stems from the subsampling of the surface at a 10x density.

twall added 2 commits July 31, 2019 10:24

adding noether_simulator

86a7dac

fixing stylistic errors.

5f2227e

drchrislewis reviewed Jul 31, 2019

View reviewed changes

mpowelson suggested changes Jul 31, 2019

View reviewed changes

twall added 3 commits August 1, 2019 08:57

More stylisitic updates

af2599a

making more stylistic changes

d2a1a41

finishing up stylistic changes.

6690365

drchrislewis reviewed Aug 2, 2019

View reviewed changes

more stylistic changes

8330092

drchrislewis reviewed Aug 2, 2019

View reviewed changes

twall added 6 commits August 2, 2019 15:02

small fixes

bebc44e

added comments to noether_simulator.cpp to make it more understandable.x

dbdc80e

changed cylinder to cone and added display for paths

43c9702

changed example and test meshes to be rectangular instead of square

49aa6bf

updated node names in covage_sim.launch

0a9f437

code for demo

0c9981f

		}

		#endif // NOETHER_SIMULATOR_H

adding noether_simulator #51

Are you sure you want to change the base?

adding noether_simulator #51

Conversation

trentwall commented Jul 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpowelson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drchrislewis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drchrislewis Aug 2, 2019 • edited Loading

Choose a reason for hiding this comment

drchrislewis Aug 2, 2019 •

edited

Loading