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

Add Cartographer3D V3 with Touch configuration #684

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

Conversation

typetetris
Copy link

This adds configuration for a Cartographer 3D with Touch enabled and an adxl accelerometer.

Please give it a thorough review, as its my first work on such stuff, which I didn't do only for myself.

Reasons for some of the decisions I made:

We need to be able to override some settings for
[bed_mesh] so the include in user_templates/printer.cfg is placed after the bed mesh definitions and not
in the probe section.

Cartographer3D V3 doesn't touch the bed with the nozzle except for the CARTOGRAPHER_TOUCH command. Therefore we need to pay attention to the nozzle temperature for that. But not for all the other actions like
homing or quad gantry leveling or some such.

The documentation for Cartographer3D advises against an adaptive bed mesh and the full bed mesh will
be fast enough in most cases, so we deactivated
the adaptive bed mesh if a Cartographer3D probe
is used. Maybe this should be configurable? Tell me, and I will alter it.

Also, what do you think about variable_cartographer_touch_max_probing_temp maybe I should simply use
variable_safe_extruder_temp instead?

This would make it easier and more backwards compatible. But for now I took inspiration
from variable_tap_max_probing_temp.

@typetetris typetetris marked this pull request as ready for review January 8, 2025 09:26
We need to be able to override some settings for
`[bed_mesh]` so the include in `user_templates/printer.cfg`
is placed after the bed mesh definitions and not
in the probe section.

Cartographer3D V3 doesn't touch the bed with the nozzle
except for the `CARTOGRAPHER_TOUCH` command. Therefore
we need to pay attention to the nozzle temperature
for that. But not for all the other actions like
homing or quad gantry leveling or some such.

The documentation for Cartographer3D advises against
an adaptive bed mesh and the full bed mesh will
be fast enough in most cases, so we deactivated
the adaptive bed mesh if a Cartographer3D probe
is used.
@Frix-x
Copy link
Owner

Frix-x commented Jan 8, 2025

Hello, thanks for the PR!

It's a good starting point. But there's a couple of changes that are needed to align it more with current Klippain logic and simplify the code.

So the first thing would be to use the ACTIVATE/DEACTIVATE_PROBE to manage the temperature limit, etc... and remove the cartographer_touch action that will not be needed anymore as it can be a simple z_offset instead. Like what is done using the TAP and that is also currently under rework on #637 (Beacon contact PR).

Regarding your decisions, here are my comments:

We need to be able to override some settings for [bed_mesh] so the include in user_templates/printer.cfg is placed after the bed mesh definitions and not in the probe section.

This can be removed since the changes in bed_mesh are pretty anecdotal (they are not needed for it to works) and could be placed in the overrides.cfg file by the user if he want to change the number of points for example. But I agree that it's not the most convenient and I've got some idea to improve this in the future (i.e. adding a variables for the number of point that is defined automatically by the probe_type cfgs and used when calling the BED_MESH_CALIBRATE macro). So this will not be a problem anymore in the near future.

Cartographer3D V3 doesn't touch the bed with the nozzle except for the CARTOGRAPHER_TOUCH command. Therefore we need to pay attention to the nozzle temperature for that. But not for all the other actions like homing or quad gantry leveling or some such.

Yes you are right on this and I've got now way to manage it currently... So I will have to think about a refactor to make it better. In the meantime I would prefer that you use the ACTIVATE/DEACTIVATE_PROBE, even if not perfect, but if the START_PRINT sequence is well defined, it shouldn't be a problem.

The documentation for Cartographer3D advises against an adaptive bed mesh and the full bed mesh will be fast enough in most cases, so we deactivated the adaptive bed mesh if a Cartographer3D probe is used. Maybe this should be configurable? Tell me, and I will alter it.

I've got no problem with that. Maybe we can do the same with the beacon later on as well.

Also, what do you think about variable_cartographer_touch_max_probing_temp maybe I should simply use
variable_safe_extruder_temp instead? This would make it easier and more backwards compatible. But for now I took inspiration
from variable_tap_max_probing_temp.

Yes, this would definitely be better, but it would also be a breaking change for a lot of people unfortunately... So maybe we can move this PR to the develop branch, to allow merging also the beacon contact, move it to variable_safe_extruder_temp there and do a proper release of Klippain with the breaking change in the release notes. I agree that it would be better.

@typetetris
Copy link
Author

So the first thing would be to use the ACTIVATE/DEACTIVATE_PROBE to manage the temperature limit, etc... and remove the cartographer_touch action that will not be needed anymore as it can be a simple z_offset instead.

Regarding using the z_offset action: At the moment I use that to home the z axis again, after doing the tilt_calib, which is recommended by the Cartographer documentation.

Should I include rehoming the z axis in the tilt_calib action if the probe_type_enabled is cartographer_touch or would you suggest a different approach?

@Frix-x
Copy link
Owner

Frix-x commented Jan 16, 2025

Should I include rehoming the z axis in the tilt_calib action if the probe_type_enabled is cartographer_touch or would you suggest a different approach?

No, this is not necessary. In fact, it is necessary for all probes to do this: after a QGL, you have to do a Z home to take a new, more correct height reference. But in Klippain, to avoid wasting time and doing too many Z homeings during the START_PRINT, especially for probes that need to have a lower temperature like TAP, the start print sequences are made by taking this into account automatically and the Z homeings are done most of the time only once, just before taking the final Z offset. So there's no need to add specific things, as it's already taken into account later :)

@elpopo-eng
Copy link
Collaborator

@typetetris We've done something a litle bit different and more transparent in klippain-chocolate. We use carto like a voron tap-probe
You can take a look here and here

@Frix-x
Copy link
Owner

Frix-x commented Jan 21, 2025 via email

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