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

GoTo Button Enhancements #760

Open
iceblu3710 opened this issue Aug 24, 2018 · 8 comments
Open

GoTo Button Enhancements #760

iceblu3710 opened this issue Aug 24, 2018 · 8 comments

Comments

@iceblu3710
Copy link
Contributor

iceblu3710 commented Aug 24, 2018

I am adding extra functionality to the GoTo button on screen to allow Gcode entry and Line numbers easily.

It's all working but the _keyboard bindings only work on numbers somehow?! I wanted the user to be able to type in XYZ directly and haveg toggle G0/G1 and l toggle 'L`

If I put a print in the keydown_popup function it is never called and on inspection global_variables._keyboard is always None. Does anybody know how the keyboard keypress bindings work in GroundControl? My Branch HERE I used the Kivy doc HERE and now _keyboard gets an object but STILL no call into keydown_popup.

groundcontrol_008

@BarbourSmith
Copy link
Member

I remember the keyboard binding being a little strange. I would look at the code which is used to bind the keyboard to the popup for entering numbers, it could be not unbinding completely causing you issues elsewhere.

I'm not 100% sure I understand the feature. The GoTo button lets you jump to a known gcode line number, when would users want to enter a line of gcode there? I totally agree that the GoTo could use a revamp because if you jump into the midst of a G2/G3 command the behavior is unpredictable

@blurfl
Copy link
Collaborator

blurfl commented Aug 24, 2018

The keyboard binding is a bit of a mystery. I was working on using the number keypad for moving the sled a while back, shelved that after some thrashing.

@iceblu3710
Copy link
Contributor Author

As for why: I frequently want to jog the sled to a specific coordinate and currently I need to enter a jog value then move x/-x then a new jog value and y/-y. The GoTo popup opens with L as the default but give the easy addons of quick positioning.

Ultimately a right click/hold menu on the main canvas with a jog to here command would be ideal. This was just simpler.

I do notice the unbind call comes only then you call a new popup, not when closing the current popup. It just blows my mind that the number pad works but isn't called somehow. I will put breaks in all key calls and see if the lagging unbind is the issue. If it is that will be a separate PR for a big fix.

@iceblu3710
Copy link
Contributor Author

Also, I have no real way of testing on a touch device. What kind of touch device/tablet/OS is the intended audience?

@iceblu3710
Copy link
Contributor Author

It appears that all they keyboard handling is skewed and a call order is messing things up. I put prints in all areas to ID what was called and the bind in frontpage is overriding ALL the popups binds.

Process of operations:

Load groundcontrol
keypress '1' on frontpage
open GoTo popup
keypress '1'
close GoTo popup
keypress '1' on frontpage
open Jog popup
keypress '1'
close Jog popup
keypress '1' on frontpage
TouchGoToInput: Unind
TouchGoToInput: Bind
FrontPage/gotoLinePopup: Bind
FrontPage: Keycode
FrontPage: Unbind
TouchGoToInput: Keycode
TouchNumberInput: Bind
FrontPage/textInputPopup: Bind
FrontPage: Keycode
FrontPage: Unbind
TouchNumberInput: Keycode

The main issue is frontpage overriding the binding, the second issue is popup.ondismiss_popup() is called on the next popup instead of when the popup actually disappears. This causes the additional problem of the bind sticking around after the popup is gone.

It's funny this error doesn't show up as frontpage declares the same key check routine so it effectively doesn't care. If you shuffle things around a bit I get the features class to handle the bind, keypress and unbind directly as expected.

def gotoLinePopup(self):
    self.popupContent = TouchGoToInput(done=self.dismiss_gotoLinePopup, data=self.data)
    self._popup = Popup(title="GoTo...", content=self.popupContent,
                        size_hint=(0.9, 0.9))
    self._popup.open()
    self._popup.bind(on_dismiss=self.popupContent.ondismiss_popup)

After removing the offending _keyboard binds and binding the on_dismiss to the popups method we get the following messages when following the same process as before:

TouchGoToInput: Unind
TouchGoToInput: Bind
TouchGoToInput: Keycode
TouchGoToInput: Unind
TouchNumberInput: Bind
TouchNumberInput: Keycode
TouchNumberInput: Unbind

Fixing this binding issue will clean out A LOT of surperflous code. For EXAMPLE If the TouchNumberInput presents the user a number pad it makes sense for that widgit to handle the bind, keymapping and unbind directly. You would never override it as its a re-useable widget. That means lines 28-62 are completely unneccicary.

I wouldn't mind a second opinion on all this before I do a trial run and gut the binding in the system.

@iceblu3710
Copy link
Contributor Author

iceblu3710 commented Aug 25, 2018

@BarbourSmith Bug confirmed:
Current master branch. Open ground control and the PgUp/PgDown to zoom the canvas works. Open the GoTo or Jog value popups and upon returning to the canvas PgUp/PgDown no longer work.

@BarbourSmith
Copy link
Member

Fixing this binding issue will clean out A LOT of surperflous code. For EXAMPLE If the TouchNumberInput presents the user a number pad it makes sense for that widgit to handle the bind, keymapping and unbind directly. You would never override it as its a re-useable widget. That means lines 28-62 are completely unneccicary.

I wouldn't mind a second opinion on all this before I do a trial run and gut the binding in the system.

I agree with this. I've been thinking for a while that the way the touch number input is done with so much copped and pasted code there must be a better way. I think that a clean up of the system is a great idea. It will pave the way for more keyboard use.

Ultimately a right click/hold menu on the main canvas with a jog to here command would be ideal. This was just simpler.

Is that different from the click and hold menu we have?

image

@iceblu3710
Copy link
Contributor Author

I literally had no idea this menu existed...... I have never felt the need to hold down on any one location for long periods of time....

Oh well, I found a bug making a semi-useless feature. A manual GoTo window may still be handy for some.

BTW, the hold menu displays pretty wonky on my Ubuntu machine.
screenshot at 2018-08-28 19 58 07

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

No branches or pull requests

3 participants