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

Support beacon_klipper version >= 2.0.0, a.k.a Beacon Contact #637

Open
wants to merge 47 commits into
base: main
Choose a base branch
from

Conversation

crkochan
Copy link

@crkochan crkochan commented Aug 9, 2024

My initial take on fixing #636.

Beacon Contact introduced code in their BeaconHomingHelper class that specifically balks when [homing_override] is present in the Klipper config.

My attempt at working around this was to...

  • Create a separate beacon_contact.cfg in the hardware probes folder
  • Specify beacon_contact as variable_probe_type_enabled to be used by future g-code conditionals
  • Update machine.cfg so that homing_override.cfg is not always included
  • Move the homing_override.cfg include into generic_probe.cfg base macro so that every other probe can still get it

There are still a couple more changes I want to make based on the Beacon documentation that are centered on the start_print macro, but I'm opening this as a draft PR so that I can also include any feedback, since this is my first contribution.

crkochan added 30 commits August 8, 2024 20:54
…plus some whitespace clean-up
Pretty sure this isn’t really neaded, but added just in case.
* Added macro include for activation and deactivation gcode
* Added macro include for homing hooks that re-implement portions of homing_override
Macros re-purposed from vorontap because the nozzle will be touching the bed surface
Pre and post homing hooks are being used to re-implement large portions of functionality from homing_override.

* Current adjustments for sensorless homing
* Endstop backoffs
* Status lighting
* Accel adjustments
Keeps the LEDs from quickly cyling between X and Y homing.
Variable can be used for conditional actions based on the active color value.
beacon_klipper has undocumented hook features that are called before or after homing either the x or y axis
* Fix incorrect command

* Add macro variable to STATUS_LEDS

Variable can be used for conditional actions based on the active color value.

* Present value as a string literal

* Case and quotes adjustments

* Change quote type

* More quotes

* Discard use of delayed_gcode

beacon_klipper has undocumented hook features that are called before or after homing either the x or y axis

* Add xy hooks

* Add activation macro

* Add contact macro

* Case change

* Add recommend homing configuration

* Revert status_leds
@crkochan crkochan marked this pull request as ready for review August 11, 2024 07:08
* Fix incorrect command

* Add macro variable to STATUS_LEDS

Variable can be used for conditional actions based on the active color value.

* Present value as a string literal

* Case and quotes adjustments

* Change quote type

* More quotes

* Discard use of delayed_gcode

beacon_klipper has undocumented hook features that are called before or after homing either the x or y axis

* Add xy hooks

* Add activation macro

* Add contact macro

* Case change

* Add recommend homing configuration

* Revert status_leds

* Add fast QGL/Z-Tilt macros

* Chomp whitespace
* Add calibration to G28 on probe activation

Guarantees some calibration occurs when home_method is set to proximity, and home_autocalibrate is set to none.

* Track probe activation status

Trying to avoid the probing hooks changing status LEDs when they shouldn’t
ksummers92 added a commit to ksummers92/klippain that referenced this pull request Aug 25, 2024
@ksummers92
Copy link
Contributor

I tried your implementation and there are some default values that are not included that causes an error "Option 'home_method' is not valid in section 'beacon'". This is likely due to home_xy_position not being defined by some kind of defaults. Probably best to retrieve the zero reference position/bed center like the homing_override.cfg does and assume that as the home_xy_position.

@crkochan
Copy link
Author

I started working on this PR after I already had Beacon more or less up and running, so you're right, I already had home_xy_position defined in my overrides.cfg file.

Unfortunately, the value needs to be defined outside of a gcode macro, so I can't use the same trick that homing_override.cfg does.

I'm not quite sure what the optimal solution is. I can add the documented recommended defaults into beacon_contact.cfg, and pick a set of coordinates that are not totally outlandish for a Voron Trident or V2, but that might not fly for users of other printers.

Maybe that, plus adding a commented beacon block to the user_templates overrides.cfg, basically some way of signaling that for contact, a user is probably going to want to have some sort of config override setup.

Just to share, this is my beacon block present in overrides.cfg.

[beacon]
serial: /dev/serial/by-id/usb-Beacon_Beacon_RevH_9AA5D66B5154354D38202020FF0A0B36-if00
x_offset: 0
y_offset: 26.5
mesh_main_direction: x
mesh_runs: 2
accel_axes_map: x, y, z
home_xy_position: 150, 150
home_z_hop: 5
home_z_hop_speed: 30
home_xy_move_speed: 300
autocal_speed: 5
home_method: proximity
home_method_when_homed: proximity
home_autocalibrate: never

If home_xy_position is unset, Klipper will throw an `Option 'home_method' is not valid` error message.

Adding it, plus other defaults from the Beacon Contact docs.
* Add calibration to G28 on probe activation

Guarantees some calibration occurs when home_method is set to proximity, and home_autocalibrate is set to none.

* Track probe activation status

Trying to avoid the probing hooks changing status LEDs when they shouldn’t

* Calibrate after tilt

* Update model prior to tilt

* Fix inconsistent indents
Copy link

📌 This pull request has been marked as stale because it has not had activity in the past 30 days.
Please update the PR or comment to keep it active. Otherwise, this will be closed in 14 days.
We appreciate your contribution!

@zanmoskotevc
Copy link

Hey @crkochan, any update on this? I'd love to see support for this added.

@crkochan
Copy link
Author

None from my side. I've been running a series of post-kit-build prints for some new parts and future upgrades using my fork branch, and the pieces have been turning out just fine on my V2.4.

It may not be up to snuff in terms of being merged into mainline because of the need for users to add Contact-specific settings that are perhaps unique to their machine, or class of printer, into their overrides file in order to avoid the potential for cryptic and/or misleading error messages on Klipper start that would come from the Beacon module.

I'm happy to implement any feedback from the maintainers.

@KazachThi
Copy link

KazachThi commented Nov 16, 2024

Hey, @crkochan I have been using your setup to run beacon contact with no problems. All is working as intended.
One thing (and i might be wrong on this one), we have one unnecessary beacon contact calibration.
Order in my case :

  • Homing
  • pre-heating hotend
  • bed heating
  • chamber heating
  • Beacon contact calibration (first)
  • QGL
  • Beacon contact calibration (second)
  • Beacon contact calibration (third)
  • Bed mesh
  • Beacon contact calibration (fourth)
  • Heating hotend
  • Nozzle clean
  • Prime line
  • Printing

So, second and third are redundant ?
Hope this helps.

@crkochan
Copy link
Author

So, second and third are redundant ? Hope this helps.

Yes. Per the Beacon docs, QGL/z tilt adjust and bed mesh should be book-ended by calibrations.

I'm not sure where the extra calibrations are coming from in your case.

@Frix-x
Copy link
Owner

Frix-x commented Jan 6, 2025

Hello, thanks for the PR. I'm back on the subject and bought a beacon to test it. I'll merge it as soon as I get it installed :)

@Frix-x Frix-x self-assigned this Jan 6, 2025
@Frix-x Frix-x added the tracking This issue is tracked and work will be done label Jan 6, 2025
Copy link
Owner

@Frix-x Frix-x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I've made a review of the code (I haven't tested it yet as I'm still waiting for my beacon to arrive), but I already have some changes requested to simplify the code a bit.

I really want to keep it simple for Klippain users to use it and avoid duplicating macros, etc...

Comment on lines +44 to +59
[gcode_macro _beacon_contact_hardware_check]
gcode:
{% if 'mcu beacon' in printer %}
{% set version = printer['mcu beacon'].mcu_version.split(' ')|last %}
{% set major = version.split('.')|first|int %}
{% if version < '2' %}
{action_raise_error('Beacon firmware is too old. Found: %s, Expected: 2.0.0 or higher' % version)}
{% endif %}
{% else %}
{action_raise_error('Beacon MCU was not found')}
{% endif %}

[delayed_gcode _do_beacon_contact_hardware_check]
initial_duration: 5
gcode:
_beacon_contact_hardware_check
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to move this check in the startup.cfg file? There's already a few things that are checked at Klippain startup and this could be added in the _INIT_CHECKPROBECONF macro that is done exactly for this purpose.

https://github.com/Frix-x/klippain/blob/main/macros/miscs/startup.cfg

Comment on lines +1 to +62
# This file is used as an interface to activate/deactivate every probe type
# depending of the needs:
# - Beacon: need to be at a correct nozzle temperature to avoid burning the PEI when used

[gcode_macro ACTIVATE_PROBE]
description: Put the machine in a state being able to probe
variable_temperature: 0
variable_is_active: False
gcode:
{% set beacon_max_probing_temp = printer["gcode_macro _USER_VARIABLES"].beacon_max_probing_temp|float %}

SET_GCODE_VARIABLE MACRO=ACTIVATE_PROBE VARIABLE=is_active VALUE=True

# Check the temperature and lower it if needed
SAVE_GCODE_STATE NAME=BEFORE_BEACON_ACTION

{% set ACTUAL_TEMP = printer.extruder.temperature %}
{% set TARGET_TEMP = printer.extruder.target %}

SET_GCODE_VARIABLE MACRO=ACTIVATE_PROBE VARIABLE=temperature VALUE={TARGET_TEMP}

{% if TARGET_TEMP > beacon_max_probing_temp %}
{ action_respond_info('Extruder temperature target of %.1fC is too high for Beacon probing, lowering to %.1fC' % (TARGET_TEMP, beacon_max_probing_temp)) }
M106 S255 ; 100% the part cooling fan to help the extruder cooling
M109 S{beacon_max_probing_temp}
M106 S0 ; Stop the part cooling fan
{% else %}
# Temperature target is already low enough, but nozzle may still be too hot
{% if ACTUAL_TEMP > beacon_max_probing_temp + 3 %}
M106 S255 ; 100% the part cooling fan to help the extruder cooling
TEMPERATURE_WAIT SENSOR=extruder MAXIMUM={beacon_max_probing_temp}
M106 S0 ; Stop the part cooling fan
{% endif %}
{% endif %}

G28 Z METHOD=CONTACT CALIBRATE=1


[gcode_macro DEACTIVATE_PROBE]
description: Revert the machine to a normal state after probing
gcode:
{% set beacon_deactivation_zhop = printer["gcode_macro _USER_VARIABLES"].beacon_deactivation_zhop %}
{% set Sz = printer["gcode_macro _USER_VARIABLES"].z_drop_speed * 60 %}

G28 Z METHOD=CONTACT CALIBRATE=1

# Check and restore the nozzle temperature if needed
# Small Z hop to avoid restoring the temperature directly on the PEI
{% set z_safe = printer.toolhead.position.z + beacon_deactivation_zhop %}
{% if z_safe > printer.toolhead.axis_maximum.z %}
{% set z_safe = printer.toolhead.axis_maximum.z %}
{% endif %}
G90
G1 Z{z_safe} F{Sz}

# Then restoring the temperature
{% set old_target_temperature = printer["gcode_macro ACTIVATE_PROBE"].temperature %}
M109 S{old_target_temperature}

RESTORE_GCODE_STATE NAME=BEFORE_BEACON_ACTION

SET_GCODE_VARIABLE MACRO=ACTIVATE_PROBE VARIABLE=is_active VALUE=False
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this duplication of ACTIVATE/DEACTIVATE_PROBE doesn't make sense and could be avoided. In fact, these original macros were designed to be generic and accept any type of probe.

If I understand correctly, you made this duplication choice because you added [include ../homing/homing_override.cfg] to the generic_probe.cfg file. But maybe you can move that line to any probe type that needs the default homing_override sequence, and keep the generic_probe.cfg really generic to accept that content for beacon as well.

See my next comments.

@@ -4,6 +4,8 @@
# - Dockable probe: need to be attached/docked when used
# - ... can be improved depending of new probes needs ...

[include ../homing/homing_override.cfg]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this line into each probe type that require it.

@@ -4,6 +4,8 @@
# - Dockable probe: need to be attached/docked when used
# - ... can be improved depending of new probes needs ...

[include ../homing/homing_override.cfg]

[gcode_macro ACTIVATE_PROBE]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And add the beacon specificity in these macros.

# Improvements are from the ANNEX Discord and allow for QGL/Tilt in scan mode
#############################################################################

[gcode_macro FAST_QUAD_GANTRY_LEVEL]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of using a new FAST_... macro for a duplication of the same feature that QGL already provide but adapted to be faster. I'm pretty sure everyone will always want to use the fast version.

So why not merging this in the original overrides files with a check on the beacon_contact (and also standard beacon?) probe type to do the fast version?
https://github.com/Frix-x/klippain/blob/main/macros/base/probing/overrides/qgl.cfg
https://github.com/Frix-x/klippain/blob/main/macros/base/probing/overrides/z_tilt.cfg

This way, when running the standard QGL/Z_TILT macro that is already available in the frontends, it will always work for everyone, using the fast way if you can go fast with a beacon.

{% endif %}


[gcode_macro FAST_Z_TILT_ADJUST]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about the FAST_... version of the Z_TILT_ADJUST macro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracking This issue is tracked and work will be done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants