Feature request: BLTouch/Servo probe support

[wakass]

I see there are several support questions regarding a bltouch probe, and servo support. But am unable to find any congruous approach to the topic, so I've started thinking about how to implement it 'properly'.

The way i see the probe in this case is starting of at

1. mc_probe_cycle: this is basically called for a g38.2 sequence.

2. Probe definition Separate plugin or additional driver code. This seems to just require overwriting the hal.probe.configure in the driver.c setup. Of course the configure part needs to be able to call the relevant pwm/servo_set functions, which I think are cleanest to implement separately in a servo-type plugin.

3. servo control -> plugin? This would implement M280 commands for debugging and general servo support. It does seem to have a different role than say a spindle. Other issues regarding this topic mention the spindle as a good starting point. I thought about implementing the servo plugin by calling spindle_control functions but it seems to muddle the role of a servo vs e.g. a spindle (with syncing..etc). Perhaps it's better to re-implement some functions found in the spindle_control?

I'd be happy to hear your thoughts.

regards, waka

[terjeio]

A separate plugin (or plugins) is the way to go. Servo control could (should?) be implemented by the ioports analog interface. Analog outputs should have properties such as DAC and PWM, PWM outputs with programatically settable frequency etc? A bltouch plugin could then claim the required port and configure it to its needs.

[wakass]

Hey, i've gone ahead and taken a stab at pwm support (no DACs yet). It works pretty well at this point in driving a bltouch module through the M-commands.

I realize this implementation is a bit half-baked, and could use some massaging. I'm also running into some architectural decision that need to made, maybe you can chime in.

Main implementation

https://github.com/wakass/RP2040/tree/servosupport

List of changes

https://github.com/wakass/RP2040/compare/picocnc-wip...wakass:RP2040:servosupport

Potential issues

pico_cnc_map is the base for this. It has its own implementation for ioports. I've deleted some of this boiler-plate code, which I think should find a better implementation in the framework before it reaches full functional parity.

Plugin

https://github.com/wakass/grlbhal_servo/tree/main

potential issues

Not many so far. Perhaps a better implementation on claiming ports and improvement on integration/dealing with setting pwm values.

Grbl-core fork, analog implementation

https://github.com/wakass/grblhal-core/tree/ioports

Potential issues

I've brazenly overwritten the Settings_IoPort_OD_Enable for now.

Some tips would be welcome. Cheers

[terjeio]

Nice, I have left some comments in your commits.

[wakass]

Changes ongoing on this plugin overhere. Code is based on the marlin implementation here.

I'm running into some delay in the stowing of the probe (roughly 0.1-0.5 s) after triggering. Ideally there would be little to no delay in the stowing of the probe after the trigger is issued by the probe.

Two possible issues might interplay:

1. Delay in the bltouch-driver, as implemented with hal.delay_ms. Not sure what is the right delay function here as it's in the motion_control loop, and how this interacts.
2. A general delay/interupt issue? Not sure how to approach this. However, when I issue a stow command from the loop below (when probe.triggered) there is very little delay, but the general probe trigger doesnt fire anymore. Most likely due to the alarm on the probe being disabled on being stowed.

https://github.com/grblHAL/RP2040/blob/ed7d90800b41376a29081c463c7e8810992e64c8/driver.c#L2367-L2380

[terjeio]

1. Try delay_sec() instead of hal.delay_ms()? This keeps the main protocol loop active during long delays.

2. You should not (or rather can not) call code that is not exclusively used by the interrupt handler as most is not reentrant. To initiate lenghty processing from an interrupt handler queue it for the foreground process to handle by registering a callback via protocol_enqueue_rt_command(). However, stowing from the interrupt handler is not possible if you want bltouch support to be available for all drivers without code duplication. IMO bltouch support should be in a plugin, not in the driver. It should be added to the servo plugin?

[wakass]

Thanks for the feedback. Already getting some better results with delay_sec and an enqueue_rt. There do appear some instances where it's a bit unreliable. It stows, but doesn't register a probe-event.

1. I was also wondering on: https://github.com/grblHAL/RP2040/blob/ed7d90800b41376a29081c463c7e8810992e64c8/driver.c#L2375C31-L2375C31

Is the order of LOW,HIGH correct? Compared to the next line? I would naively expect these two code paths to re-enable the same interrupt.

2. Agree on the plugin comment. It was just a matter of convenience for now to make it 'part of' the driver. As far as bltouch as part of the servo plugin: For practicalities' sake it indeed might just be the most easy. It also has the direct dependency. I'll stow it (harhar) in there for now, and should there be major issues with it, it can always be changed.

[terjeio]

Is the order of LOW,HIGH correct? Compared to the next line?

I did not write this code, but it was me that suggested a coded SR latch to trap probe triggered as fast as possible without any potential delay due to switch bounces. This since the probe input is polled in the stepper interrupt handler. At least for the probe input it might be better to use edge trigger instead of level? And the SR latch (probe.triggered) should be set to the correct state when the probe input is configured?

The RP2040 driver is currrently the only driver that has the SR latch idea implemented, I guess that I should look into this for other drivers as well and at the same time try to implement better probe protection. For those drivers that can have the probe input attached to an interrupt it might be useful to trigger estop or feed hold if the probe is triggered during normal motion?

[wakass]

Ah i see. Does this mean the probe input is immediately causing a halt in the stepping? I'm not able to directly observe this kind of behaviour since i'm desktop testing with just the probe. This might protect the probe more than I'm naively assuming at this point. I suspect it's possible to treat the probe similarly to any limit-switches. The safety door also has some latching behaviour.

I think i kind of follow the HIGH/LOW logic, with the debounce logic forcing a delay in registering for detecting a "de-assert" condition. Perhaps a longer debounce time would fix the instabilities i'm seeing.

[terjeio]

Does this mean the probe input is immediately causing a halt in the stepping?

Yes, by decelerating to a halt - not an abrupt stop. There is a compile time option to "flush" the segment buffer of most of the queued motion in order to start decelerate faster, this to reduce overshoot.

I suspect it's possible to treat the probe similarly to any limit-switches.

Yes, but it is not straightforward to implement - a setting is needed and/or an input for "probe connected". I do not want a flood of complaints about motion beeing stopped by a noisy probe input...

Perhaps a longer debounce time would fix the instabilities i'm seeing.

There should not be any debounce for the probe input. The idea behind the SR latch was that the interrupt handler should set the latch (probe.triggered) on the first contact, then reset via a call to probeConfigure().

[andrewmarles]

FWIW, I have implemented a bunch of the "probe connected" features in my probing plugin:

https://github.com/Expatria-Technologies/grblhal_probe_plugin

When the probe is connected and protection is enabled, the probe signal is polled on all non-probing moves as part of the probe protection. Probe connected signal can be asserted in various ways.

I have no experience with how BLTouch works, but maybe the above plugin is structurally useful - it is not driver specific other than if using an external pin for the probe connection signal it must be interrupt capable.

[wakass]

Preliminary alpha version now up here Implementation is here

No integration tests (i.e. integrated in setup) done yet.