Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable Ctrl+C to stop the GUI #65

Merged
merged 2 commits into from
Feb 5, 2023
Merged

Enable Ctrl+C to stop the GUI #65

merged 2 commits into from
Feb 5, 2023

Conversation

sea-bass
Copy link
Owner

@sea-bass sea-bass commented Feb 4, 2023

Also helps address #64.

Now any program that uses the GUI will terminate correctly when Ctrl+C is pressed. I also realized I was duplicating code to start the GUI and moved it to a utility.

pip install lark pytest pytest-dependency pytest-html wheel
pip install catkin-pkg empy lark pytest pytest-dependency pytest-html wheel
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snuck this in because it helps resolve some weird colcon build issues

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the heck, catkin? That's really weird

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you at least leave a comment with a link to where you found this?

@AndyZe
Copy link
Collaborator

AndyZe commented Feb 5, 2023

In testing, it does kill the GUI with Ctrl+C 👍 but I get this error, too:

python3 examples/demo.py -->

[robot] Navigating to bedroom
^CTraceback (most recent call last):
  File "/home/andy/python-virtualenvs/pyrobosim/lib/python3.10/site-packages/pyrobosim/gui/main.py", line 22, in <lambda>
    timer.timeout.connect(lambda: None)
KeyboardInterrupt
Aborted (core dumped)
(pyrobosim)

Is that new?

Edit: I also get Aborted (core dumped) on main. So that's not a regression.

@AndyZe
Copy link
Collaborator

AndyZe commented Feb 5, 2023

I'll push a comment about catkin-pkg then merge this.

@AndyZe AndyZe merged commit b37340a into main Feb 5, 2023
@AndyZe AndyZe deleted the gui-signal-exit branch February 5, 2023 05:04
@sea-bass
Copy link
Owner Author

sea-bass commented Feb 5, 2023

The catkin thing is because it's actually used by colcon! See e.g.

colcon/colcon-ros#118

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants