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

kinematics: add duplication/mirror modes to idex_mode #4464

Closed
wants to merge 5 commits into from

Conversation

Tircown
Copy link
Contributor

@Tircown Tircown commented Jul 6, 2021

Add duplication and mirror modes to idex_modes.
Allows hybrid-corexy and hybrid-corexz to use these modes.

Idex_mode avoids collisions if the user correctly sets the position_min and position_max for stepper_x and dual_carriage.

Do not merge before PR #4445 #4489. I will add a sample configuration and a document on how to set the IDEX parameters.

Signed-off-by: Fabrice GALLET tircown@gmail.com

Tircown added 2 commits July 6, 2021 21:17
Add duplication and mirror modes to idex_modes.
Allows hybrid-corexy and hybrid-corexz to use these modes.
Remove unused var
@DrumClock
Copy link

DrumClock commented Jul 14, 2021

This video shows the functionality of IDEX mods on a test device for Cartesian printers.

https://youtu.be/UbeUd0DCNsc

I used this files for configuraation: IDEX_mode.cfg
and for testing: M605 test all mode.gcode

used files.zip

Test description:

M605 S1 - AUTO-PARK test of automatic exchange of T0 / T1 including z-hop and switching of fan X1 / X2 for print cooling.

M605 S2 - DUPLICATION test of setting carriages offset and synchronization T0 / T1 including both fans X1 and X2

M605 S3 - MIRRORED test of setting carriages offset and synchronization T0 / T1 including synchronization of both fans X1 and X2

M605 S0 - FULL-CONTROL test of functionality of full control T0 / T1 including and switching of fan X1 / X2 for print cooling.

Everything has run several times and I haven't notice any bug in the mods and functionality.

For my part, I recommend adding these modes to FW Klipper.

@Tircown Tircown mentioned this pull request Jul 26, 2021
Fix get_status to be called without eventtime parameter.
Merge a143921
@KevinOConnor
Copy link
Collaborator

Thanks. Sorry for the delay in responding. In general, it looks fine to me.

Some high-level questions:

  1. Is there a reason to add a new SET_DUAL_CARRIAGE_MODE command instead of adding a parameter to the existing SET_DUAL_CARRIAGE command?
  2. What is the intent of the a == b == c statements? (For example, self.dc[0].is_active() == self.dc[1].is_active() == True) I'm not sure what this does in Python.
  3. I'm not sure it's a good idea to use value is False directives, as that would cause confusing results if the value got assigned to a literal 0 (or similar) for some reason.
  4. Does this PR stand on its own, or does it require Add sync_extruder_steppers #4489 to be useful?

Thanks again,
-Kevin

@Tircown
Copy link
Contributor Author

Tircown commented Sep 1, 2021

Thank you for your comments. No problem for the delay.

Is there a reason to add a new SET_DUAL_CARRIAGE_MODE command instead of adding a parameter to the existing SET_DUAL_CARRIAGE command?

The only reason is that duplication and mirroring mode always activate the carriage 0. So if one set the parameter MODE, the parameter CARRIAGE is automatically ignored or must raise an error if the value is not 0.

What is the intent of the a == b == c statements? (For example, self.dc[0].is_active() == self.dc[1].is_active() == True) I'm not sure what this does in Python.

A shorter syntax than self.dc[0].is_active() == True and self.dc[1].is_active() == True. It's probably more confortable to do self.dc[0].is_active() and self.dc[1].is_active()

I'm not sure it's a good idea to use value is False directives, as that would cause confusing results if the value got assigned to a literal 0 (or similar) for some reason.

Ok. For if positive_dir is False I will turn it as it is in the kinematics: if hi.positive_dir. The function is_active() and is_reversed are in the same file as idex_mode and returns a boolean. Do you suggest to change the condition too?

Does this PR stand on its own, or does it require Add sync_extruder_steppers #4489 to be useful?

This PR only adds the duplication and mirroring modes which are not usable without synchronisation of the extruders but this PR stand on it's own code wise.

The documentation I want to produce interferes in some way with #4508. As the config sample does.
The most practical thing for me would be to merge this PR without documentation first, wait for #4508 to be merged (or cancelled), then I'll do a PR to complete the documentation with the implementation of carriages limits (anti-collision system), etc.

@KevinOConnor
Copy link
Collaborator

The function is_active() and is_reversed are in the same file as idex_mode and returns a boolean. Do you suggest to change the condition too?

FWIW, I find the comparisons to True/False (eg, A == True or A is False) to be confusing. I fear different versions of Python may do different things and/or slight value differences could lead to surprising results. I suggest using the more common syntax of if A and if not A.

The most practical thing for me would be to merge this PR without documentation first, wait for #4508 to be merged (or cancelled), then I'll do a PR to complete the documentation with the implementation of carriages limits (anti-collision system), etc.

That's fine with me. One thing we need to figure out, though, is if we want to merge this prior to v0.10.0 release or after. I'm fine with merging before the release, but then we should also have a solution for #4489 before the release. Alternatively, we can look to merge this after the release - in October/November.

Thanks.
-Kevin

@Tircown
Copy link
Contributor Author

Tircown commented Sep 2, 2021

[...] but then we should also have a solution for #4489 before the release.

I would consider the bypass with the [extruder_stepper] as a temporary but working solution to not block the PR. That would let time to rework the extruder architecture. Anyway, those who want to print in dupe/mirrors mode use #4489 files; they are not "blocked".

@KevinOConnor
Copy link
Collaborator

I'm not sure what you're proposing.

-Kevin

@DrumClock
Copy link

DrumClock commented Sep 3, 2021

Hi @KevinOConnor
it would be really good to add duplication / mirror modes to Klipper.
If IDEX printing machines already allow it, why not use it.

Unfortunately, I'm not a programmer, but I offer you help with testing just like I offered @Tircown, with which I work hard to tune the macro M605 Snnn configuration for the Klipper.

Thanks
-Petr Forejt

@charlespick
Copy link
Contributor

Concerning the conflicting documentation,

I say we should merge in the code in this pr with it's appropriate documentation. I am still working on my pr, specifically a mechanism for easily adjusting the alignment of the extruders. Since my PR mainly focuses on files, samples, and documentation for calibrating an IDEX printer, it shouldn't interfere to much with whatever work is introduced here.

I will make whatever adjustments are necessary to my pr to after this one is merged.

@grigi
Copy link
Contributor

grigi commented Sep 17, 2021

@Tircown I just realised that this won't work for Cartesian IDEX printers (like the one I am building).

Reason I'm saying this is:
https://github.com/KevinOConnor/klipper/blob/4d559633e3a3f9e4aba585c30422c8f5772f2b46/klippy/kinematics/cartesian.py#L35

Cartesian handles the dual carriage stuff separately, and doesn't use the idex_modes.DualCarriages object to manage it.
Hence a cartesian IDEX doesn't even have a printer.dual_carriage variable.

@dgz
Copy link
Contributor

dgz commented Oct 23, 2021

I might have missed something, is there a second PR for cartesian kinematics or is that planned for later after this one is merged? The video in the first comment shows the function on a cartesian printer but it appears the new idex_modes only apply to the two hybrid core kinematics.

@KevinOConnor KevinOConnor self-assigned this Jan 8, 2022
@KevinOConnor
Copy link
Collaborator

I do think it would be useful to merge this support into Klipper. If there is still interest in working on this then please update it to the latest code and I will prioritize a review of this work.

Thanks,
-Kevin

@KevinOConnor KevinOConnor added the pending feedback Topic is pending feedback from submitter label Jan 8, 2022
@Tircown
Copy link
Contributor Author

Tircown commented Jan 10, 2022

Thank you. I will do the changes in the next days.

@DrumClock
Copy link

Hi @KevinOConnor
I tested the new "phyton" files from Tircown on FIRMWARE_VERSION: v0.10.0-225-g6e6ad7b5-dirty.

For these IDEX modes:
M605 S1 - AUTO-PARK test of automatic exchange of T0 / T1 including z-hop and switching of fan X1 / X2 for print cooling.
M605 S2 - DUPLICATION test of setting carriages offset and synchronization T0 / T1 including both fans X1 and X2
M605 S3 - MIRRORED test of setting carriages offset and synchronization T0 / T1 including synchronization of both fans X1 and X2
M605 S0 - FULL-CONTROL test of functionality of full control T0 / T1 including and switching of fan X1 / X2 for print cooling.

I did not find any printing error.

I can also provide very sophisticated macros for dual_carriage and M605 mods.

- Following comment 911859551: replacement of A==True and A is False by the more common syntax if A and if not A.
- Add of linebreaks to suit the rule of 80char or less per line
@Tircown Tircown marked this pull request as ready for review January 13, 2022 20:48
@Tircown
Copy link
Contributor Author

Tircown commented Jan 13, 2022

Ready for a new revision step.
(it is the version tested by DrumClock)
Thanks.

@DrumClock
Copy link

DrumClock commented Jan 14, 2022

Hi @KevinOConnor and @Tircown

Note: if printing ends with X2 carriage, then G28 is performed with X2 on which, however there is no BL-Touch (Z-probe).

I have to solve this with [homing_override] and setting:

#### The BL-Touch is located on the X1 carriage ####
#### Reset position X1, Extruder sync and offset ####
 
    SET_GCODE_OFFSET X = 0 Y = 0 Z = 0
    SYNC_STEPPER_TO_EXTRUDER STEPPER = extruder1 EXTRUDER = extruder1
    SET_DUAL_CARRIAGE_MODE MODE = FULL_CONTROL
    ACTIVATE_EXTRUDER EXTRUDER = extruder
    SET_DUAL_CARRIAGE CARRIAGE = 0

@KevinOConnor
Copy link
Collaborator

Thanks. In general, it looks good to me and I think it would be useful to merge this functionality into the Klipper master branch. I have some high-level comments and questions:

  1. I think we should enhance the low-level stepper class so that it is possible to reverse a stepper motor without requiring new "reverse" kinematic code. I will try to put together some sample code for this in the next couple of days.
  2. I'm uncertain on adding a SET_DUAL_CARRIAGE_MODE command. I think the documentation and usage would be more clear if a new mode parameter was added to the existing SET_DUAL_CARRIAGE.
  3. What is the intent of the new toolhead.manual_move() code in restore_idex_state()?
  4. Can you provide a high-level overview of your plans for cartesian idex "mode" support (if any)?

Cheers,
-Kevin

@Tircown
Copy link
Contributor Author

Tircown commented Jan 18, 2022

Thanks.

  1. That's a good point and would be a lot more clean. If it can include the extruder stepper then it would solve some issues for the tilting toolheads. kin_cartesian already has the reverse kinematics (kinematics: Add dual_carriage to hybrid-corexyz #4296).
  2. Ok. If one set the mode parameter in SET_DUAL_CARRIAGE then the carriage parameter must not be set and vice-versa. For duplication and mirrored modes, the carriage 0 is always the master. So there will be 4 valid G-code commands:
    SET_DUAL_CARRIAGE CARRIAGE=0
    SET_DUAL_CARRIAGE CARRIAGE=1
    SET_DUAL_CARRIAGE MODE=DUPLICATION
    SET_DUAL_CARRIAGE MODE=MIRRORED
    I can make this change quite quickly if it meets your expectations.
  3. To not block the G28 [X] when the both carriages are synchronized, the home function in the kinematics temporarily desynchronizes and home each carriages. For duplication mode especially, the carriage_1 must be positioned in the middle of the axis range before set the synchronization back. In some case it's possible to start a duplication print with the carriage_1 positioned closer to the carriage_0 - e.g nested PEE face shields. In some case it's possible to start a mirrored print with a carriage slightly shifted - e.g. unsymmetrical toolheads. Whatever the user sets as initial positioning of each tool, it is restored and the move is accomplish with the toolhead.manual_move().
  4. The cartesian.py file compatible with the idex modes is ready and tested by few beta testers with success. Until now I was waiting to know how much the idex_mode code has to change following your feedback and then if I was still able to adapt the cartesian kinematics myself . I can include the file in this PR, create a separate one now or after this PR merge.

@zruncho3d
Copy link

Looks like the github bot marked this as idle and auto-closed it.

Looks like really useful functionality though, maybe one of the key patches to making IDEX more useful and mainstream.

@Tircown / @KevinOConnor - I'm interested in testing this on a Hybrid-CoreXY IDEX (a Double Dragon), across all modes.

Before I begin, do you have any suggestions for what to focus on here, with regard to testing, to help this get towards mergeable? Thanks.

@novarlynx
Copy link

I'd really like this feature to not get buried and forgotten about. I've got an IDEX printer and i'd really like this feature.

@Googliola
Copy link

It's a pity that the additional modes are not implemented. Icing on the cake....
I have cartesian IDEX and could help testing (I know this branch only supports CoreXY)

@JediRAMA
Copy link

We have made IDEX printer. And Laker made working idex klipper. Can someone check it? there used code from here, and added cartesian kinematics. All IDEX works. Mirror, duplaication and 2 color printing. Input shaping also works for 2 heads. pls check it.
im very noob with coding. but want to help.
https://github.com/Laker87/klipper

@gbkwiatt
Copy link

gbkwiatt commented Dec 5, 2022

I actually just got idex printer, and coming from other klipper based printer I am very surprised to find out that klipper does not yet support all idex modes.

@ujastic
Copy link

ujastic commented Jan 24, 2023

it would be interesting to see support for duplication and mirroring in idex klipper

@gbkwiatt
Copy link

From what I am aware @Laker87 is still working on proper support.

@Googliola
Copy link

We need testers for Lakers fork. I'll do it as soon as possible, anyone else?

@gbkwiatt
Copy link

We need testers for Lakers fork. I'll do it as soon as possible, anyone else?

Is it officially ready for testing ?
I have Cartesian IDEX I can do some tests on. (Sovol SV04)

@nick-parker
Copy link

