Analog joystick jogging control

[vector76]

Description

This update adds support for jogging with an analog joystick, as shown in this video: https://www.youtube.com/watch?v=b2xEmkZEF6Y

Benefits

This is probably not useful for a 3D printer but for CNC it is helpful because a common workflow is to 1. Home machine 2. Jog to workpiece 3. G92 X0 Y0 Z0 4. Run gcode. Step 2 can be much more pleasant with physical, analog control.

Related Issues

Not sure if this is "appropriate" for Marlin, since it might be somewhat of a niche use. No hard feelings if this is rejected.

Also I would welcome any feedback regarding code style or if it belongs in a separate file besides temperature.cpp.

[vector76]

Oops, comment in Configuration_adv.h says to use M114 but it should be M119. That's where I added reporting on the ADC values.

[thinkyhead]

This has been rebased and squashed, with changes added in a new commit. To get the code into your working copy use the Git Console and the following commands:

git fetch origin
git checkout bugfix-2.0.x
git reset --hard origin/bugfix-2.0.x

The construction of the G-code command has been modified to use less stack space. A recursion barrier was added to inject_joy_action so it cannot be called while enqueue_one_now is waiting for an open slot (not expected to be a problem since this only injects moves into an empty queue).

Tomorrow I'll look at whether this can send moves directly to the planner, and I have an analog stick here so I can spend some time testing and refining until it feels about perfect (on a Cartesian). Who knows how it will work with a Delta or SCARA?

[vector76]

Awesome, thank you!
I had tried to construct the string in place but I ran into pointers not behaving the way I expected.
Speaking of which, one thing looks a little odd to me is this:

     dtostrf(move_dist[X_AXIS], 0, 4, tmp[strlen(tmp)]);

Which seems like it perhaps ought to be this:

     dtostrf(move_dist[X_AXIS], 0, 4, &tmp[strlen(tmp)]);

Anyway I will give it a go when I get a chance. Thanks again!

[thinkyhead]

Which seems like it perhaps ought to be this:

Yes, that's correct! Or, tmp + strlen(tmp). Whichever looks nicer to you.

[vector76]

Ok I tested it and it works. I had to fix a few other little things that slipped through the cracks.
I'm not sure how to propose changes in this context but I have them in a commit here:
vector76@1efe2dc

And I can confirm that with these edits it does work.

[thinkyhead]

I'm not sure how to propose changes in this context but I have them in a commit here:

Pushing the commit to your origin was all that was required to update this PR.

[vector76]

Just to follow up, I think this is complete as far as I'm concerned. The Travis CI appears to have failed on something unrelated, something to do with Nozzle::clean. Maybe this is because I didn't rename my branch?

I'm happy to contribute/fix if there is more I can do, but if so, I will need a bit of guidance.

[marcio-ao]

What is impressive to me about this is that the motion is smooth and seems to stop very soon after you release the trigger. We added a jog control to one of our printers that uses the ExtensibleUI, but the motion is jerky rather than smooth. It's pretty bad. If there was a method in ExtensibleUI that performed this type of motion, it would be way better than what I am currently using!

[marcio-ao]

After a quick look at the code, it looks like your code is way more sophisticated than what I have, which is why it is smooth. I would very much like to see this refactored a bit so the jog code was independent from the analog joystick interface. That way touch screens could implement a "virtual joystick" that operated in the same manner as your physical joystick. This could be a method in the ExtUI where you provided a direction vector to tell the printer to jog in a specific direction.

[marcio-ao]

@vector76: Could I ask you to refactor the code into two parts? One which accesses the physical joystick through I/O pins, and the other which does the jogging? Ideally there would be a single function that sets the jogging direction, maybe as a velocity vector. Once I had that, I could integrate it our printers which have a touch screen and we could see how well a virtual joystick works.

[jeffeb3]

@marcio-ao If you just put the code after it determines the nornalized adc into a function, you could call it from other joystick code.

You could also do some #define magic to just set the normalized adc values yourself (they are (-1,1)[3]).

[vector76]

I can see a case for generalizing this. I'm not familiar with ExtUI so I don't know if I'm thinking the same thing, but the joystick-specific stuff (ADC readings) could be separated from the polling-jogging. Ordinary command-based jogging will always remain separate I would think, but I can imagine other cases where poll-based jogging would be useful.

