luc-github / ESP3D-WEBUI

A Web UI for ESP8266 or ESP32 based boards connected to 3D printers / CNC
GNU General Public License v3.0
747 stars 305 forks source link

Implement key map overrides for jogging. #283

Closed aaronse closed 1 year ago

aaronse commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

I agree with the main topic, with the end goal is just to be able to remap the keyboard shortcuts. Is it possible to make the editable in the config section to assign our own?

For us, using it with a CNC machine and a 3D printer the default mapping is odd. On the keyboard: X+ up arrow (preferred Y+) X- down arrow (preferred be Y-)

Y+ right arrow (preferred be x+) Y- left arrow (preferred be x-)

Z is fine what I would consider standard.

When facing a machine, all the interfaces I have used are 90 degrees off from that. Y is the up down arrow, X axis is left and right.

Is there something I am missing on why they are assigned like that?

luc-github commented 1 year ago

@V1EngineeringInc

I agree with the main topic, with the end goal is just to be able to remap the keyboard shortcuts. Is it possible to make the editable in the config section to assign our own?

This is the main purpose of this PR - I do not understand your question

For us, using it with a CNC machine and a 3D printer the default mapping is odd. That the purpose of the PR to adapt according User setup

When facing a machine, all the interfaces I have used are 90 degrees off from that. Y is the up down arrow, X axis is left and right.

The UI I use is the common one image

if you are 90 degres off is due to the fact you have inverted wiring X and Y motors on your system - That said some interface allow to switch XY , invert X axis aas well as Y axis On ESP3D - WebUI it is still a POC for Plotter Plotter settings: image Plotter Jog: image

It is not implemented on 3D printer UI (keyboard shortcut enabled), neither CNC UI because it make no sense - UI is not reflective CNC Jog: image 3D printer Jog Mobile view / Shortcut enabled UI image

The only part that is missing is the 3DPrinter UI - not shortcut enabled / mobile view 3D printer Jog Desktop view: image

I still have to work on it - it is something I forget to be honest but this is out of scope of this PR -

Edit: it is already in issue tracker actually : https://github.com/luc-github/ESP3D-WEBUI/issues/262

if you have other suggestion/concern about UI please open new ticket - to keep this ticket clean for this PR discussion, in all case shortcuts use component id so this PR won't be affected by any UI changes once defined

V1EngineeringInc commented 1 year ago

I think that is just it, all the interfaces are perfect. The keyboard mapping seems to be contradictory to all the interfaces you made.

The up arrow on a keyboard is X+ when all your interfaces the up arrows shows Y+ (what we all expect).

So some of my users are using wireless keyboards to drive a CNC machine to the work. The default mapping of the arrow keys is the issue they are having. The webui interfaces all work very well! (P.S. most of us are using V3)

V1EngineeringInc commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

sure - thank you