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

rqt_reconfigure: Parameters values do not update if changed from keyboard #101

Closed
bgromov opened this issue Jul 14, 2013 · 17 comments
Closed
Assignees
Labels

Comments

@bgromov
Copy link

bgromov commented Jul 14, 2013

Changing values with a slider by arrow keys on keyboard does not update parameters. Though directly entered in edit box values work.

@ghost ghost assigned 130s Jul 15, 2013
@130s
Copy link
Member

130s commented Jul 15, 2013

@bgromov Does this happen with all kind of data type? Or only with enum? I can't test right now because I don't have an access to any robot that uses Parameter Server. If it's only enum, do you think it's related to the issue that's just solved by this patch?

@bgromov
Copy link
Author

bgromov commented Jul 15, 2013

I noticed that while changing parameters of stereo_image_proc/disparity, so all parameters there are numerical.

Just tried the version from groovy-devel, the problem is still there.

@bgromov
Copy link
Author

bgromov commented Jul 17, 2013

Changing position of slider with mouse wheel also makes no effect. Seems it reacts only to mouse Left-Click.

@aleeper
Copy link

aleeper commented Jan 30, 2014

bump
Being able to use the arrow keys to take steps along the sliders was a really useful feature in Fuerte. We still often use the Fuerte reconfigure because of this...

@dirk-thomas
Copy link
Contributor

The original maintainer is not actively working in this.

And due to the many ticket pending for this plugin it has been marked to be experimental (https://github.com/ros-visualization/rqt_common_plugins/blob/groovy-devel/rqt_reconfigure/plugin.xml#L12).

Please consider to provide pull requests and/or take over the maintainer role for this plugin.

@dirk-thomas
Copy link
Contributor

Please checkout the latest code from the repo and give it a try and provide feedback if the problem is solved for you, too.

Until then I will mark this a closed.

@bgromov
Copy link
Author

bgromov commented Feb 13, 2014

I am getting Segmentation fault and python: double free or corruption (fasttop) in some cases. I have tried it on my ros/geometry#25. Here is the output.

Could some one else try to reproduce the problem?

To reproduce:

  1. Clone and build reconfigure_static_tf branch of https://github.com/bgromov/geometry.git

  2. Run static_tf_publisher, e.g.

    $ rosrun tf static_tf_publisher 0 0 0 0 0 0 /world /my_tf 100

  3. Run rqt_reconfigure and select yaw_rad slider.

  4. Move the slider back and forth by holding arrows for some time. It usually takes not more than 3-4 repetitions for it to hang or crash.

I did not face this problem while changing x, y or z, it crashes only with RPY parameters. Maybe that is because there are several linked parameters in my config, thus many sliders updated at the same time.

@cottsay
Copy link
Member

cottsay commented Feb 13, 2014

Okay, I'll take a look sometime tonight.

@cottsay cottsay reopened this Feb 13, 2014
@cottsay
Copy link
Member

cottsay commented Feb 13, 2014

Okay, I can't reproduce this, but I think I know what is causing it. This is similar to when rqt_reconfigure was sending parameter server updates while sliding the slider bar. The backtrace is a pretty clear indicator that there was an internal crash in PyQt. I think there is simply too much going on for PyQt to handle it.

The key repeat rate causes the changes to be sent, and as the slider moves, we update the parameter server VERY quickly (at the key repeat rate). When the updates from the server come back to rqt_reconfigure, the delay often puts the slider in a different place (making the slider move in a rather jagged motion).

Since the slider is moved for every keystroke, I can't control the rate at which the slider is modified.

Would it be reasonable to NOT update the parameter server until you release the key? This would prevent the rapid reconfigurations and subsequent rapid updates.

@aleeper
Copy link

aleeper commented Feb 13, 2014

I'll put a vote in for that being reasonable.

@dirk-thomas
Copy link
Contributor

I am not sure how the threading model of dynamic_reconfigure and rqt_reconfigure is done. It could be that the incoming parameter changes are handled in the ROS callback thread and then try to change values of the Qt controls which is forbidden.

Any UI element in Qt can only be changed from within the Qt event loop. So usually the callback would use a Qt signal to trigger a slot in the context of the event loop which actually interacts with the widget.

@cottsay
Copy link
Member

cottsay commented Feb 13, 2014

This makes sense. I'll try using some connect()s to make the changes happen and see if that helps.

I was able to repro' this after some time. I think I have it narrowed down to updating the gui from the parameter server, which would align with what @dirk-thomas is saying.

It might be possible to leave the updates from keyboard on slider as-is, and fix the segfault instead. I'd like to find a way to get rid of the stuttering if we keep it like this, though.

@cottsay
Copy link
Member

cottsay commented Feb 15, 2014

Pull Request #210:
  • Should fix your problem
  • Is in testing and has not been merged yet

Please comment on the pull request when you have tested for your issue.

@cottsay cottsay self-assigned this Feb 15, 2014
@130s
Copy link
Member

130s commented Feb 17, 2014

@cottsay thank you for taking over the maintainer-ship, and I confirm that the set of your commits resolves two issues raised in this ticket (changing values by arrow key and mouse wheel).

Just fyi how I confirmed:

term-1$ git clone https://github.com/130s/test_dynamic_reconfigure.git
term-1$ rosrun test_dynamic_reconfigure test_dynreconf_server.py  (I have no access to robots with param server)

term-2$ rqt &
(And operate rqt_reconfigure)
term-2$ rosparam get /testnode_dynamic_reconfigure_30428_1392614214764/test_int_param  (or other params available)

@130s
Copy link
Member

130s commented Feb 17, 2014

(sorry this isn't related to either this ticket nor rqt_reconfigure, but I can't find a way to report this)

@bgromov just wanted to mention that I first tried to test by using the repository you mentioned in this comment but the building was stopped by this orocos_kdl issue, and trying to rebase with the geometry upstream ended up too much conflicts to handle.

@130s
Copy link
Member

130s commented Feb 17, 2014

Now I confirmed the patch also works with what @bgromov wrote in this comment without segfault after using it for several minutes.

(to compile his geometry I just removed /geometry/eigen_conversions and geometry/kdl_conversions and catkin_make as a tentative workaround. Hope this doesn't effect the issue)

@cottsay
Copy link
Member

cottsay commented Feb 18, 2014

Fixed via 51715b2

@cottsay cottsay closed this as completed Feb 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants