teamspatzenhirn / rig_reconfigure

GUI tool for editing ROS 2 parameters at runtime
MIT License
79 stars 8 forks source link

Replace linear node list with tree representation #34

Closed authaldo closed 7 months ago

authaldo commented 7 months ago

Changes:

Requires a bit more testing before it can be merged

ottojo commented 7 months ago

testing notes:

details ``` terminate called after throwing an instance of 'std::out_of_range' what(): basic_string::substr: __pos (which is 25) > this->size() (which is 24) Process finished with exit code 134 (interrupted by signal 6:SIGABRT) ``` backtrace: ``` (gdb) bt #0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=140737330390656) at ./nptl/pthread_kill.c:44 #1 __pthread_kill_internal (signo=6, threadid=140737330390656) at ./nptl/pthread_kill.c:78 #2 __GI___pthread_kill (threadid=140737330390656, signo=signo@entry=6) at ./nptl/pthread_kill.c:89 #3 0x00007ffff6d3f476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 #4 0x00007ffff6d257f3 in __GI_abort () at ./stdlib/abort.c:79 #5 0x00007ffff70d1b9e in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #6 0x00007ffff70dd20c in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #7 0x00007ffff70dd277 in std::terminate() () from /lib/x86_64-linux-gnu/libstdc++.so.6 #8 0x00007ffff70dd4d8 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6 #9 0x00007ffff70d44cd in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #10 0x00005555556a128b in std::__cxx11::basic_string, std::allocator >::_M_check (this=0x5555562f5530, __pos=25, __s=0x5555557fbd13 "basic_string::substr") at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:390 #11 0x00005555556a9d7b in std::__cxx11::basic_string, std::allocator >::substr (this=0x5555562f5530, __pos=25, __n=18446744073709551615) at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:3132 #12 0x000055555570c0ef in NodeTree::addNode (this=0x7fffffffd2f8, curNode=std::shared_ptr (use count 2, weak count 0) = {...}, name="local_trajectory_planner/preprocessing", fullName="/control/planning/local_trajectory_planner/preprocessing") at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:138 #13 0x000055555570c2e7 in NodeTree::addNode (this=0x7fffffffd2f8, curNode=std::shared_ptr (use count 2, weak count 0) = {...}, name="planning/local_trajectory_planner/preprocessing", fullName="/control/planning/local_trajectory_planner/preprocessing") at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:146 #14 0x000055555570c2e7 in NodeTree::addNode (this=0x7fffffffd2f8, curNode=std::shared_ptr (use count 1, weak count 0) = {...}, name="control/planning/local_trajectory_planner/preprocessing", fullName="/control/planning/local_trajectory_planner/preprocessing") at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:146 #15 0x000055555570bbf6 in NodeTree::NodeTree (this=0x7fffffffd2f8, nodes=std::vector of length 9, capacity 9 = {...}) at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:91 #16 0x000055555570b6d2 in renderNodeWindow (windowName=0x5555557fda83 "Nodes", nodeNames=std::vector of length 9, capacity 9 = {...}, serviceWrapper=..., selectedNode="", status=...) at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:66 #17 0x0000555555698a3d in main (argc=1, argv=0x7fffffffe048) at /home/jonas/workspace/rig_reconfigure/src/rig_reconfigure.cpp:293 ```

also happens if node name is only part of namespace of other node (node /control/planning/local_trajectory_planner and /control/planning/local_trajectory_planner/utilities/preprocessing):

