Add script for node creation and update templates. #213

destogl · 2024-05-21T07:58:58Z

No description provided.

Nibanovic · 2024-05-21T08:46:50Z

scripts/nodes/setup-ros-node.bash

+    s=${s#*"$delimiter"}
+    CLASS_NAME="$CLASS_NAME${part^}"
+  done
+  echo -e "${TERMINAL_COLOR_USER_CONFIRMATION}ClassName guessed from the '$FILE_NAME': '$CLASS_NAME'. Is this correct? If not, provide it as the second parameter.${TERMINAL_COLOR_NC}"


line 39, it says the FILE_NAME is first parameter, PKG_DEPS is second and CLASS_NAME is only third. Here it says provide it as the second parameter, which does not work.

If i only call
setup-ros-node my_node, PKG_DEPS is "", which is expected

However, if i call
setup-ros-node my_node MyFirstNode, the class name is still being guessed.

Below, I removed the PKG_DEPS argument, as it is not used anywhere.

you are right! I have fixed nos using PKG_DEPS. Can you check again?

The thing is that as we are using positional arguments, you have to provide empty second argument to set the third.

then, if i want to define a special class name, I have to know to that the second parameter should be empty, which seems convoluted.

For example, now running:

setup-ros-node my_node MyFirstNode

places <depend>MyFirstNode</depend> in the package.xml.

In my opinion, this script is trying to do too much.

This script should generate the required files and boilerplate for CMakeLists.txt and ROS parameters. That is the most tedious part, as I see it.

Adding dependancies should be done by the user, manually, as we usually add dependancies anyways while developing the node.

I suggest we remove adding package dependancies except <build_depend>generate_parameter_library</build_depend>, and make the custom node name parameter $2, in the remote case the class name will not correspond to the file name, but CamelCase.

In the spirit of my previous comment, can we remove the is the package already configured? prompt?

when using create-new-package, we are already "configuring the package" by setting up some CMakeLists.txt code.

We can just make this script add dependancy to generate_parameter_library, link the new files and call it a day? This prompt also adds some unecessary complexity, in my opinion.

We can always expand the functionality later, as we use the script.

the functionality about adding the is already configured is when you are adding a second node to already setup package. Not wehn you configure your package with create-new-package.

We can remove adding dependecies, but more the simplicity then chaning the interface. The documentation is clear that if you want to add a custom class (Which is rearly the case to be honest) then you have to provde all three arguemnts. That is how bash works.

On the other hand, I am lazy, and would rather do small fixes later for the dependecies then have to add them manually. We are talking here not about 100% dependecy coverage when you gnerate your file, but more about initial dependecies.

scripts/nodes/setup-ros-node.bash

--------- Co-authored-by: Daniel Azanov <31107191+muritane@users.noreply.github.com> Co-authored-by: Dr. Denis <denis@stoglrobotics.de>

Nibanovic · 2024-05-21T09:47:25Z

scripts/nodes/setup-ros-node.bash

+if [[ "$package_configured" == "no" ]]; then
+
+  # Add `THIS_PACKAGE_INCLUDE_DEPENDS` structure to simplify the rest of the file
+  TEST_LINE=`awk '$1 == "find_package(ament_cmake" { print NR }' CMakeLists.txt`  # get line before `ament_cmake` dependency


head and tail commands are having trouble parsing the expanded string, I'm getting these warnings:

tail: invalid number of lines: ‘+’ head: unrecognized option '--1' Try 'head --help' for more information. tail: invalid number of lines: ‘+’ tail: invalid number of lines: ‘+’ tail: invalid number of lines: ‘+’

This is happening both when $package_configured is no or yes

Is this working as expected on your machine?

What is the initial state of your package when testing this?

Nibanovic · 2024-05-21T09:53:04Z

scripts/nodes/setup-ros-node.bash

+
+# Get line with if(BUILD_TESTING)
+TEST_LINE=`awk '$1 == "if(BUILD_TESTING)" { print NR }' CMakeLists.txt`
+let CUT_LINE=$TEST_LINE-1
+
+# Add Plugin library related stuff
+echo "# Add ${FILE_NAME} library related compile commands" >> $TMP_FILE
+echo "generate_parameter_library(${FILE_NAME}_parameters" >> $TMP_FILE
+echo "  ${NODE_PARAMS_YAML}" >> $TMP_FILE
+echo ")" >> $TMP_FILE
+
+echo "add_executable($FILE_NAME $NODE_CPP)" >> $TMP_FILE
+echo "target_include_directories($FILE_NAME PRIVATE" >> $TMP_FILE
+echo '  "$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>"' >> $TMP_FILE
+echo '  "$<INSTALL_INTERFACE:include/${PROJECT_NAME}>")' >> $TMP_FILE
+echo "target_link_libraries($FILE_NAME ${FILE_NAME}_parameters)" >> $TMP_FILE
+echo "ament_target_dependencies($FILE_NAME \${THIS_PACKAGE_INCLUDE_DEPENDS})" >> $TMP_FILE


I tried adding a my_node in a package where I already have one node set up.

when selecting package_configured = yes, the script deletes my whole CmakeLists.txt and writes only

# Add my_node library related compile commands generate_parameter_library(my_node_parameters src/my_node.yaml ) add_executable(my_node src/my_node.cpp) target_include_directories(my_node PRIVATE "$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>" "$<INSTALL_INTERFACE:include/${PROJECT_NAME}>") target_link_libraries(my_node my_node_parameters) ament_target_dependencies(my_node ${THIS_PACKAGE_INCLUDE_DEPENDS}) ament_export_libraries( my_node )

This is not happening when selecting package_configured = no in the same package. In that case, it adds the above text to the existing CmakeLists.txt

OK, this is a bug then. I didn't tested it at already configured packages. I just copy paseted some thing so we have it.

templates/nodes/dummy_node_params.yaml

templates/nodes/dummy_node.cpp

scripts/nodes/setup-ros-node.bash

Nibanovic

There are issues with editing existing CMakeLists.txt which need to be resolved, mainly when using head and tail commands.

Also, there are some features I think are unecessary.

Nibanovic · 2024-05-24T11:55:28Z

scripts/nodes/setup-ros-node.bash

+  echo "install(" >> $TMP_FILE
+  echo "  TARGETS" >> $TMP_FILE
+  echo "  $FILE_NAME" >> $TMP_FILE
+  echo "  RUNTIME DESTINATION lib/${PROJECT_NAME}" >> $TMP_FILE


single quotes are required here

Suggested change

echo " RUNTIME DESTINATION lib/${PROJECT_NAME}" >> $TMP_FILE

echo ' RUNTIME DESTINATION lib/${PROJECT_NAME}' >> $TMP_FILE

why? I want that ${PROJECT_NAME} is parsed to the string. so this should work

Co-authored-by: Nikola Banović <nibanovic@gmail.com>

Add script for node creation and update templates.

4cb0297

destogl added the enhancement New feature or request label May 21, 2024

destogl self-assigned this May 21, 2024

destogl requested a review from Nibanovic May 21, 2024 07:58

Nibanovic reviewed May 21, 2024

View reviewed changes

destogl commented May 21, 2024

View reviewed changes

scripts/nodes/setup-ros-node.bash Outdated Show resolved Hide resolved

destogl and others added 3 commits May 21, 2024 11:00

Fix using package dependecies input.

8179794

Add additional colcon build commands (#190)

f7b357f

--------- Co-authored-by: Daniel Azanov <31107191+muritane@users.noreply.github.com> Co-authored-by: Dr. Denis <denis@stoglrobotics.de>

Add docs.

772ea3d

Nibanovic reviewed May 21, 2024

View reviewed changes

templates/nodes/dummy_node_params.yaml Show resolved Hide resolved

Nibanovic reviewed May 21, 2024

View reviewed changes

templates/nodes/dummy_node.cpp Outdated Show resolved Hide resolved

Nibanovic reviewed May 21, 2024

View reviewed changes

scripts/nodes/setup-ros-node.bash Outdated Show resolved Hide resolved

Nibanovic reviewed May 21, 2024

View reviewed changes

scripts/nodes/setup-ros-node.bash Outdated Show resolved Hide resolved

Nibanovic requested changes May 21, 2024

View reviewed changes

Nibanovic reviewed May 24, 2024

View reviewed changes

destogl and others added 2 commits June 19, 2024 15:29

Update scripts/nodes/setup-ros-node.bash

bcf4818

Co-authored-by: Nikola Banović <nibanovic@gmail.com>

Update scripts/nodes/setup-ros-node.bash

fcc8caa

Co-authored-by: Nikola Banović <nibanovic@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add script for node creation and update templates. #213

Add script for node creation and update templates. #213

destogl commented May 21, 2024

Nibanovic May 21, 2024

destogl May 21, 2024

Nibanovic May 21, 2024 •

edited

Loading

Nibanovic May 21, 2024

destogl May 22, 2024

destogl May 22, 2024

destogl May 22, 2024

Nibanovic May 21, 2024

destogl May 22, 2024

Nibanovic May 21, 2024 •

edited

Loading

destogl May 22, 2024

Nibanovic left a comment

Nibanovic May 24, 2024

destogl Jun 19, 2024

	echo " RUNTIME DESTINATION lib/${PROJECT_NAME}" >> $TMP_FILE
	echo ' RUNTIME DESTINATION lib/${PROJECT_NAME}' >> $TMP_FILE

Add script for node creation and update templates. #213

Are you sure you want to change the base?

Add script for node creation and update templates. #213

Conversation

destogl commented May 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nibanovic May 21, 2024 • edited Loading

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

Nibanovic May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nibanovic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nibanovic May 21, 2024 •

edited

Loading

Nibanovic May 21, 2024 •

edited

Loading