Aon3D has been running a bunch of machines on a fork I made from @Tircown 's branch over the last several months without much trouble.

I'll look into rebasing our custom stuff onto Laker's and have our test eng. put it through paces if it's ready for testing. I've been putting off polishing my own code well enough to upstream so I'd love to draft off Laker's efforts

@Laker87
Copy link

Laker87 commented Jan 25, 2023

We need testers for Lakers fork. I'll do it as soon as possible, anyone else?

Is it officially ready for testing ? I have Cartesian IDEX I can do some tests on. (Sovol SV04)

Cartesian kinematics support was added by me and it works fine. Already tested by many people.

@Googliola
Copy link

Googliola commented Jan 25, 2023

@Laker87 thanks a lot for your time and efforts. I hope your pull request gets implemented in the main branch!

Someone should re-open this issue.

@DrumClock
Copy link

DrumClock commented Jan 25, 2023

Hi @Laker87
Cartesian kinematics was created by @Tircown, you just added it to your repo,

I tested her for @Tircown already in 2021 here: #4464 (comment)

Otherwise, I can confirm that it works without problems.
@Tircown just needs to tweak a few things that @KevinOConnor n needs for better compatibility.

We all have to wait.

-Cheers
Petr

@DrumClock
Copy link

DrumClock commented Jan 25, 2023

I have created a macro for sync carriage exchange, just using idex_mode duplication.
Video here:
https://youtu.be/lQxptT_Pz28

@Googliola
Copy link

@DrumClock what fork should I use to test / use it? I was under the impression that Tircown's fork only supports coreXY, not cartesian.

@DrumClock
Copy link

Test files for cartesian kinematics provided directly by @Tircown and I don't have permission to redistribute them.
You must contact him directly.
I am still working with us and we are testing it so that everything is error free.

@Laker87
Copy link

Laker87 commented Jan 25, 2023

Test files for cartesian kinematics provided directly by @Tircown and I don't have permission to redistribute them. You must contact him directly. I am still working with us and we are testing it so that everything is error free.

I don't have Tircown's cartesian implementation, I added it by myself.

@gbkwiatt
Copy link

Is there any PR on a horizon then ? Not sure if I should checkout other branches/repos or just wait and test when there is PR opened.

@DrumClock
Copy link

Test files for cartesian kinematics provided directly by @Tircown and I don't have permission to redistribute them. You must contact him directly. I am still working with us and we are testing it so that everything is error free.

I don't have Tircown's cartesian implementation, I added it by myself.

Hi, I'm sorry. So in that case there are two different versions.
I can provide my macros that I have tried files from Tircown.

https://github.com/DrumClock/my_config

Hopefully they will be useful to someone.

@Bully85
Copy link

Bully85 commented Feb 9, 2023

Is there anything new or a working version to test?

@Scanner001
Copy link

Is there any update on the Cartesian duplication and mirror modes. I have 4 IDEX waiting on Klipper and at least one manufacturer.

@Nathan22211
Copy link

@Laker87 did you happen to add examples to your fork as to how to add mirror and duplicate modes into your config?

@Laker87
Copy link

Laker87 commented May 12, 2023

@Laker87 did you happen to add examples to your fork as to how to add mirror and duplicate modes into your config?

Use SET_DUAL_CARRIAGE_MODE MODE=DUPLICATION for duplication mode or SET_DUAL_CARRIAGE_MODE MODE=MIRRORED for mirrored mode in your start g-code.

@TheOnlyBeardedBeast
Copy link

Any update on this?

@DrumClock
Copy link

Klipper already supports IDEX mode

@TheOnlyBeardedBeast
Copy link

TheOnlyBeardedBeast commented Jan 18, 2024

@DrumClock my mistake, not the idex mode, but the dual gantry mode which I am interested in. Sorry maybe I wrote a comment to the wrong PR

@zruncho3d
Copy link

@TheOnlyBeardedBeast I'm not aware of those modes being tested for Dual Gantry kinematics in Klipper.

My patch with the base DG kins lives here:
https://github.com/zruncho3d/klipper/tree/dual_gantry_main

@TheOnlyBeardedBeast
Copy link

TheOnlyBeardedBeast commented Jan 18, 2024

@zruncho3d thanks, in the second half of this year I am planning to build a dual gantry trident based on the dueling zero, can I contact you in need of any advice / help?

@zruncho3d
Copy link

Great! In that case, post on the Dueling-X user project thread in the DoomCube discord. A few people are building Dueling Tridents using the parts from https://github.com/zruncho3d/DuelingX (the generalization of Dueling Zero).

Let's move any conversation there, given this is a closed GitHub thread, for a different kind of kinematics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive Not currently being worked on pending feedback Topic is pending feedback from submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.