RethinkRobotics-opensource / gennodejs

ROS JavaScript message definition and serialization generators
Apache License 2.0
7 stars 21 forks source link

Fixes gennodejs in install space #7

Closed chris-smith closed 8 years ago

chris-smith commented 8 years ago

This should fix operation of gennodejs from install space. I ran an install build with gennodejs in one workspace, created an overlay for that workspace with some message packages and built (devel and install) without issues. This should also solve problems using catkin tools with --link-devel because the js files provided by gennodejs are copied into each module. In my overlaid workspace I ran rosjs' test scripts and example without issue. This resolves the CMake Error in #8 .

IanTheEngineer commented 8 years ago

Interesting Very clever solution here. Thanks for the quick response, Chris.

IanTheEngineer commented 8 years ago

Is it necessary to copy all of the generic JS files into the packages in the install space too?

~/dev/gennodejs_tests/install$ find share/ -iname *.js
share/gennodejs/find.js
share/gennodejs/base_serialize.js
share/gennodejs/base_deserialize.js
share/gennodejs/ros/std_srvs/find.js
share/gennodejs/ros/std_srvs/srv/_index.js
share/gennodejs/ros/std_srvs/srv/SetBool.js
share/gennodejs/ros/std_srvs/srv/Empty.js
share/gennodejs/ros/std_srvs/srv/Trigger.js
share/gennodejs/ros/std_srvs/_index.js
share/gennodejs/ros/std_srvs/base_serialize.js
share/gennodejs/ros/std_srvs/base_deserialize.js
share/gennodejs/ros/test_msgs/find.js
share/gennodejs/ros/test_msgs/srv/TestService.js
share/gennodejs/ros/test_msgs/srv/BasicService.js
share/gennodejs/ros/test_msgs/srv/_index.js
share/gennodejs/ros/test_msgs/srv/HeaderService.js
share/gennodejs/ros/test_msgs/msg/VariableLengthArray.js
share/gennodejs/ros/test_msgs/msg/BaseTypeVariableLengthArray.js
share/gennodejs/ros/test_msgs/msg/StdMsg.js
share/gennodejs/ros/test_msgs/msg/TestActionActionFeedback.js
share/gennodejs/ros/test_msgs/msg/TestActionActionResult.js
share/gennodejs/ros/test_msgs/msg/TestActionActionGoal.js
share/gennodejs/ros/test_msgs/msg/_index.js
share/gennodejs/ros/test_msgs/msg/ConstantLengthArray.js
share/gennodejs/ros/test_msgs/msg/TestActionResult.js
share/gennodejs/ros/test_msgs/msg/BaseType.js
share/gennodejs/ros/test_msgs/msg/TestActionAction.js
share/gennodejs/ros/test_msgs/msg/TestActionFeedback.js
share/gennodejs/ros/test_msgs/msg/BaseTypeConstantLengthArray.js
share/gennodejs/ros/test_msgs/msg/TestActionGoal.js
share/gennodejs/ros/test_msgs/_index.js
share/gennodejs/ros/test_msgs/base_serialize.js
share/gennodejs/ros/test_msgs/base_deserialize.js
share/gennodejs/ros/std_msgs/find.js
share/gennodejs/ros/std_msgs/msg/UInt64.js
share/gennodejs/ros/std_msgs/msg/Int16.js
share/gennodejs/ros/std_msgs/msg/Int64.js
share/gennodejs/ros/std_msgs/msg/String.js
share/gennodejs/ros/std_msgs/msg/UInt32MultiArray.js
share/gennodejs/ros/std_msgs/msg/UInt8.js
share/gennodejs/ros/std_msgs/msg/Float32.js
share/gennodejs/ros/std_msgs/msg/UInt64MultiArray.js
share/gennodejs/ros/std_msgs/msg/MultiArrayDimension.js
share/gennodejs/ros/std_msgs/msg/Int64MultiArray.js
share/gennodejs/ros/std_msgs/msg/Byte.js
share/gennodejs/ros/std_msgs/msg/_index.js
share/gennodejs/ros/std_msgs/msg/ColorRGBA.js
share/gennodejs/ros/std_msgs/msg/UInt32.js
share/gennodejs/ros/std_msgs/msg/ByteMultiArray.js
share/gennodejs/ros/std_msgs/msg/UInt8MultiArray.js
share/gennodejs/ros/std_msgs/msg/UInt16.js
share/gennodejs/ros/std_msgs/msg/MultiArrayLayout.js
share/gennodejs/ros/std_msgs/msg/Bool.js
share/gennodejs/ros/std_msgs/msg/Char.js
share/gennodejs/ros/std_msgs/msg/UInt16MultiArray.js
share/gennodejs/ros/std_msgs/msg/Time.js
share/gennodejs/ros/std_msgs/msg/Empty.js
share/gennodejs/ros/std_msgs/msg/Int32.js
share/gennodejs/ros/std_msgs/msg/Float32MultiArray.js
share/gennodejs/ros/std_msgs/msg/Int8MultiArray.js
share/gennodejs/ros/std_msgs/msg/Float64.js
share/gennodejs/ros/std_msgs/msg/Int32MultiArray.js
share/gennodejs/ros/std_msgs/msg/Duration.js
share/gennodejs/ros/std_msgs/msg/Int16MultiArray.js
share/gennodejs/ros/std_msgs/msg/Float64MultiArray.js
share/gennodejs/ros/std_msgs/msg/Int8.js
share/gennodejs/ros/std_msgs/msg/Header.js
share/gennodejs/ros/std_msgs/_index.js
share/gennodejs/ros/std_msgs/base_serialize.js
share/gennodejs/ros/std_msgs/base_deserialize.js
chris-smith commented 8 years ago

In the _generate_module_nodejs macro, we could probably copy the files a few directories back e.g. file(COPY ${base_files} DESTINATION "${ARG_GEN_OUTPUT_DIR}/../.."). I originally did this because of the linked devel space catkin tools creates but the structure used in the linked private space is equivalent, it just may be more brittle to catkin tools changes

IanTheEngineer commented 8 years ago

I think I'm coming around to the per-package idea. It creates 3x copies per message/service package, which isn't too bad (it's certainly better than the number of extra files genlisp constructs). If there was a standard structure that we could rely on, obviously that would be the better solution, but that is seeming hard to determine with the recent catkin_tools changes.

chris-smith commented 8 years ago

I have done all these, but if you could also test out these scenarios that would be excellent.

  1. With gennodejs in your workspace, does it generate usable messages for development builds (linked and merged devel space)
  2. With gennodejs in your workspace, does it generate usable messages for install builds
  3. In a workspace overlaying an installed gennodejs, does it generate usable messages for development builds (linked and merged)
  4. In a workspace overlaying an installed gennodejs, does it generate usable messages for install builds.

In rosnodejs, you'll also need to change jsMsgPath in utils/message_utils.js to

let jsMsgPath = path.join('share', 'gennodejs', 'ros');

IanTheEngineer commented 8 years ago

Ok, I've run all of these tests, including one additional where I patched my install of gennodejs in ROS Kinetic. Everything builds and works fine in each of these instances including my additional test.