-
Notifications
You must be signed in to change notification settings - Fork 104
[joint_state_publisher] Introduce Gridlayout & tweaking to GUI #150
Conversation
Any comments @wjwwood ? |
@bmagyar The code changes look fine to me, but I lack a ready example to "exercise" the feature. What are you using to test it? Is it something I could easily set up and try out? |
Should this test be part of the PR or is it ok if I put a short list of On 2 Sep 2016 19:06, "William Woodall" notifications@github.com wrote: @bmagyar https://github.com/bmagyar The code changes look fine to me, but — |
Just a list of commands. If you think you can automate the test it would be great, but I wasn't suggesting that. |
I've cleaned up the code a bit, I think now it's worthy for merging.
do the usual stuff, then |
I still want to test this out before merging. I'll try to do that soon. |
Let me know if you need anything for that. I tested my commands above so it should in theory be fine for you |
Just tested this (sorry for the delay) and it looks good. I'm going to merge this, but I agree with @VictorLamoine that a label on the spinbox would be really great. If anyone has time to add that, please open a new pr, I'll be sure to merge it fast. |
Thanks again @bmagyar! |
This should fix the issue of #146. Excuse my sketchy Python :)
As noted above, this is a work in progress PR. Please make comments.
Task list: