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

Implement key map overrides for jogging. #283

Closed
wants to merge 0 commits into from
Closed

Conversation

aaronse
Copy link

@aaronse aaronse commented Dec 26, 2022

Enable Users to optionally adapt/define/suppress key shortcuts for their machine and keyboard setups.

Sent this pull request to better help communicate the ask, and a possible solution approach.

@luc-github
Copy link
Owner

luc-github commented Dec 27, 2022

Nice - indeed the shortkeys management need improvement
Here my comments :

  • I think the available controls for shortkey should be listed in array because user has no idea of id of different controls , using a search may not work if the panel is not visible and we need to configure key in settings
  • Shortkeys should be auto detected (ask user to type them) or listed because user does not know what is the syntax of keys neither
  • instead of using string which is ok when you know all use a ui like panels: with json format:
    name (fixed) which could be the id of control but it may be meaning less for user - TBD
    the id of control to be controled (fixed by above list of bullet 1), and the shortcut
  • Use the json definition from the begining so user can see all keys configured in settings
  • Reflect the help using the json definition would then be very easy even after update
  • doing this will allow to implement it for cnc with more / less than 3 motors

Has you did the shortcut management already from API point of view it just need an additionnal list type (shortkeys) to be displayed properly and use json and parse them

Let me know what do you think ,

@V1EngineeringInc

This comment was marked as off-topic.

@luc-github

This comment was marked as off-topic.

@V1EngineeringInc

This comment was marked as off-topic.

@V1EngineeringInc
Copy link

The other odd thing about the physical keyboard bindings is homing.
keys

So when typing in a terminal, Using X, Y, or Z, homes the machine. We tend to use the terminal a lot. Would it make more sense to use Alt+X,Y, Z to home?

@luc-github
Copy link
Owner

luc-github commented Dec 28, 2022

there is no current support for combination key shortcut, I am not sure it will be address in this PR, original implementation was as simple as possible - waiting for feedback and this PR is the first one I got

@aaronse sorry I have just make some change for current consistency issue raised by @V1EngineeringInc - just rebase your fork should fix the conflict - you do not need to rebuild package - it is better to build only at the end because need changing version as well, so you can leave it to me

@aaronse
Copy link
Author

aaronse commented Dec 28, 2022

Hmm, wasn't sure how to reopen/continue this pull request after rebasing to absorb recent edits. Ended up creating #284 which references this one. Will continue discussion there unless you say otherwise. Cheers!

@luc-github
Copy link
Owner

sure - thank you

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.

3 participants