If you wanted to use an arcade joystick for example that uses four digital pins, or a keypad that jogs when you press-and-hold, you could reuse the polling structure but replace the generation of the norm_adc values. So this would mean you would use inject_joy_action called from the idle loop, and keep the logic around the queue depth and movement distances and feed rates to maintain the queue depth and retain similar lag behavior. But instead of using joy_x.raw, it would be delegated to another function that could be swapped out depending on the hardware you have.

This could be organized as a POLL_JOG capability that would enable inject_jog_action() from the idle loop and define most of the inject_jog_action function. It could have additional #ifdef for ANALOG_JOYSTICK that enables all (or some combination) of joy_x and the ADC functionality to read it. Then for keypad jogging a separate #ifdef KEYPAD_JOG (for example) would fill in the norm_adc values based on the current state of the keypad. The 'norm_adc' is then incorrectly named and should probably be norm_jog.

I suppose you could even extend jogging to include an E axis if you wanted, but I would suggest waiting until a concrete use case appears where this is actually desirable. There would be some choices to make in how the feed rates for E are calculated and these are likely to be wrong if I'm speculating about an imaginary future use case.

I'll start putting something together in a separate branch. I don't want to derail this PR.

[vector76]

Well, it turned out to be simple, and I think the outcome is better having poll-based jogging not dependent on the analog joystick. I've got the commit here: vector76@b17e296.

What do you guys think? Should I update the PR? And if I merge this commit into my 'bugfix-2.0.x' branch it should include in in the PR, yes?

[marcio-ao]

@vector76: What function would I call to start jogging in a specific direction? I don't see it in your re-factored code.

EDIT: Oh, I see. I just write the values I want into norm_jog and it starts jogging. That would work for me. If this is merged in, I can add the appropriate methods to ExtUI.

[marcio-ao]

@vector76: I guess norm_jog would need to be a global variable so it can be set from other places, but other than that, it looks pretty good.

[vector76]

I think this organization is an improvement, so jogging can come from other interfaces besides the joystick.

I am concerned the global variable approach would not scale well if there are multiple interfaces to this jogging method. What I had in mind was inserting a function for each interface, so you can keep your own statefulness for your virtual joystick, and the update function would apply that state to the norm_jog variable. This would be analogous to the way calculate_joy_value determines the jog value from the ADC values and applies it to norm_jog.

[marcio-ao]

I am concerned the global variable approach would not scale well if there are multiple interfaces to this jogging method. What I had in mind was inserting a function for each interface, so you can keep your own statefulness for your virtual joystick, and the update function would apply that state to the norm_jog variable.

Okay, I see what you mean. Let's leave your PR exactly as is. I think I know how I can handle the problem from ExtUI without adding extra global variables.

[marcio-ao]

@vector76: On another topic, it's interesting to me that you are injecting moves using GCODE strings and passing them to gcode.parse. There are other ways in Marlin to inject commands into the buffer that don't involve building and then parsing GCODE strings.

I wrote some code here that uses prepare_move_to_destination, for example.

Maybe @thinkyhead can comment on whether he has a preference on how to do this.

[marcio-ao]

@vector76: So, I guess if you compare my code liked above to yours, you'll see they are actually doing very similar things. Here are the similarities and differences:

• My code moves the toolhead towards a specific point in space (which might be adjusted on the fly by the user).
• Your code moves the toolhead in a specific direction.
• My code uses prepare_move_to_destination to inject commands
• Your code uses gode.parse
• Both your code and mine break up the move into small segments to achieve smooth motion and allow quick interruption.

Overall, I think with a little bit more thought, both use cases could be condensed into one, so there isn't duplication of code. So overall, I like the direction this is going :)

[marcio-ao]

Another thought. There is no reason for this code to be in module\temperature.cpp. How about moving it to a new file features\joystick.cpp?

EDIT: On second thought, maybe features\jogging.cpp. The joystick is merely a way to interact with the jogging function. I could see a touch interface as being another way. So jogging seems like a more general name.

[vector76]

@marcio-ao All fair points. I see there is quite a similarity with your code. I think the prepare_move_to_destination approach is probably better. I didn't realize that the planner queue was already reflected in the "current" location, so I didn't see another way to get a relative movement at the end of the planner queue. But your method of incrementing from the current position would work, and I wouldn't have to mess with the relative state (fake G91/G90). It also wouldn't demand that the unparsed buffer be empty. Overall it would be cleaner.

As for whether it should be separated from module\temperature.cpp, it was somewhat (but not completely) justified when it was closely tied to ADC readings. Now with the jogging independent from the ADC stuff, it's a different picture. The joystick ADC stuff sorta fits with the temperature ADC pattern, but the jogging injection definitely feels out of place. I would support moving it to its own file, although I would defer to @thinkyhead as to how it fits in the Grand Vision for how it should be organized overall.

