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

Fix usability bugs in RobotState display #455

Conversation

Projects
None yet
2 participants
@v4hn
Copy link
Member

commented Mar 1, 2017

I just tried to make use of this display for the first time
and now I keep wondering how people managed to use it in the first place...

Over here it segfaults when pressing Enter after changing the topic
and it does not connect to the topic when the display becomes enabled.

These patches address both problems.

Should be picked to jade.

@v4hn

This comment has been minimized.

Copy link
Member Author

commented Mar 3, 2017

ok, I missed that 4e72ac5 was not backported to indigo.
I'll have another look into this and fixup the commits later.

@v4hn v4hn changed the title Fix crucial usability bugs in RobotState display Fix usability bugs in RobotState display Mar 4, 2017

@v4hn v4hn force-pushed the v4hn:pr-indigo-improve-robot-state-display branch from de0e55d to 763668a Mar 7, 2017

robot state display: subscribe on enable / unsubscribe on disable
The display didn't connect in onEnable.
It only did after the user changed the topic name.

Also, disconnect onDisable. There's no need to keep listening to the topic
until the display is enabled again.
@v4hn

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2017

ok, I failed to reproduce the segfault on a standard ubuntu trusty, so I'll remove it from this request.
I'll add a request for kinetic with both patches instead.

The missing subscribe/unsubscribe is relevant for indigo imo, so I leave the request open for this patch.

@davetcoleman
Copy link
Member

left a comment

lgtm

@davetcoleman davetcoleman merged commit aac7921 into ros-planning:indigo-devel Mar 11, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

davetcoleman added a commit that referenced this pull request Mar 11, 2017

robot state display: subscribe on enable / unsubscribe on disable (#455)
The display didn't connect in onEnable.
It only did after the user changed the topic name.

Also, disconnect onDisable. There's no need to keep listening to the topic
until the display is enabled again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.