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

fix for issue #1849 #1850

Merged
merged 4 commits into from
Apr 9, 2022
Merged

fix for issue #1849 #1850

merged 4 commits into from
Apr 9, 2022

Conversation

bertieconfundo
Copy link
Contributor

This is a fix for issue #1849. I detailed the reasons why the fix is necessary in the issue. With these changes I no longer have unwanted jog movement on log diagonal jogs after the long press is released.

@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2022

Codecov Report

Merging #1850 (e1abf1f) into master (99fe13f) will decrease coverage by 0.00%.
The diff coverage is 16.66%.

@@             Coverage Diff              @@
##             master    #1850      +/-   ##
============================================
- Coverage     18.33%   18.33%   -0.01%     
  Complexity     1938     1938              
============================================
  Files           699      699              
  Lines         29466    29471       +5     
  Branches       2318     2317       -1     
============================================
  Hits           5403     5403              
- Misses        23613    23617       +4     
- Partials        450      451       +1     
Impacted Files Coverage Δ
...inder/universalgcodesender/AbstractController.java 65.08% <0.00%> (-0.33%) ⬇️
...nder/universalgcodesender/services/JogService.java 6.04% <0.00%> (ø)
...niversalgcodesender/utils/ContinuousJogWorker.java 0.00% <0.00%> (ø)
...va/com/willwinder/ugs/nbp/jog/JogTopComponent.java 0.00% <0.00%> (ø)
...illwinder/universalgcodesender/GrblController.java 54.54% <58.33%> (-0.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31e0d2b...e1abf1f. Read the comment docs.

@breiler
Copy link
Collaborator

breiler commented Mar 30, 2022

Nice!

I will give it a try later tonight. I am really glad if you found a solution for this long going issue!

@breiler
Copy link
Collaborator

breiler commented Mar 30, 2022

I just tried it and it works a lot better when using the jog buttons. However it did not work when using a gamepad with analog inputs. I don't have time to look at it until this weekend, but it looks promising so far.

I noticed one problem though (which might have been there all along). If I set a jog feed rate to 6000 mm/min but my machine only moves at a maximum rate of 1000 mm/min I get a soft limit alarm fairly quick. I might add a feed rate limit so that it doesn't exceed the max rate of the machine.

@bertieconfundo
Copy link
Contributor Author

If you'd like, I can take a look at the gamepad use case. I don't use that on my own rig, but I've got a joystick I can connect up to test it out. I have an idea about why it might not be working, and what would need to be done to fix it: in the ContinuousJogWorker if you are changing the x,y,z values during a continuous jog (before the stick returns to the neutral position) then it's possible that we'll need to move the calculation of the step size into the loop so it updates each time through the loop. Also, I didn't anticipate that the x/y/z values would be anything other than -1 or +1. I bet when you do the joystick jogging you're setting them to floats to control the speed, in which case the math where we scale the step size based on vector direction needs to be rewritten at sqrt(x^2 + y^2 + z^2).

Your observation of the jog feed rate exceeding the machine's capabilities is something that I didn't anticipate. I agree that limiting the jog feed rate to be within the limits of the machine makes the most sense here, so you don't have to have a bunch of code all over the place checking for that case and dealing with it.

Anyhow, I will probably have some time this weekend to look into this more if you'd like me to adjust the solution so it works with the gamepad jogging as well.

There's also another part of this that I'd like to fix at some point: when you're doing a continuous jog and the machine sees that the commands in the buffer will cause you to exceed a soft limit, the machine stops and it doesn't go all the way to the limit first. To fix this, the JogService needs to have awareness of where the machine is (and is going to be with each jog command sent) and it should prevent sending of jog commands that would cause the machine to exceed it's soft limits. One way to do this would be to still send a jog command, but change the x/y/z delta values to zero if they would cause the machine to go outside of it's soft limits.

@bertieconfundo
Copy link
Contributor Author

I just got ahold of a gamepad to test the jog control. In version 2.0.11 I'm able to get it working with the gamepad, but the motion seems to be irregular. I think I should be able to get this to work smoothly now that I have something to test it with. I will look at it this weekend.

@breiler
Copy link
Collaborator

breiler commented Apr 1, 2022

Excellent, I wanted to offer help on this as I didn't know if you had the sufficient hardware. But now that you do I really appreciate you taking a stab at this. Great work!

@bertieconfundo
Copy link
Contributor Author

Ok, I got this working well today =) Will update soon. There was one hardware/driver related issue that was causing me trouble: I'm using a Mac with a genuine Arduino Nano (with an FTDI serial chip). The Mac FTDI USB serial drivers have horrible latency - it looks like the mac is polling for new RX serial data every 200ms or so. Part of the recommended jogging algorithm relies on the fact that when you send a jog command, the grbl controller parses it and sends an "ok" in less than 10ms or so. I got around this problem by replacing my genuine arduino with a clone that is using a different serial usb chip (CH340 I think), and there isn't a latency issue there. More to follow.

