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

Added beacon support #1350

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added beacon support #1350

wants to merge 1 commit into from

Conversation

Zeanon
Copy link

@Zeanon Zeanon commented May 2, 2024

As more and more people are running beacon, it would make sense to allow people to properly use KlipperScreen with that probe.
These changes just check whether the beacon_calibrate command exists and adjust accordingly. completely backwards compatible, no breaking change.

Check whether beacon_calibrate exists and if so, use Beacon_Calibrate instead of Probe_Calibrate
@alfrix
Copy link
Collaborator

alfrix commented May 4, 2024

As i answered in #1050 there are a lot of extra modules at this point and adding them would quickly result in a big burden, i would prefer sticking to mainline Klipper.

I've seen that fairly recently probe_eddy_current has been added to mainline

maybe the beacon can be configured as one of those?

but it seems that work may not be complete: Klipper3d/klipper#6558

@Zeanon
Copy link
Author

Zeanon commented May 4, 2024

Doesn't seem like the best stance since beacon is becoming increasingly popular but can't be merged into mainline due to licensing issues but hey what do I know, I'm just a measily user wanting to properly use my printer.

@alfrix
Copy link
Collaborator

alfrix commented May 4, 2024

I'm just a measily user wanting to properly use my printer.

that's how i started too

let's address the issues: it seems that you are making the BEACON_CALIBRATE a replacement for PROBE_CALIBRATE if it's available, that doesn't sound correct, it would be the same as doing:

[gcode_macro PROBE_CALIBRATE]
rename_existing: PROBE_CALIBRATE_BASE
gcode:
    BEACON_CALIBRATE

at that point i'm just confused on why they haven't registered the command as PROBE_CALIBRATE

in addition you need to add the section name here: (used to show the z_offset in various places)

probe_types = ["probe", "bltouch", "smart_effector", "dockable_probe"]

@Zeanon
Copy link
Author

Zeanon commented May 4, 2024

that's a good point
Reason for not registering it as a probe is to avoid confusion, according to the beacon devs
Using the wrapper macro is a good idea though.
I might look into what needs to be modified as well to make it work properly, make it an additional method next to probe_calibrate and redo the pr I guess?
Cause since the dockable probe module from DK is in there, I see no reason not to include Beacon as well

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.

None yet

2 participants