I'll experiment with the prepare_move_to_destination approach and confirm I can get it working. It should be cleaner.

[marcio-ao]

I didn't realize that the planner queue was already reflected in the "current" location, so I didn't see another way to get a relative movement at the end of the planner queue

Yes, this confused me at first. You would expect "current_position" to be where the toolhead is at this instant, but in fact getting that information is not possible in Marlin. Rather "current_position" is where the toolhead will be once all moves in the planner are completed, which may actually be sometime in the future. The trick, as you and I are already using, is to keep the planner queue filled with tiny moves, so that the lag between the actual position and current_position is minimized.

[vector76]

I'm unclear on what needs to happen for this to move forward (or to be rejected). Is there anything I can or should do to get this closed out?

[marcio-ao]

@vector76: I would like to see this merged, as I could build upon it to make a jog control for touch screens.

I'm unclear on what needs to happen for this to move forward (or to be rejected). Is there anything I can or should do to get this closed out?

Well, weren't you going to experiment with using prepare_move_to_destination instead of gcode.parse? That does not appear to have been done yet.

[vector76]

Ah yes, I will look into that within the next few days and report back.

[vector76]

Turns out (with your example) integrating prepare_move_to_destination() was fairly easy. Feels much better than synthesizing a stupid string just to turn around and parse it.

I've compiled this code and run it with my joystick and it works.

[vector76]

One more thing, this update restores the feed rate, whereas the previous string based method would leave the machine with a modified feedrate from the joystick. After finely tuning the position, the feed rate would tend to be extremely low.

Sometimes the first few G0 of my job would leave the feedrate unspecified (by mistake), which is mostly not a problem, and not even noticed if the previous movement was reasonable. But after fine tuning the starting position with the joystick, the starting movements would be ridiculously slow and take forever.

I checked that after slow movements with the joystick, a manual G1 X0 Y0 (without feedrate) travels at the previous speed, not the super slow joystick speed.

[0x3333]

What about using these kinds of handwheel?

[vector76]

What about using these kinds of handwheel?

That style of handwheel can't work with this approach. It's doable but it would need to be a completely separate feature. The reason is that it uses an encoder that sends a stream of digital pulses which need to be decoded digitally, whereas the approach in this PR is based on an analog voltage and ADC readings.

[0x3333]

@vector76 , sorry, I wasn't clear. I know that it is totally different regarding the interface.

Do you guys think that this kind of handle is good for a 3D printer? I used these in CNC machines, and they are super precise when positioning. Like to know if worth the shot, my board has several I/O I can use.

[vector76]

I think it's interesting and probably worth doing. Maybe not even that hard, depending on the encoder resolution and whether an Arduino can keep up. I don't mean to sound like I don't like the idea, I do like the idea, it just doesn't fit in this pull request. I'd suggest a feature request.

[0x3333]

Yeah, I'll buy one of these and give it a shot. Thanks.

[vector76]

What else can I do to help this forward to get it closed out? (I am realizing the importance of feature branches since my "bugfix-2.0.x" is now tied up with this pull request and I am scared if I try to rename the branch it could mess up the PR.)

I believe the Travis-CI failure was an unrelated issue and I don't know how to re-run the check apart from bumping with a meaningless commit.

Let me know if there is anything else I can help with.

[thinkyhead]

I'll do a rebase as part of regular maintenance, and if everything looks clean it will be merged forthwith.

[vector76]

Loving it so far. I like the separation into its own feature instead of being bundled with temperature. Thanks!

[kurtnelle]

@vector76 Great, so now that it is in main stream Marlin, how do I invert and axis (specifically the y axis). My joystick has been mounted upside down, so I need to have the Y axis control be inverted. Also, I will be unable to rewire the pot to achieve this. From what I understand thus far should I multiply norm_jog.y by -1 in the Joystick::calculate function in the #if HAS_JOY_ADC_Y segment?

[vector76]

@kurtnelle yes your approach would work. The inability to easily switch polarity in software is a weakness that I think deserves a fix, so I made this PR: #16466

[kurtnelle]

@vector76 , I got your invert code to work and it's configured (thank you), but now I've noticed that the sensitivity of the stick is way too high. Just for reference, I'm using a thumb stick https://www.sparkfun.com/products/9032 which has a narrow range of travel. How do I adjust the speed scaler?