backtrace ``` (gdb) bt #0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=140737330390656) at ./nptl/pthread_kill.c:44 #1 __pthread_kill_internal (signo=6, threadid=140737330390656) at ./nptl/pthread_kill.c:78 #2 __GI___pthread_kill (threadid=140737330390656, signo=signo@entry=6) at ./nptl/pthread_kill.c:89 #3 0x00007ffff6d3f476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 #4 0x00007ffff6d257f3 in __GI_abort () at ./stdlib/abort.c:79 #5 0x00007ffff70d1b9e in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #6 0x00007ffff70dd20c in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #7 0x00007ffff70dd277 in std::terminate() () from /lib/x86_64-linux-gnu/libstdc++.so.6 #8 0x00007ffff70dd4d8 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6 #9 0x00007ffff70d44cd in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #10 0x00005555556a128b in std::__cxx11::basic_string, std::allocator >::_M_check (this=0x555556321ba0, __pos=25, __s=0x5555557fbd13 "basic_string::substr") at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:390 #11 0x00005555556a9d7b in std::__cxx11::basic_string, std::allocator >::substr (this=0x555556321ba0, __pos=25, __n=18446744073709551615) at /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:3132 #12 0x000055555570c0ef in NodeTree::addNode (this=0x7fffffffd348, curNode=std::shared_ptr (use count 2, weak count 0) = {...}, name="local_trajectory_planner/utilities/preprocessing", fullName="/control/planning/local_trajectory_planner/utilities/preprocessing") at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:138 #13 0x000055555570c2e7 in NodeTree::addNode (this=0x7fffffffd348, curNode=std::shared_ptr (use count 2, weak count 0) = {...}, name="planning/local_trajectory_planner/utilities/preprocessing", fullName="/control/planning/local_trajectory_planner/utilities/preprocessing") at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:146 #14 0x000055555570c2e7 in NodeTree::addNode (this=0x7fffffffd348, curNode=std::shared_ptr (use count 1, weak count 0) = {...}, name="control/planning/local_trajectory_planner/utilities/preprocessing", fullName="/control/planning/local_trajectory_planner/utilities/preprocessing") at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:146 #15 0x000055555570bbf6 in NodeTree::NodeTree (this=0x7fffffffd348, nodes=std::vector of length 9, capacity 9 = {...}) at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:91 #16 0x000055555570b6d2 in renderNodeWindow (windowName=0x5555557fda83 "Nodes", nodeNames=std::vector of length 9, capacity 9 = {...}, serviceWrapper=..., selectedNode="", status=...) at /home/jonas/workspace/rig_reconfigure/src/node_window.cpp:66 #17 0x0000555555698a3d in main (argc=1, argv=0x7fffffffe098) at /home/jonas/workspace/rig_reconfigure/src/rig_reconfigure.cpp:293 #18 0x00007ffff6d26d90 in __libc_start_call_main (main=main@entry=0x555555697180 , argc=argc@entry=1, argv=argv@entry=0x7fffffffe098) at ../sysdeps/nptl/libc_start_call_main.h:58 #19 0x00007ffff6d26e40 in __libc_start_main_impl (main=0x555555697180 , argc=1, argv=0x7fffffffe098, init=, fini=, rtld_fini=, stack_end=0x7fffffffe088) at ../csu/libc-start.c:392 #20 0x00005555556970b5 in _start () ```
ottojo commented 7 months ago

Proposed fix for dead nodes not dissapearing:

diff --git a/src/service_wrapper.cpp b/src/service_wrapper.cpp
index 992846f..a49f604 100644
--- a/src/service_wrapper.cpp
+++ b/src/service_wrapper.cpp
@@ -126,6 +148,20 @@ void ServiceWrapper::handleRequest(const RequestPtr &request) {
                     continue;
                 }
+
+                {
+                    auto tmpclient = node->create_client<rcl_interfaces::srv::ListParameters>(serviceName);
+                    if (!tmpclient->service_is_ready()) {
+                        // Service is known, but not ready.
+                        // This happens e.g. if this is the currently selected node,
+                        // so we still have clients for the service, but the node has died.
+                        continue;
+                    }
+                }
+
+
                 nodeNames.push_back(extractedNodeName);
             }
ottojo commented 7 months ago

proposed fix for name prefix crash in #35

authaldo commented 7 months ago

I like the highlighting and service availability checking, thanks for that!

Everything seems to be working in my small test example, thus, I'll remove the draft state of the MR.

I would opt for adding additional functionality like "expand" / "collapse all" buttons in separate MRs.