luxonis / rae-ros

Implementation of RAE ROS and gazebo stack
MIT License
32 stars 13 forks source link

Unhandled KeyboardInterrupt in battery node #49

Closed sskorol closed 1 year ago

sskorol commented 1 year ago

Steps:

Expected: battery node is correctly stopped Actual: unhandled KeyboardInterrupt exception occurs:

[battery_status.py-6] Traceback (most recent call last):
[battery_status.py-6]   File "/ws/install/rae_bringup/lib/rae_bringup/battery_status.py", line 122, in <module>
[battery_status.py-6]     main()
[battery_status.py-6]   File "/ws/install/rae_bringup/lib/rae_bringup/battery_status.py", line 114, in main
[battery_status.py-6]     rclpy.spin(battery_status_node)
[battery_status.py-6]   File "/opt/ros/humble/local/lib/python3.10/dist-packages/rclpy/__init__.py", line 222, in spin
[battery_status.py-6]     executor.spin_once()
[battery_status.py-6]   File "/opt/ros/humble/local/lib/python3.10/dist-packages/rclpy/executors.py", line 705, in spin_once
[battery_status.py-6]     handler, entity, node = self.wait_for_ready_callbacks(timeout_sec=timeout_sec)
[battery_status.py-6]   File "/opt/ros/humble/local/lib/python3.10/dist-packages/rclpy/executors.py", line 691, in wait_for_ready_callbacks
[battery_status.py-6]     return next(self._cb_iter)
[battery_status.py-6]   File "/opt/ros/humble/local/lib/python3.10/dist-packages/rclpy/executors.py", line 588, in _wait_for_ready_callbacks
[battery_status.py-6]     wait_set.wait(timeout_nsec)
[battery_status.py-6] KeyboardInterrupt
danilo-pejovic commented 1 year ago

I will close this issue and we can continue this conversation at the #50 so we are not talking about same thing at 2 places.

sskorol commented 1 year ago

Ok, but, you know, that's a good practice to create issues for changes you're working on for better planning, progress tracking and clearer history. Moreover, there might be a big epic-like issue that would likely be covered by several PRs. In case of regression, the first thing people usually do, is searching among existing issues. And it's quite easy to track the related PRs when you use GH id in corresponding commit messages or PR description. BTW, as I used a special keyword "fixes #N" in PR description, this issue would be automatically closed on merge. So it's not required to explicitly close it. Anyway, of course, it's just a recommendation, but issue tracking would help a lot the others who would face similar issues in the future. Otherwise, there's a high probability of duplicates.