Remove keyboard depend - Githubissues

davetcoleman commented 7 years ago

Questions for @mcevoyandy:

are there still keyboard shortcuts or is it only by mouse now?
.rviztf is long, why not just .tf?
is the "# (comment line)" because you can't have unlimited comments?
why have IDs in the file format? seems it would just create annoyance?
move the "xyz delta" and "rpy delta" boxes onto the same row to save height in the box.
overall the GUI is kind of tall
it has a ton of debug output, but its not a big deal

mcevoyandy commented 7 years ago

@davetcoleman

are there still keyboard shortcuts or is it only by mouse now?

Yes, that Keyboard package is what we were using to listen for key strokes. Not sure if there's something in Qt we could use instead.

.rviztf is long, why not just .tf?

Started with *.tf, but thought not to since tf is a package and might cause confusion... was going to ask your opinion but forgot about it.

is the "# (comment line)" because you can't have unlimited comments?

no limit, just my own file type so wanted to make sure that was noted somewhere. I guess the real answer to this and above is to switch to some other supported file type. yaml?

why have IDs in the file format? seems it would just create annoyance?

I guess it doesn't need to be in there. Can take that out.

move the "xyz delta" and "rpy delta" boxes onto the same row to save height in the box.
overall the GUI is kind of tall

I agree. Open to alternate layouts.

it has a ton of debug output, but its not a big deal

I'll clean up msgs that aren't being used any more or aren't helpful.

mcevoyandy commented 7 years ago

@davetcoleman what do you think about the repo name? tf_keyboard_cal seems misleading now that you don't use the keyboard.

davetcoleman commented 7 years ago

But you should use the keyboard.... here some example code for Rviz: https://github.com/davetcoleman/rviz_visual_tools/blob/kinetic-devel/src/key_tool.cpp#L70

I guess the real answer to this and above is to switch to some other supported file type. yaml?

That would be sweet but not super high priority

I guess it doesn't need to be in there. Can take that out.

+1

mcevoyandy commented 7 years ago

@davetcoleman I Just added keyboard functionality when on the manipulation tab. I've got my own opinions about functionality/usability, but want to hear what you think before I make any further changes. No doc, but keyboard layout is the same. Save is not functional at the moment.

mcevoyandy commented 7 years ago

@davetcoleman added ability to have an imarker on top of tf... so use keyboard, gui, or just drag the marker around. Need help testing and finding cases where stuff doesn't work. Number of possible edge cases is getting hard to manage.

mcevoyandy commented 7 years ago

Think I've got all the functionality in there now... gui, keyboard (same buttons as before), interactive markers & menus if you want them. Could you look over and check out the functionality? I'll work on cleaning up and README when I get back to Boulder. Examples of anything you need to load will be in the config folder.

Will also work on the format of the GUI since I shouldn't be adding any more buttons.

davetcoleman commented 7 years ago

@awesomebytes fyi I think we are going to replace your python-based interactive markers tool with this new approach that also includes that functionality

davetcoleman commented 7 years ago

@mcevoyandy I see your point about renaming this package. It hasn't been released since ROS Jade, so for Kinetic and Lunar we could easily rename it. What are you thinking? How about tf_manual_cal? The name is available. This could be a really powerful package

davetcoleman commented 7 years ago

@mcevoyandy Questions on latest GUI (just built your PR locally again):

The default .rviz file needs to include the interactive marker display with the correct topic
Somehow I ended up with two interactive markers for the same TF frame
Are the checkboxes necessary for the frames? Why not just have imarkers and menus for all the frames?
Did you add keyboard shortcuts? How do they work?

mcevoyandy commented 7 years ago

@davetcoleman

Somehow I ended up with two interactive markers for the same TF frame

I've not noticed anything like this before. I'll see if I can reproduce it.
Are the checkboxes necessary for the frames? Why not just have imarkers and menus for all the frames?

Wasn't sure how to handle this... not all your markers need menus, but each marker that has a menu will have the same menu. We could load imarkers for all frames... could always just turn the topic off in rviz if you didn't want to see them.

Did you add keyboard shortcuts? How do they work?

keyboard shortcuts are there, you have to be on the manipulate tab. same keys as before. Save removed and only contained in the save tab.

I think the basic functionality is there, just need input on matters like these and help looking for any bugs that show up around edge cases that I haven't thought of.

mcevoyandy commented 7 years ago

what about tf_visual_tools? Also, need a way to launch from Rviz where the tf publisher also gets launched, not just the GUI interface.

davetcoleman commented 7 years ago

I like tf_visual_tools! I'm trying to decide if we should rename this repo or create a new one... I think a new one would be cleanest. Its your codebase at this point so you should keep it under your name (though I'm happy to host it and maintain the release process if you want).

mcevoyandy commented 7 years ago

I'd like to learn more about hosting and maintaining for release... not sure what it takes. I'll move the code to a tf_visual_tools repo tonight and continue clean up and documentation.

Andy McEvoy | (832) 439 - 6150

On Wed, Jun 28, 2017 at 10:56 AM, Dave Coleman notifications@github.com wrote:

I like tf_visual_tools! I'm trying to decide if we should rename this repo or create a new one... I think a new one would be cleanest. Its your codebase at this point so you should keep it under your name (though I'm happy to host it and maintain the release process if you want).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/davetcoleman/tf_keyboard_cal/pull/11#issuecomment-311722072, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSamf7X9685s5KfVm6r9ntCMLnSLXyxks5sIoXbgaJpZM4No1M7 .

davetcoleman commented 7 years ago

Cool - if you don't mind I'd still like to be a maintainer / admin of your repo

mcevoyandy commented 7 years ago

what do you mean? replace the imarker stuff in tf_keyboard_cal with imarker_simple?

Andy McEvoy | (832) 439 - 6150

On Wed, Jun 28, 2017 at 6:50 PM, Dave Coleman notifications@github.com wrote:

@davetcoleman commented on this pull request.

In CMakeLists.txt https://github.com/davetcoleman/tf_keyboard_cal/pull/11#discussion_r124690996 :

LIBRARIES

${PROJECT_NAME}_manual_tf_alignment

${PROJECT_NAME}_imarker_simple

${PROJECT_NAME}_gui +# ${PROJECT_NAME}_manual_tf_alignment +# ${PROJECT_NAME}_imarker_simple

... i recommend you use it for your imarker needs though

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/davetcoleman/tf_keyboard_cal/pull/11#discussion_r124690996, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSamc3Gooc3WiZMxIRpgTnJUKq7ENFaks5sIvTOgaJpZM4No1M7 .

PickNikRobotics / tf_keyboard_cal

Remove keyboard depend #11

@davetcoleman commented on this pull request.