…/grbl/wiki/Grbl-v1.1-Jogging

Added better support for hardware jog cancel in the GrblController
Added jog speedFactor control for continuous jog in JogService
Added logic to supress errors resulting from a jog command (hit machine soft limits)
@bertieconfundo
Copy link
Contributor Author

@breiler, ok, I've updated the branch - take a look and let me know what you think.
One thing I noticed about the joystick - if has to be connected before UGS is loaded, or it won't be seen. This is a little annoying with the bluetooth controller I'm using as it disconnects itself when idle, and I have to remember to wake it up before starting UGS. I saw there was a loop in the joystick code that calls "update()" every 3 seconds if there are no controllers found, but this only works for reconnecting controllers that were enumerated on startup. I wonder if we could deinitialize/reinitialize the entire joystick API after a delay if there are no controllers found?

@bertieconfundo
Copy link
Contributor Author

btw, there is more to be done here, I think: One thing that happens right now that isn't ideal is when you jog up against a soft limit of the machine, while you are jogging in multiple axes at the same time (diagonal). The entire jog command is ignored if it takes the machine across any of the soft limits. A better behavior would be for the jog service to keep track of where the machine is in the jog, and prevent it from sending jog commands that move the machine across a soft limit by independently filtering out moves in each of the axes (X, Y, Z)-- so that if you move up against the edge of an axis diagonally, the machine will continue to move in the directions it can. I can take a look at modifying the JogService so that it has awareness of the machine location. I need to look into how to get the soft limit information generically from the controller.

@breiler
Copy link
Collaborator

breiler commented Apr 9, 2022

I have just tried this and it works perfectly. It is running a lot smoother and more responsive than before using a gamepad controller. Great work!

I wonder if we could deinitialize/reinitialize the entire joystick API after a delay if there are no controllers found?

I would like to replace the current joystick library Jamepad with sdl2gdx which supposedly supports "hot plugging".

A better behavior would be for the jog service to keep track of where the machine is in the jog...

It could probably be done in UGS but increases the complexity a lot. We currently have a problem with figuring out the machine boundaries using the soft limit settings. Due to a compile time flag in GRBL there is no way of knowing the min/max edges (#1690). In FluidNC they solved the problem of jogging out of bounds in the firmware which I think is a lot better solution.

@breiler breiler merged commit 959485e into winder:master Apr 9, 2022
@bertieconfundo bertieconfundo deleted the fix1849 branch April 9, 2022 15:58
@bertieconfundo
Copy link
Contributor Author

I would like to replace the current joystick library Jamepad with sdl2gdx which supposedly supports "hot plugging".

Sounds like a good approach rather than adding additional complexity to UGS.

It could probably be done in UGS but increases the complexity a lot. We currently have a problem with figuring out the machine boundaries using the soft limit settings. Due to a compile time flag in GRBL there is no way of knowing the min/max edges (#1690). In FluidNC they solved the problem of jogging out of bounds in the firmware which I think is a lot better solution.

Fixing the grbl firmware to ignore axes of jog commands that go outside the soft limits of the machine is another way I thought to solve this. I'm already running a version of grbl that has a few changes to it - I might go see what I can do on that side. Not sure the owner of grbl wants to make any more changes though (or whether or not I can come up with a change that doesn't exceed the code storage limitations of the Arduino Uno/Nano.

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